[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
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@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
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@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
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@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_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' 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_ResourceAllocationSett...' xmlns:vssd='http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingDa...' 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
Acked-by: Richard W.M. Jones <rjones@redhat.com> Series looks great now, thanks. 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
On 12/10/21 12:56, Richard W.M. Jones wrote:
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Series looks great now, thanks.
Thank you very much! Series merged as commit range 9bb0e7f1d229..a2a4f7a09996. Thanks! Laszlo
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones