[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH v3 02/13] v2v: factor out overlay creation



On Tue, Oct 20, 2015 at 04:08:10PM +0300, Roman Kagan wrote:
> Iterating over source disks and creating temporary overlays for easy
> rollback fits nicely into a separate function.  In addition, determining
> their size doesn't need to wait until the guestfs is launched: the size
> can be obtained via disk_virtual_size() method.
> 
> Signed-off-by: Roman Kagan <rkagan virtuozzo com>
> ---
>  v2v/v2v.ml | 91 +++++++++++++++++++++++++++++---------------------------------
>  1 file changed, 42 insertions(+), 49 deletions(-)
> 
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index 564c5da..155eb83 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -57,40 +57,7 @@ let rec main () =
>  
>    let source = open_source input print_source in
>    let source = amend_source source output_name network_map in
> -
> -  (* Create a qcow2 v3 overlay to protect the source image(s).  There
> -   * is a specific reason to use the newer qcow2 variant: Because the
> -   * L2 table can store zero clusters efficiently, and because
> -   * discarded blocks are stored as zero clusters, this should allow us
> -   * to fstrim/blkdiscard and avoid copying significant parts of the
> -   * data over the wire.
> -   *)
> -  message (f_"Creating an overlay to protect the source from being modified");
> -  let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in
> -  let overlays =
> -    List.map (
> -      fun ({ s_qemu_uri = qemu_uri; s_format = format } as source) ->
> -        let overlay_file =
> -          Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in
> -        unlink_on_exit overlay_file;
> -
> -        let options =
> -          "compat=1.1" ^
> -            (match format with None -> ""
> -            | Some fmt -> ",backing_fmt=" ^ fmt) in
> -        let cmd =
> -          sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s"
> -            (quote qemu_uri) (quote options) overlay_file in
> -        if verbose () then printf "%s\n%!" cmd;
> -        if Sys.command cmd <> 0 then
> -          error (f_"qemu-img command failed, see earlier errors");
> -
> -        (* Sanity check created overlay (see below). *)
> -        if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then
> -          error (f_"internal error: qemu-img did not create overlay with backing file");
> -
> -        overlay_file, source
> -    ) source.s_disks in
> +  let overlays = create_overlays source.s_disks in
>  
>    (* Open the guestfs handle. *)
>    message (f_"Opening the overlay");
> @@ -100,7 +67,7 @@ let rec main () =
>    if verbose () then g#set_verbose true;
>    g#set_network true;
>    List.iter (
> -    fun (overlay_file, _) ->
> +    fun ({ov_overlay_file = overlay_file}) ->
>        g#add_drive_opts overlay_file
>          ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort"
>          ~copyonread:true
> @@ -108,20 +75,6 @@ let rec main () =
>  
>    g#launch ();
>  
> -  (* Create the list of overlays structs.  Query each disk for its
> -   * virtual size, and fill in a few other fields.
> -   *)
> -  let overlays =
> -    mapi (
> -      fun i (overlay_file, source) ->
> -        let sd = "sd" ^ drive_name i in
> -        let dev = "/dev/" ^ sd in
> -        let vsize = g#blockdev_getsize64 dev in
> -
> -        { ov_overlay_file = overlay_file; ov_sd = sd;
> -          ov_virtual_size = vsize; ov_source = source }
> -    ) overlays in
> -
>    (* Work out where we will write the final output.  Do this early
>     * just so we can display errors to the user before doing too much
>     * work.
> @@ -385,6 +338,7 @@ let rec main () =
>  
>    (* Save overlays if --debug-overlays option was used. *)
>    if debug_overlays then (
> +    let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in
>      List.iter (
>        fun ov ->
>          let saved_filename =
> @@ -460,6 +414,45 @@ and amend_source source output_name network_map =
>  
>    { source with s_nics = nics }
>  
> +and create_overlays src_disks =
> +  (* Create a qcow2 v3 overlay to protect the source image(s).  There
> +   * is a specific reason to use the newer qcow2 variant: Because the
> +   * L2 table can store zero clusters efficiently, and because
> +   * discarded blocks are stored as zero clusters, this should allow us
> +   * to fstrim/blkdiscard and avoid copying significant parts of the
> +   * data over the wire.
> +   *)
> +  message (f_"Creating an overlay to protect the source from being modified");
> +  let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in
> +  List.mapi (
> +    fun i ({ s_qemu_uri = qemu_uri; s_format = format } as source) ->
> +      let overlay_file =
> +        Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in
> +      unlink_on_exit overlay_file;
> +
> +      let options =
> +        "compat=1.1" ^
> +          (match format with None -> ""
> +          | Some fmt -> ",backing_fmt=" ^ fmt) in
> +      let cmd =
> +        sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s"
> +          (quote qemu_uri) (quote options) overlay_file in
> +      if verbose () then printf "%s\n%!" cmd;
> +      if Sys.command cmd <> 0 then
> +        error (f_"qemu-img command failed, see earlier errors");
> +
> +      (* Sanity check created overlay (see below). *)
> +      if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then
> +        error (f_"internal error: qemu-img did not create overlay with backing file");
> +
> +      let sd = "sd" ^ drive_name i in
> +
> +      let vsize = (new G.guestfs ())#disk_virtual_size overlay_file in
> +
> +      { ov_overlay_file = overlay_file; ov_sd = sd;
> +        ov_virtual_size = vsize; ov_source = source }
> +  ) src_disks
> +
>  and inspect_source g root_choice =
>    let roots = g#inspect_os () in
>    let roots = Array.to_list roots in

Pretty much refactoring, but you're combining the two loops together
and using #disk_virtual_size instead of #blockdev_getsize64.  Looks
good, so:

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]