thr3ads.net - Libguestfs - [Libguestfs] [virt-v2v PATCH v2 0/3] lib/create_ovf: populate "actual size" attributes again [Dec 2021]

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

Laszlo Ersek

2021-Dec-10 11:35 UTC

[Libguestfs] [virt-v2v PATCH v2 0/3] lib/create_ovf: populate "actual size" attributes again

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598
v1:      
https://listman.redhat.com/archives/libguestfs/2021-December/msg00096.html

In v2, move the new functions to the Utils module, from Nbdkit.

Thanks,
Laszlo

Laszlo Ersek (3):
  lib/utils: add the "Utils.with_nbd_connect_unix" helper function
  lib/utils: add the "Utils.get_disk_allocated" helper function
  lib/create_ovf: populate "actual size" attributes again

 lib/Makefile.am                            |  2 +-
 lib/create_ovf.ml                          | 33 +++++++--------
 lib/create_ovf.mli                         |  2 +-
 lib/utils.ml                               | 42 ++++++++++++++++++++
 lib/utils.mli                              | 18 +++++++++
 output/output_rhv.ml                       |  2 +-
 output/output_rhv_upload.ml                |  2 +-
 output/output_vdsm.ml                      |  1 +
 tests/test-v2v-o-rhv.ovf.expected          |  4 +-
 tests/test-v2v-o-vdsm-options.ovf.expected |  4 +-
 10 files changed, 84 insertions(+), 26 deletions(-)


base-commit: 9bb0e7f1d22936bf2c6c328c5ca48f32c7b9420a
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2021-Dec-10 11:35 UTC

head link

[Libguestfs] [virt-v2v PATCH v2 1/3] lib/utils: add the "Utils.with_nbd_connect_unix" helper function

Connecting to an NBD server temporarily, for a "one-shot" operation,
is
quite similar to "Std_utils.with_open_in" and
"Std_utils.with_open_out",
as there are cleanup operations regardless of whether the "one-shot"
operation completes successfully or throws an exception.

Introduce the "Utils.with_nbd_connect_unix" function, which takes a
Unix
domain socket pathname, a list of metadata contexts to request from the
NBD server, and calls a function with the live NBD server connection.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - move the function to the Utils module, from Nbdkit [Rich]
    
    - rename the function to "with_nbd_connect_unix" (from
      "with_connect_unix")
    
    - update the commit message

 lib/Makefile.am |  2 +-
 lib/utils.mli   | 10 ++++++++++
 lib/utils.ml    | 12 ++++++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index c274b9ecf6c7..1fab25b326f5 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -80,7 +80,7 @@ BOBJECTS = config.cmo $(SOURCES_ML:.ml=.cmo)
 XOBJECTS = $(BOBJECTS:.cmo=.cmx)
 
 OCAMLPACKAGES = \
-	-package str,unix \
+	-package str,unix,nbd \
 	-I $(builddir) \
 	-I $(top_builddir)/common/mlgettext \
 	-I $(top_builddir)/common/mlpcre \
diff --git a/lib/utils.mli b/lib/utils.mli
index c9b4bd631d65..f41e825e747d 100644
--- a/lib/utils.mli
+++ b/lib/utils.mli
@@ -77,3 +77,13 @@ val metaversion : string
 
     Eventually we may switch to using an "open metadata" format
instead
     (eg. XML). *)
+
+val with_nbd_connect_unix : socket:string ->
+                            meta_contexts:string list ->
+                            f:(NBD.t -> 'a) ->
+                            'a
+(** [with_nbd_connect_unix socket meta_contexts f] calls function [f] with the
+    NBD server at Unix domain socket [socket] connected, and the metadata
+    contexts in [meta_contexts] requested (each of which is not necessarily
+    supported by the server though). The connection is torn down either on
+    normal return or if the function [f] throws an exception. *)
diff --git a/lib/utils.ml b/lib/utils.ml
index c18a467cb57c..86967f342e6e 100644
--- a/lib/utils.ml
+++ b/lib/utils.ml
@@ -164,3 +164,15 @@ let rec wait_for_file filename timeout    )
 
 let metaversion = Digest.to_hex (Digest.string Config.package_version_full)
