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

If this information is useful, please help other people find it:
Share via:

Richard W.M. Jones

2022-Jul-01 11:01 UTC

[Libguestfs] [PATCH v2v 0/3] Implement -of qcow2 -oo compressed

Pre-modularised virt-v2v allows you to use either --compress or -oo
compressed which basically added the -c option to qemu-img convert,
compressing the qcow2 output.  Since we switched over to nbdcopy this
option is no longer available.  The alternative is to set up the
"compress" driver in qemu-nbd.  However that didn't work because
nbdcopy used to ignore the target minimum block size, and the compress
driver requires you to obey the 64k minimum when writing, else it
fails.  Now that nbdcopy 1.13.5 actually fixes this problem, we can
reimplement -oo compressed.

This is a minimal fix for https://bugzilla.redhat.com/show_bug.cgi?id=2047660

I only implemented this for small selection of output modes that go to
local files.

Old virt-v2v also implemented this option (at least, in theory) for -o
glance, -o rhv and -o vdsm, but all of these modes are deprecated to
some degree.  -o openstack which replaces glance only allows raw
format uploads, and -o rhv-upload which replaces -o rhv/vdsm does its
own thing completely.

Rich.

Richard W.M. Jones

2022-Jul-01 11:01 UTC

head link

[Libguestfs] [PATCH v2v 1/3] qemu-nbd: Implement output compression for qcow2 files

---
 lib/qemuNBD.ml    | 11 +++++++++--
 lib/qemuNBD.mli   |  5 +++++
 output/output.ml  | 38 +++++++++++++++++++++++++++++++++++---
 output/output.mli |  1 +
 4 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/lib/qemuNBD.ml b/lib/qemuNBD.ml
index ae21b17c7d..bbb65f4155 100644
--- a/lib/qemuNBD.ml
+++ b/lib/qemuNBD.ml
@@ -55,14 +55,16 @@ type cmd = {
   disk : string;
   mutable snapshot : bool;
   mutable format : string option;
+  mutable imgopts : bool;
 }
 
-let create disk = { disk; snapshot = false; format = None }
+let create disk = { disk; snapshot = false; format = None; imgopts = false }
 
 let set_snapshot cmd snap = cmd.snapshot <- snap
 let set_format cmd format = cmd.format <- format
+let set_image_opts cmd imgopts = cmd.imgopts <- imgopts
 
-let run_unix socket { disk; snapshot; format } +let run_unix socket { disk;
snapshot; format; imgopts }    assert (disk <> "");
 
   (* Create a temporary directory where we place the PID file. *)
@@ -85,6 +87,11 @@ let run_unix socket { disk; snapshot; format }    (* -s adds
a protective overlay. *)
   if snapshot then List.push_back args "-s";
 
