[Libguestfs] [v2v PATCH] output_rhv: restrict block status collection to the old RHV output
Nir Soffer
nsoffer at redhat.com
Mon Dec 20 16:20:49 UTC 2021
On Mon, Dec 20, 2021 at 3:56 PM Laszlo Ersek <lersek at redhat.com> wrote:
>
> Nir reports that, despite the comment we removed in commit a2a4f7a09996,
> we generally cannot access the output NBD servers in the finalization
> stage. In particular, in the "rhv_upload_finalize" function
> [output/output_rhv_upload.ml], the output disks are disconnected before
> "create_ovf" is called.
>
> Consequently, the "get_disk_allocated" call in the "create_ovf" ->
> "add_disks" -> "get_disk_allocated" chain fails.
>
> Rich suggests that we explicitly restrict the "get_disk_allocated" call
> with a new optional boolean parameter to the one output plugin that really
> needs it, namely the old RHV one.
>
> Accordingly, revert the VDSM test case to its state at (a2a4f7a09996^).
>
> Cc: Nir Soffer <nsoffer at redhat.com>
> Fixes: a2a4f7a09996a5e66d027d0d9692e083eb0a8128
> Reported-by: Nir Soffer <nsoffer at redhat.com>
> Suggested-by: Richard W.M. Jones <rjones at redhat.com>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2034240
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> lib/create_ovf.mli | 3 +-
> lib/create_ovf.ml | 35 +++++++++++++---------
> output/output_rhv.ml | 4 +--
> tests/test-v2v-o-vdsm-options.ovf.expected | 4 +--
> 4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/lib/create_ovf.mli b/lib/create_ovf.mli
> index 0d1cc5a9311a..d6d4e62eeb86 100644
> --- a/lib/create_ovf.mli
> +++ b/lib/create_ovf.mli
> @@ -46,7 +46,8 @@ 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 -> string -> ovf_flavour -> DOM.doc
> + string list -> ?need_actual_sizes:bool -> 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 dbac3405989b..678d29942abe 100644
> --- a/lib/create_ovf.ml
> +++ b/lib/create_ovf.ml
> @@ -531,7 +531,8 @@ let rec create_ovf source inspect
> { output_name; guestcaps; target_firmware; target_nics }
> sizes
> output_alloc output_format
> - sd_uuid image_uuids vol_uuids dir vm_uuid ovf_flavour =
> + sd_uuid image_uuids vol_uuids ?(need_actual_sizes = false) dir
> + vm_uuid ovf_flavour =
> assert (List.length sizes = List.length vol_uuids);
>
> let memsize_mb = source.s_memory /^ 1024L /^ 1024L in
> @@ -745,7 +746,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 dir ovf_flavour ovf;
> + sd_uuid image_uuids vol_uuids need_actual_sizes dir ovf_flavour ovf;
>
> (* Old virt-v2v ignored removable media. XXX *)
>
> @@ -791,7 +792,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 dir ovf_flavour ovf =
> + sd_uuid image_uuids vol_uuids need_actual_sizes dir ovf_flavour ovf =
> let references =
> let nodes = path_to_nodes ovf ["ovf:Envelope"; "References"] in
> match nodes with
> @@ -839,7 +840,12 @@ and add_disks sizes guestcaps output_alloc output_format
> b /^ 1073741824L
> in
> let size_gb = bytes_to_gb size in
> - let actual_size = get_disk_allocated ~dir ~disknr:i in
> + let actual_size =
> + if need_actual_sizes then
> + get_disk_allocated ~dir ~disknr:i
> + else
> + None
> + in
>
> let format_for_rhv =
> match output_format with
> @@ -891,16 +897,17 @@ 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
> - (* 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)
> - );
> + if (need_actual_sizes) then
> + (* 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 b902a7ee4619..6a67b7aa152b 100644
> --- a/output/output_rhv.ml
> +++ b/output/output_rhv.ml
> @@ -183,8 +183,8 @@ 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 dir vm_uuid
> - Create_ovf.RHVExportStorageDomain in
> + output_alloc output_format esd_uuid image_uuids vol_uuids
> + ~need_actual_sizes:true dir vm_uuid Create_ovf.RHVExportStorageDomain in
>
> (* Write it to the metadata file. *)
> let dir = esd_mp // esd_uuid // "master" // "vms" // vm_uuid in
> diff --git a/tests/test-v2v-o-vdsm-options.ovf.expected b/tests/test-v2v-o-vdsm-options.ovf.expected
> index bd5b5e7d38ec..23ca180f4c2f 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' ovf:size='#SIZE#'/>
> + <File ovf:href='VOL' ovf:id='VOL' ovf:description='generated by virt-v2v'/>
> </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' ovf:actual_size='1'/>
> + <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'/>
> </DiskSection>
> <VirtualSystem ovf:id='VM'>
> <Name>windows</Name>
> --
> 2.19.1.3.g30247aa5d201
>
I don't fully understand the ocaml part, but the intent looks good,
and the patch works for me.
I tested it on rhel 8.6 host with upstream nbdkit, libnbd, and ovirt,
virt-v2v master with your patch applied, and
https://listman.redhat.com/archives/libguestfs/2021-December/msg00187.html
Without
https://listman.redhat.com/archives/libguestfs/2021-December/msg00189.html
Tested using:
$ cat v2v/v2v.sh
#!/bin/sh
vm_name=${1:-v2v}
shift
./run virt-v2v \
-i disk /var/tmp/fedora-32.raw \
-o rhv-upload \
-oc https://engine-dev/ovirt-engine/api \
-op ~/.config/ovirt/engine-dev/password \
-on $vm_name \
-os alpine-nfs-00 \
-of qcow2 \
-oo rhv-cafile=/etc/pki/vdsm/certs/cacert.pem \
-oo rhv-cluster=el8 \
-oo rhv-direct=true \
"$@"
$ ~/v2v/v2v.sh
[ 12.7] Opening the source
[ 16.6] Inspecting the source
[ 40.9] Checking for sufficient free disk space in the guest
[ 40.9] Converting Fedora 32 (Thirty Two) to run on KVM
virt-v2v: warning: /files/boot/grub2/device.map/hd0 references unknown
device "vda". You may have to fix this entry manually after conversion.
virt-v2v: This guest has virtio drivers installed.
[ 63.6] Mapping filesystem data to avoid copying unused and blank areas
[ 64.6] Closing the overlay
[ 64.7] Assigning disks to buses
[ 64.7] Checking if the guest needs BIOS or UEFI to boot
[ 64.7] Copying disk 1/1
█ 100% [****************************************]
[ 68.7] Creating output metadata
[ 79.0] Finishing off
You can add
Tested-by: Nir Soffer <nsoffer at redhat.com>
Nir
More information about the Libguestfs
mailing list