+
+let with_nbd_connect_unix ~socket ~meta_contexts ~f +  let nbd = NBD.create ()
in
+  protect
+    ~f:(fun () ->
+          List.iter (NBD.add_meta_context nbd) meta_contexts;
+          NBD.connect_unix nbd socket;
+          protect
+            ~f:(fun () -> f nbd)
+            ~finally:(fun () -> NBD.shutdown nbd)
+       )
+    ~finally:(fun () -> NBD.close nbd)
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2021-Dec-10 11:35 UTC

head link

[Libguestfs] [virt-v2v PATCH v2 2/3] lib/utils: add the "Utils.get_disk_allocated" helper function

Add the "Utils.get_disk_allocated" helper function, which calculates
the
number of allocated bytes in an output disk image, through a corresponding
NBD server connection.

Original "NBD.block_status" summation example code by Rich Jones
(thanks!), chunking, metadata context simplification, and virt-v2v
integration by myself. Chunking added because "NBD.block_status" is
not
64-bit clean just yet (Eric Blake is working on it).

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - move the function to the Utils module, from Nbdkit [Rich]
    
    - update the commit message
    
    - replace with_connect_unix call with a call to with_nbd_connect_unix

 lib/utils.mli |  8 ++++++
 lib/utils.ml  | 30 ++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/lib/utils.mli b/lib/utils.mli
index f41e825e747d..3096eb146650 100644
--- a/lib/utils.mli
+++ b/lib/utils.mli
@@ -87,3 +87,11 @@ val with_nbd_connect_unix : socket:string ->
     contexts in [meta_contexts] requested (each of which is not necessarily
     supported by the server though). The connection is torn down either on
     normal return or if the function [f] throws an exception. *)
+
+val get_disk_allocated : dir:string -> disknr:int -> int64 option
+(** Callable only in the finalization step. [get_disk_allocated dir disknr]
+    examines output disk [disknr] through the corresponding NBD server socket
+    that resides in [dir]. Returns the number of bytes allocated in the disk
+    image, according to the "base:allocation" metadata context. If
the context
+    is not supported by the NBD server behind the socket, the function returns
+    None. *)
diff --git a/lib/utils.ml b/lib/utils.ml
index 86967f342e6e..d6861d08898f 100644
--- a/lib/utils.ml
+++ b/lib/utils.ml
@@ -176,3 +176,33 @@ let with_nbd_connect_unix ~socket ~meta_contexts ~f        
~finally:(fun () -> NBD.shutdown nbd)
        )
     ~finally:(fun () -> NBD.close nbd)
+
+let get_disk_allocated ~dir ~disknr +  let socket = sprintf
"%s/out%d" dir disknr
+  and alloc_ctx = "base:allocation" in
+  with_nbd_connect_unix ~socket ~meta_contexts:[alloc_ctx]
+    ~f:(fun nbd ->
+         if NBD.can_meta_context nbd alloc_ctx then (
+           (* Get the list of extents, using a 2GiB chunk size as hint. *)
+           let size = NBD.get_size nbd
+           and allocated = ref 0_L
+           and fetch_offset = ref 0_L in
+           while !fetch_offset < size do
+             let remaining = size -^ !fetch_offset in
+             let fetch_size = min 0x8000_0000_L remaining in
+             NBD.block_status nbd fetch_size !fetch_offset
+               (fun ctx offset entries err ->
+                  assert (ctx = alloc_ctx);
+                  for i = 0 to Array.length entries / 2 - 1 do
+                    let len = Int64.of_int32 entries.(i * 2)
+                    and typ = entries.(i * 2 + 1) in
+                    if Int32.logand typ 1_l = 0_l then
+                      allocated := !allocated +^ len;
+                    fetch_offset := !fetch_offset +^ len
+                  done;
+                  0
+               )
+           done;
+           Some !allocated
+         ) else None
+       )
-- 
2.19.1.3.g30247aa5d201

Laszlo Ersek

2021-Dec-10 11:35 UTC

head link

[Libguestfs] [virt-v2v PATCH v2 3/3] lib/create_ovf: populate "actual size" attributes again