+  (* --image-opts reinterprets the filename parameter as a set of
+   * image options.
+   *)
+  if imgopts then List.push_back args "--image-opts";
+
   if have_selinux && qemu_nbd_has_selinux_label_option () then (
     List.push_back args "--selinux-label";
     List.push_back args "system_u:object_r:svirt_socket_t:s0"
diff --git a/lib/qemuNBD.mli b/lib/qemuNBD.mli
index e10d3106c7..afe9d944e1 100644
--- a/lib/qemuNBD.mli
+++ b/lib/qemuNBD.mli
@@ -43,6 +43,11 @@ val set_snapshot : cmd -> bool -> unit
 val set_format : cmd -> string option -> unit
 (** Set the format [--format] parameter. *)
 
+val set_image_opts : cmd -> bool -> unit
+(** Set whether the [--image-opts] parameter is used.  This changes
+    the meaning of the [filename] parameter to a set of image options.
+    Consult the qemu-nbd man page for more details. *)
+
 val run_unix : string -> cmd -> string * int
 (** Start qemu-nbd command listening on a Unix domain socket,
     waiting for the process to start up.
diff --git a/output/output.ml b/output/output.ml
index 5c6670b99c..5fe8555a7a 100644
--- a/output/output.ml
+++ b/output/output.ml
@@ -69,7 +69,7 @@ let error_if_disk_count_gt dir n    if Sys.file_exists socket
then
     error (f_"this output module doesn't support copying more than %d
disks") n
 
-let output_to_local_file ?(changeuid = fun f -> f ())
+let output_to_local_file ?(changeuid = fun f -> f ()) ?(compressed = false)
       output_alloc output_format filename size socket    (* Check nbdkit is
installed and has the required plugin. *)
   if not (Nbdkit.is_installed ()) then
@@ -78,6 +78,10 @@ let output_to_local_file ?(changeuid = fun f -> f ())
     error (f_"nbdkit-file-plugin is not installed or not working");
   let nbdkit_config = Nbdkit.config () in
 
+  (* Only allow compressed with -of qcow2. *)
+  if compressed && output_format <> "qcow2" then
+    error (f_"?-oo compressed? is only allowed when the output format is a
local qcow2-format file, ie ?-of qcow2?");
+
   let g = open_guestfs () in
   let preallocation      match output_alloc with
@@ -103,9 +107,37 @@ let output_to_local_file ?(changeuid = fun f -> f ())
      On_exit.kill pid
 
   | "qcow2" ->
-     let cmd = QemuNBD.create filename in
+     let cmd +       if not compressed then (
+         let cmd = QemuNBD.create filename in
+         QemuNBD.set_format cmd (Some "qcow2");
+         cmd
+       )
+       else (
+         (* Check nbdcopy is new enough.  This assumes that
+          * the version of libnbd is the same as the version
+          * of nbdcopy, but parsing this is easier.  We can
+          * remove this check when we build-depend on
+          * libnbd >= 1.14.
+          *)
+         let () +           let version = NBD.create () |> NBD.get_version
in
+           let version = String.nsplit "." version in
+           let version = List.map int_of_string version in
+           if version < [1; 13; 5] then
+             error (f_"-oo compressed option requires nbdcopy >=
1.13.5") in
+
+         let qemu_quote str = String.replace str "," ",,"
in
+         let image_opts = [ "driver=compress";
+                            "file.driver=qcow2";
+                            "file.file.driver=file";
+                            "file.file.filename=" ^ qemu_quote
filename ] in
+         let image_opts = String.concat "," image_opts in
+         let cmd = QemuNBD.create image_opts in
+         QemuNBD.set_image_opts cmd true;
+         cmd
+       ) in
      QemuNBD.set_snapshot cmd false;
-     QemuNBD.set_format cmd (Some "qcow2");
      let _, pid = QemuNBD.run_unix socket cmd in
      On_exit.kill pid
 
diff --git a/output/output.mli b/output/output.mli
index 8d3d686594..c1f0f53d8e 100644
--- a/output/output.mli
+++ b/output/output.mli
@@ -84,6 +84,7 @@ val error_if_disk_count_gt : string -> int -> unit
     called. *)
 
 val output_to_local_file : ?changeuid:((unit -> unit) -> unit) ->
+                           ?compressed:bool ->
                            Types.output_allocation ->
                            string -> string -> int64 -> string ->
                            unit
-- 
2.37.0.rc2

Richard W.M. Jones

2022-Jul-01 11:01 UTC

head link

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

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
@@ -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
-- 
2.37.0.rc2

Richard W.M. Jones

2022-Jul-01 11:01 UTC

head link

[Libguestfs] [PATCH v2v 3/3] tests: Add a simple test of -o local -of qcow2 -oo compressed

This only tests that it doesn't completely fail, which it did before
we fixed nbdcopy.  I checked the file sizes manually and with
compression the resulting file is about half the size.
---
 tests/Makefile.am                          |  2 +
 tests/test-v2v-o-local-qcow2-compressed.sh | 51 ++++++++++++++++++++++
 tests/test-v2v-of-option.sh                |  2 +
 3 files changed, 55 insertions(+)

diff --git a/tests/Makefile.am b/tests/Makefile.am
index ebc433ae5e..fb068624c7 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -83,6 +83,7 @@ TESTS = \
 	test-v2v-networks-and-bridges.sh \
 	test-v2v-o-glance.sh \
 	test-v2v-o-libvirt.sh \
+	test-v2v-o-local-qcow2-compressed.sh \
 	test-v2v-o-null.sh \
 	test-v2v-o-openstack.sh \
 	test-v2v-o-qemu.sh \
@@ -242,6 +243,7 @@ EXTRA_DIST += \
 	test-v2v-networks-and-bridges-expected.xml \
 	test-v2v-o-glance.sh \
 	test-v2v-o-libvirt.sh \
+	test-v2v-o-local-qcow2-compressed.sh \
 	test-v2v-o-null.sh \
 	test-v2v-o-openstack.sh \
 	test-v2v-o-qemu.sh \
diff --git a/tests/test-v2v-o-local-qcow2-compressed.sh
b/tests/test-v2v-o-local-qcow2-compressed.sh
new file mode 100755
index 0000000000..6ebf92c976
--- /dev/null
+++ b/tests/test-v2v-o-local-qcow2-compressed.sh
@@ -0,0 +1,51 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014-2022 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test -o local -of qcow2 -oo compressed.
+
+set -e
+
+source ./functions.sh
+set -e
+set -x
+
+skip_if_skipped
+requires test -f ../test-data/phony-guests/windows.img
+
+# This requires fixed nbdcopy >= 1.13.5.
+requires nbdcopy --version
+version="$( nbdcopy --version | head -1 | awk '{print $2}' )"
+minor="$( echo "$version" | awk -F. '{print $2}' )"
+release="$( echo "$version" | awk -F. '{print $3}'
)"
+requires test $minor -gt 13 -o \( $minor -eq 13 -a $release -ge 5 \)
+
+export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
+
+d=test-v2v-o-local-qcow2-compressed.d
+rm -rf $d
+cleanup_fn rm -rf $d
+mkdir $d
+
+$VG virt-v2v --debug-gc \
+    -i disk ../test-data/phony-guests/windows.img \
+    -o local -of qcow2 -oo compressed -os $d
+
+# Test the libvirt XML metadata and a disk was created.
+ls -l $d
+test -f $d/windows.xml
+test -f $d/windows-sda
diff --git a/tests/test-v2v-of-option.sh b/tests/test-v2v-of-option.sh
index bdfd34180d..6c5f5938c8 100755
--- a/tests/test-v2v-of-option.sh
+++ b/tests/test-v2v-of-option.sh
@@ -42,6 +42,8 @@ $VG virt-v2v --debug-gc \
     -i libvirt -ic "$libvirt_uri" windows \
     -o local -os $d -of qcow2
 
+ls -l $d
+
 # Test the disk is qcow2 format.
 if [ "$(guestfish disk-format $d/windows-sda)" != qcow2 ]; then
     echo "$0: test failed: output is not qcow2"
-- 
2.37.0.rc2

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

[Libguestfs] [PATCH v2v 0/3] Implement -of qcow2 -oo compressed

[Libguestfs] [PATCH v2v 1/3] qemu-nbd: Implement output compression for qcow2 files

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

[Libguestfs] [PATCH v2v 3/3] tests: Add a simple test of -o local -of qcow2 -oo compressed