[Libguestfs] [PATCH v2v 2/3] -o disk, -o libvirt, -o qemu: Implement -of qcow2 -oo compressed

Laszlo Ersek lersek at redhat.com
Tue Jul 5 06:58:28 UTC 2022


On 07/01/22 13:01, Richard W.M. Jones wrote:
> For various output modes, implement -oo compressed which can be used
> to generate compressed qcow2 files.  This option was dropped when
> modularizing virt-v2v, and required changes to nbdcopy which are
> finally upstream in libnbd >= 1.13.5.
> 
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2047660
> Fixes: commit 255722cbf39afc0b012e2ac00d16fa6ba2f8c21f
> Reported-by: Xiaodai Wang
> ---
>  TODO                     | 14 --------------
>  output/output_disk.ml    | 29 +++++++++++++++++++++--------
>  output/output_libvirt.ml | 31 ++++++++++++++++++++++---------
>  output/output_qemu.ml    | 25 ++++++++++++++++++-------
>  4 files changed, 61 insertions(+), 38 deletions(-)
> 
> diff --git a/TODO b/TODO
> index f578d50621..04b1dd2036 100644
> --- a/TODO
> +++ b/TODO
> @@ -1,17 +1,3 @@
> -virt-v2v -o disk|qemu -oo compressed
> -------------------------------------
> -
> -This was temporarily dropped when I modularized virt-v2v.  It would
> -not be too difficult to add it back.  The following is the qemu-nbd
> -command required (to be run as the output helper) which creates a
> -compressed qcow2 disk image:
> -
> -$ qemu-nbd --image-opts driver=compress,file.driver=qcow2,file.file.driver=file,file.file.filename=new.qcow2
> -
> -Note this requires fixes in nbdcopy so it obeys the advertised block
> -alignment:
> -https://lists.gnu.org/archive/html/qemu-block/2022-01/threads.html#00729
> -
>  virt-v2v -o rhv-upload
>  ----------------------
>  
> diff --git a/output/output_disk.ml b/output/output_disk.ml
> index bc5b4e1ca6..abcfcdc0ce 100644
> --- a/output/output_disk.ml
> +++ b/output/output_disk.ml
> @@ -30,7 +30,7 @@ open Create_libvirt_xml
>  open Output
>  
>  module Disk = struct
> -  type poptions = Types.output_allocation * string * string * string
> +  type poptions = bool * Types.output_allocation * string * string * string
>  
>    type t = unit
>  
> @@ -41,11 +41,21 @@ module Disk = struct
>        | None -> ""
>  
>    let query_output_options () =
> -    printf (f_"No output options can be used in this mode.\n")
> +    printf (f_"Output options that can be used with -o disk:
> +
> +  -oo compressed      Compress the output file (used only with -of qcow2)
> +")
>  
>    let parse_options options source =
> -    if options.output_options <> [] then
> -      error (f_"no -oo (output options) are allowed here");
> +    let compressed = ref false in
> +    List.iter (
> +      function
> +      | "compressed", "" -> compressed := true
> +      | "compressed", v -> compressed := bool_of_string v
> +      | k, _ ->
> +         error (f_"-o disk: unknown output option ‘-oo %s’") k
> +    ) options.output_options;
> +
>      if options.output_password <> None then
>        error_option_cannot_be_used_in_output_mode "local" "-op";
>  
> @@ -60,11 +70,13 @@ module Disk = struct
>  
>      let output_name = Option.default source.s_name options.output_name in
>  
> -    options.output_alloc, options.output_format, output_name, output_storage
> +    !compressed, options.output_alloc, options.output_format,
> +    output_name, output_storage
>  
>    let setup dir options source =
>      let disks = get_disks dir in
> -    let output_alloc, output_format, output_name, output_storage = options in
> +    let compressed, output_alloc, output_format, output_name, output_storage =
> +      options in
>  
>      List.iter (
>        fun (i, size) ->
> @@ -73,11 +85,12 @@ module Disk = struct
>  
>          (* Create the actual output disk. *)
>          let outdisk = disk_path output_storage output_name i in
> -        output_to_local_file output_alloc output_format outdisk size socket
> +        output_to_local_file ~compressed output_alloc output_format
> +          outdisk size socket
>      ) disks
>  
>    let finalize dir options () source inspect target_meta =
> -    let output_alloc, output_format, output_name, output_storage = options in
> +    let _, output_alloc, output_format, output_name, output_storage = options in
>  
>      (* Convert metadata to libvirt XML. *)
>      (match target_meta.target_firmware with
> diff --git a/output/output_libvirt.ml b/output/output_libvirt.ml
> index e0d3432d25..04b4c5f81e 100644
> --- a/output/output_libvirt.ml
> +++ b/output/output_libvirt.ml
> @@ -32,7 +32,7 @@ open Create_libvirt_xml
>  open Output
>  
>  module Libvirt_ = struct
> -  type poptions = Libvirt.rw Libvirt.Connect.t Lazy.t *
> +  type poptions = Libvirt.rw Libvirt.Connect.t Lazy.t * bool *
>                    Types.output_allocation * string * string * string
>  
>    type t = string * string
> @@ -44,11 +44,21 @@ module Libvirt_ = struct
>        | None -> ""
>  
>    let query_output_options () =
> -    printf (f_"No output options can be used in this mode.\n")
> +    printf (f_"Output options that can be used with -o libvirt:
> +
> +  -oo compressed      Compress the output file (used only with -of qcow2)
> +")
>  
>    let parse_options options source =
> -    if options.output_options <> [] then
> -      error (f_"no -oo (output options) are allowed here");
> +    let compressed = ref false in
> +    List.iter (
> +      function
> +      | "compressed", "" -> compressed := true
> +      | "compressed", v -> compressed := bool_of_string v
> +      | k, _ ->
> +         error (f_"-o disk: unknown output option ‘-oo %s’") k
> +    ) options.output_options;
> +
>      if options.output_password <> None then
>        error_option_cannot_be_used_in_output_mode "libvirt" "-op";
>  
> @@ -59,12 +69,13 @@ module Libvirt_ = struct
>  
>      let output_name = Option.default source.s_name options.output_name in
>  
> -    (conn, options.output_alloc, options.output_format, output_name,
> -     output_pool)
> +    (conn, !compressed, options.output_alloc, options.output_format,
> +     output_name, output_pool)
>  
>    let setup dir options source =
>      let disks = get_disks dir in
> -    let conn, output_alloc, output_format, output_name, output_pool = options in
> +    let conn, compressed, output_alloc, output_format,
> +        output_name, output_pool = options in
>      let conn = Lazy.force conn in
>  
>      (* Get the capabilities from libvirt. *)
> @@ -119,13 +130,15 @@ module Libvirt_ = struct
>  
>          (* Create the actual output disk. *)
>          let outdisk = target_path // output_name ^ "-sd" ^ (drive_name i) in
> -        output_to_local_file output_alloc output_format outdisk size socket
> +        output_to_local_file ~compressed output_alloc output_format
> +          outdisk size socket
>      ) disks;
>  
>      (capabilities_xml, pool_name)
>  
>    let rec finalize dir options t source inspect target_meta =
> -    let conn, output_alloc, output_format, output_name, output_pool = options in
> +    let conn, _, output_alloc, output_format, output_name, output_pool =
> +      options in
>      let capabilities_xml, pool_name = t in
>  
>      (match target_meta.target_firmware with
> diff --git a/output/output_qemu.ml b/output/output_qemu.ml
> index 3269fba501..5081b79f99 100644
> --- a/output/output_qemu.ml
> +++ b/output/output_qemu.ml
> @@ -29,7 +29,8 @@ open Utils
>  open Output
>  
>  module QEMU = struct
> -  type poptions = bool * Types.output_allocation * string * string * string
> +  type poptions = bool * bool *
> +                  Types.output_allocation * string * string * string
>  
>    type t = unit
>  
> @@ -42,6 +43,7 @@ module QEMU = struct
>    let query_output_options () =
>      printf (f_"Output options (-oo) which can be used with -o qemu:
>  
> +  -oo compressed      Compress the output file (used only with -of qcow2)
>    -oo qemu-boot       Boot the guest in qemu after conversion
>  ")
>  
> @@ -49,10 +51,16 @@ module QEMU = struct
>      if options.output_password <> None then
>        error_option_cannot_be_used_in_output_mode "qemu" "-op";
>  
> -    let qemu_boot = ref false in
> +    let compressed = ref false
> +    and qemu_boot = ref false in
>      List.iter (
>        fun (k, v) ->
>          match k with
> +        | "compressed" ->
> +           if v = "" || v = "true" then compressed := true
> +           else if v = "false" then compressed := false
> +           else
> +             error (f_"-o qemu: use -oo compressed[=true|false]")
>          | "qemu-boot" ->
>             if v = "" || v = "true" then qemu_boot := true
>             else if v = "false" then qemu_boot := false

The pre-patch code could perhaps do with a bit of refactoring (I really
liked the bool_of_string approach with the single-level match above),
but that's not a blocker for this patch.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo

> @@ -61,7 +69,8 @@ module QEMU = struct
>          | k ->
>             error (f_"-o qemu: unknown output option ‘-oo %s’") k
>        ) options.output_options;
> -    let qemu_boot = !qemu_boot in
> +    let compressed = !compressed
> +    and qemu_boot = !qemu_boot in
>  
>      (* -os must be set to a directory. *)
>      let output_storage =
> @@ -74,12 +83,13 @@ module QEMU = struct
>  
>      let output_name = Option.default source.s_name options.output_name in
>  
> -    (qemu_boot, options.output_alloc, options.output_format,
> +    (compressed, qemu_boot, options.output_alloc, options.output_format,
>       output_name, output_storage)
>  
>    let setup dir options source =
>      let disks = get_disks dir in
> -    let _, output_alloc, output_format, output_name, output_storage = options in
> +    let compressed, _, output_alloc, output_format,
> +        output_name, output_storage = options in
>  
>      List.iter (
>        fun (i, size) ->
> @@ -88,11 +98,12 @@ module QEMU = struct
>  
>          (* Create the actual output disk. *)
>          let outdisk = disk_path output_storage output_name i in
> -        output_to_local_file output_alloc output_format outdisk size socket
> +        output_to_local_file ~compressed output_alloc output_format
> +          outdisk size socket
>      ) disks
>  
>    let finalize dir options () source inspect target_meta =
> -    let qemu_boot, output_alloc, output_format,
> +    let _, qemu_boot, output_alloc, output_format,
>          output_name, output_storage = options in
>  
>      let { guestcaps; target_buses; target_firmware } = target_meta in
> 



More information about the Libguestfs mailing list