Commit 255722cbf39a ("v2v: Modular virt-v2v", 2021-09-07) removed the
following attributes from the OVF output:

- ovf:Envelope/References/File/@ovf:size
-
ovf:Envelope/Section[@xsi:type='ovf:DiskSection_Type']/Disk/@ovf:actual_size

Unfortunately, ovirt-engine considers the second one mandatory; without
it, ovirt-engine refuses to import the OVF.

Restore both attributes, using the utility functions added to the Utils
module previously.

(If we do not have the information necessary to fill in @ovf:actual_size,
we still have to generate the attribute, only with empty contents.
Ovirt-engine does cope with that.)

Fixes: 255722cbf39afc0b012e2ac00d16fa6ba2f8c21f
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2027598
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v2:
    
    - update commit message
    
    - drop "Nbdkit." module selector (as get_disk_allocated is in
Utils now,
      and "lib/create_ovf.ml" has Utils open anyway)

 lib/create_ovf.mli                         |  2 +-
 lib/create_ovf.ml                          | 33 +++++++++-----------
 output/output_rhv.ml                       |  2 +-
 output/output_rhv_upload.ml                |  2 +-
 output/output_vdsm.ml                      |  1 +
 tests/test-v2v-o-rhv.ovf.expected          |  4 +--
 tests/test-v2v-o-vdsm-options.ovf.expected |  4 +--
 7 files changed, 23 insertions(+), 25 deletions(-)

diff --git a/lib/create_ovf.mli b/lib/create_ovf.mli
index 701152d93dd2..0d1cc5a9311a 100644
--- a/lib/create_ovf.mli
+++ b/lib/create_ovf.mli
@@ -46,7 +46,7 @@ val ovf_flavour_to_string : ovf_flavour -> string
 val create_ovf : Types.source -> Types.inspect ->
                  Types.target_meta -> int64 list ->
                  Types.output_allocation -> string -> string -> string
list ->
-                 string list -> string ->  ovf_flavour -> DOM.doc
+                 string list -> string -> string -> ovf_flavour ->
DOM.doc
 (** Create the OVF file.
 
     Actually a {!DOM} document is created, not a file.  It can be written
diff --git a/lib/create_ovf.ml b/lib/create_ovf.ml
index c9f14635363e..dbac3405989b 100644
--- a/lib/create_ovf.ml
+++ b/lib/create_ovf.ml
@@ -531,7 +531,7 @@ let rec create_ovf source inspect
           { output_name; guestcaps; target_firmware; target_nics }
           sizes
           output_alloc output_format
-          sd_uuid image_uuids vol_uuids vm_uuid ovf_flavour +          sd_uuid
image_uuids vol_uuids dir vm_uuid ovf_flavour    assert (List.length sizes =
List.length vol_uuids);
 
   let memsize_mb = source.s_memory /^ 1024L /^ 1024L in
@@ -745,7 +745,7 @@ let rec create_ovf source inspect
 
   (* Add disks to the OVF XML. *)
   add_disks sizes guestcaps output_alloc output_format
-    sd_uuid image_uuids vol_uuids ovf_flavour ovf;
+    sd_uuid image_uuids vol_uuids dir ovf_flavour ovf;
 
   (* Old virt-v2v ignored removable media. XXX *)
 
@@ -791,7 +791,7 @@ and get_flavoured_section ovf ovirt_path rhv_path
rhv_path_attr = function
 
 (* This modifies the OVF DOM, adding a section for each disk. *)
 and add_disks sizes guestcaps output_alloc output_format
-    sd_uuid image_uuids vol_uuids ovf_flavour ovf +    sd_uuid image_uuids
vol_uuids dir ovf_flavour ovf    let references      let nodes = path_to_nodes
ovf ["ovf:Envelope"; "References"] in
     match nodes with
@@ -839,13 +839,7 @@ and add_disks sizes guestcaps output_alloc output_format
         b /^ 1073741824L
       in
       let size_gb = bytes_to_gb size in
-      (* virt-v2v 1.4x used to try to collect the actual size of the
-       * sparse disk.  It would be possible to get this information
-       * accurately now by reading the extent map of the output disk
-       * (this function is called during finalization), but we don't
-       * yet do that. XXX
-       *)
-      let actual_size_gb = None in
+      let actual_size = get_disk_allocated ~dir ~disknr:i in
 
       let format_for_rhv          match output_format with
@@ -867,13 +861,11 @@ and add_disks sizes guestcaps output_alloc output_format
           "ovf:id", vol_uuid;
           "ovf:description", generated_by;
         ] in
