[Libguestfs] [PATCH v2] resize: support non-local output disks (RHBZ#1404182)
Richard W.M. Jones
rjones at redhat.com
Fri Feb 3 15:27:42 UTC 2017
On Thu, Feb 02, 2017 at 02:34:24PM +0100, Pino Toscano wrote:
> Parse the output disk as URI, and use all its attributes just like
> it is done for the input disk. The only change is that the fsync of the
> output disk is limited now for local URIs only, since it will not work
> with remote protocols.
> ---
> resize/resize.ml | 43 +++++++++++++++++++++++++++++++------------
> resize/virt-resize.pod | 6 +++---
> 2 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/resize/resize.ml b/resize/resize.ml
> index 19cd8df..7e16cb5 100644
> --- a/resize/resize.ml
> +++ b/resize/resize.ml
> @@ -315,6 +315,13 @@ read the man page virt-resize(1).
> error (f_"error parsing URI '%s'. Look for error messages printed above.")
> infile in
>
> + (* outfile can be a URI. *)
> + let outfile =
> + try (outfile, URI.parse_uri outfile)
> + with Invalid_argument "URI.parse_uri" ->
> + error (f_"error parsing URI '%s'. Look for error messages printed above.")
> + outfile in
This matches what we do with 'infile', but I wonder if that isn't
wrong, ie. we should warn or even ignore if we couldn't parse the
infile / outfile as a URI and construct a local file URI.
[...]
The following would be a bit safer if all the new variables
it introduces were scoped, ie:
let () =
> + let _, { URI.path = path; protocol = protocol;
> + server = server; username = username;
> + password = password } = outfile in
> (* The output disk is being created, so use cache=unsafe here. *)
> g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe"
> - outfile;
> + ~protocol ?server ?username ?secret:password path;
> if not (quiet ()) then Progress.set_up_progress_bar ~machine_readable g;
> g#launch ()
^ in
[...]
and the same here:
> let g = open_guestfs () in
> + let _, { URI.path = path; protocol = protocol;
> + server = server; username = username;
> + password = password } = outfile in
> (* The output disk is being created, so use cache=unsafe here. *)
> g#add_drive ?format:output_format ~readonly:false ~cachemode:"unsafe"
> - outfile;
> + ~protocol ?server ?username ?secret:password path;
> if not (quiet ()) then Progress.set_up_progress_bar ~machine_readable g;
> g#launch ();
> + (* Try to sync the destination disk only if it is a local file. *)
> + let () =
> + let _, { URI.protocol = protocol; path = path } = outfile in
> + match protocol with
> + | "" | "file" ->
Simpler to do:
match outfile with
| _, { URI.protocol = ("" | "file"); path = path } ->
> + (* Because we used cache=unsafe when writing the output file, the
> + * file might not be committed to disk. This is a problem if qemu is
> + * immediately used afterwards with cache=none (which uses O_DIRECT
> + * and therefore bypasses the host cache). In general you should not
> + * use cache=none.
> + *)
> + Fsync.file path
> + | _ -> () in
ACK with the changes above.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
More information about the Libguestfs
mailing list