Skip to content

Commit

Permalink
output_rhv: restrict block status collection to the old RHV output
Browse files Browse the repository at this point in the history
Nir reports that, despite the comment we removed in commit a2a4f7a,
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 (a2a4f7a^).

Cc: Nir Soffer <nsoffer@redhat.com>
Fixes: a2a4f7a
Reported-by: Nir Soffer <nsoffer@redhat.com>
Suggested-by: Richard W.M. Jones <rjones@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2034240
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20211220135640.12436-1-lersek@redhat.com>
Tested-by: Nir Soffer <nsoffer@redhat.com>
[lersek@redhat.com: drop parens around condition in "if"; thanks: Rich]
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
  • Loading branch information
lersek committed Dec 22, 2021
1 parent 702a511 commit 07b12fe
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 19 deletions.
35 changes: 21 additions & 14 deletions lib/create_ovf.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 *)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

Expand Down
3 changes: 2 additions & 1 deletion lib/create_ovf.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions output/output_rhv.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/test-v2v-o-vdsm-options.ovf.expected
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
<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>
<Network ovf:name='default'/>
</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>
Expand Down

0 comments on commit 07b12fe

Please sign in to comment.