-        (* See note above about actual_size_gb
-        (match t.target_overlay.ov_stats.target_actual_size with
+        (match actual_size with
          | None -> ()
          | Some actual_size ->
             List.push_back attrs ("ovf:size", Int64.to_string
actual_size)
         );
-        *)
         e "File" !attrs [] in
       append_child disk references;
 
@@ -899,11 +891,16 @@ and add_disks sizes guestcaps output_alloc output_format
           "ovf:disk-type", "System"; (* RHBZ#744538 *)
           "ovf:boot", if is_bootable_drive then "True" else
"False";
         ] in
-        (match actual_size_gb with
-         | None -> ()
-         | Some actual_size_gb ->
-            List.push_back attrs ("ovf:actual_size", Int64.to_string
actual_size_gb)
-        );
+        (* Ovirt-engine considers the "ovf:actual_size" attribute
mandatory. If
+         * we don't know the actual size, we must create the attribute with
+         * empty contents.
+         *)
+        List.push_back attrs
+          ("ovf:actual_size",
+           match actual_size with
+            | None -> ""
+            | Some actual_size -> Int64.to_string (bytes_to_gb actual_size)
+          );
         e "Disk" !attrs [] in
       append_child disk disk_section;
 
diff --git a/output/output_rhv.ml b/output/output_rhv.ml
index 4550dc4f306f..b902a7ee4619 100644
--- a/output/output_rhv.ml
+++ b/output/output_rhv.ml
@@ -183,7 +183,7 @@ and rhv_finalize dir source inspect target_meta
   (* Create the metadata. *)
   let ovf      Create_ovf.create_ovf source inspect target_meta sizes
-      output_alloc output_format esd_uuid image_uuids vol_uuids vm_uuid
+      output_alloc output_format esd_uuid image_uuids vol_uuids dir vm_uuid
       Create_ovf.RHVExportStorageDomain in
 
   (* Write it to the metadata file. *)
diff --git a/output/output_rhv_upload.ml b/output/output_rhv_upload.ml
index 7099ecefd076..7c77cace5308 100644
--- a/output/output_rhv_upload.ml
+++ b/output/output_rhv_upload.ml
@@ -419,7 +419,7 @@ and rhv_upload_finalize dir source inspect target_meta
   (* Create the metadata. *)
   let ovf      Create_ovf.create_ovf source inspect target_meta disk_sizes
-      Sparse output_format sd_uuid disk_uuids vol_uuids vm_uuid
+      Sparse output_format sd_uuid disk_uuids vol_uuids dir vm_uuid
       OVirt in
   let ovf = DOM.doc_to_string ovf in
 
diff --git a/output/output_vdsm.ml b/output/output_vdsm.ml
index c9b552805ccd..ce0d5b5e4b2a 100644
--- a/output/output_vdsm.ml
+++ b/output/output_vdsm.ml
@@ -201,6 +201,7 @@ and vdsm_finalize dir source inspect target_meta
               output_alloc output_format dd_uuid
               image_uuids
               vol_uuids
+              dir
               vm_uuid
               ovf_flavour in
 
diff --git a/tests/test-v2v-o-rhv.ovf.expected
b/tests/test-v2v-o-rhv.ovf.expected
index 5fda41c9488e..25e492fdbabc 100644
--- a/tests/test-v2v-o-rhv.ovf.expected
+++ b/tests/test-v2v-o-rhv.ovf.expected
@@ -2,7 +2,7 @@
 <ovf:Envelope
xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'
xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData'
xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/'
xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'>
   <!-- generated by virt-v2v -->
   <References>
-    <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#'
ovf:description='generated by virt-v2v'/>
+    <File ovf:href='#DISK_ID#/#VOL_ID#' ovf:id='#VOL_ID#'
ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/>
   </References>
   <Section xsi:type='ovf:NetworkSection_Type'>
     <Info>List of networks</Info>
@@ -10,7 +10,7 @@
   </Section>
   <Section xsi:type='ovf:DiskSection_Type'>
     <Info>List of Virtual Disks</Info>
-    <Disk ovf:diskId='#VOL_ID#' ovf:size='1'
ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#'
ovf:parentRef='' ovf:vm_snapshot_id='#UUID#'
ovf:volume-format='RAW' ovf:volume-type='Sparse'
ovf:format='http://en.wikipedia.org/wiki/Byte'
ovf:disk-interface='VirtIO' ovf:disk-type='System'
ovf:boot='True'/>
+    <Disk ovf:diskId='#VOL_ID#' ovf:size='1'
ovf:capacity='536870912' ovf:fileRef='#DISK_ID#/#VOL_ID#'
ovf:parentRef='' ovf:vm_snapshot_id='#UUID#'
ovf:volume-format='RAW' ovf:volume-type='Sparse'
ovf:format='http://en.wikipedia.org/wiki/Byte'
ovf:disk-interface='VirtIO' ovf:disk-type='System'
ovf:boot='True' ovf:actual_size='1'/>
   </Section>
   <Content ovf:id='out'
xsi:type='ovf:VirtualSystem_Type'>
     <Name>windows</Name>
diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected
b/tests/test-v2v-o-vdsm-options.ovf.expected
index 23ca180f4c2f..bd5b5e7d38ec 100644
--- a/tests/test-v2v-o-vdsm-options.ovf.expected
+++ b/tests/test-v2v-o-vdsm-options.ovf.expected
@@ -2,7 +2,7 @@
 <ovf:Envelope
xmlns:rasd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData'
xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData'
xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
xmlns:ovf='http://schemas.dmtf.org/ovf/envelope/1/'
xmlns:ovirt='http://www.ovirt.org/ovf' ovf:version='0.9'>
   <!-- generated by virt-v2v -->
   <References>
-    <File ovf:href='VOL' ovf:id='VOL'
ovf:description='generated by virt-v2v'/>
+    <File ovf:href='VOL' ovf:id='VOL'
ovf:description='generated by virt-v2v' ovf:size='#SIZE#'/>
   </References>
   <NetworkSection>
     <Info>List of networks</Info>
@@ -10,7 +10,7 @@
   </NetworkSection>
   <DiskSection>
     <Info>List of Virtual Disks</Info>
-    <Disk ovf:diskId='IMAGE' ovf:size='1'
ovf:capacity='536870912' ovf:fileRef='VOL'
ovf:parentRef='' ovf:vm_snapshot_id='#UUID#'
ovf:volume-format='COW' ovf:volume-type='Sparse'
ovf:format='http://en.wikipedia.org/wiki/Byte'
ovf:disk-interface='VirtIO' ovf:disk-type='System'
ovf:boot='True'/>
+    <Disk ovf:diskId='IMAGE' ovf:size='1'
ovf:capacity='536870912' ovf:fileRef='VOL'
ovf:parentRef='' ovf:vm_snapshot_id='#UUID#'
ovf:volume-format='COW' ovf:volume-type='Sparse'
ovf:format='http://en.wikipedia.org/wiki/Byte'
ovf:disk-interface='VirtIO' ovf:disk-type='System'
ovf:boot='True' ovf:actual_size='1'/>
   </DiskSection>
   <VirtualSystem ovf:id='VM'>
     <Name>windows</Name>
-- 
2.19.1.3.g30247aa5d201

Libguestfs - Dec 2021 - [virt-v2v PATCH v2 0/3] lib/create_ovf: populate "actual size" attributes again

[Libguestfs] [virt-v2v PATCH v2 0/3] lib/create_ovf: populate "actual size" attributes again

[Libguestfs] [virt-v2v PATCH v2 1/3] lib/utils: add the "Utils.with_nbd_connect_unix" helper function

[Libguestfs] [virt-v2v PATCH v2 2/3] lib/utils: add the "Utils.get_disk_allocated" helper function

[Libguestfs] [virt-v2v PATCH v2 3/3] lib/create_ovf: populate "actual size" attributes again