On Tue, Apr 19, 2022 at 05:24:14PM +0200, Laszlo Ersek wrote: > The "s_disk_id" field is supposed to be unique across all fixed disks of > the source domain. > > With "-i vmx -it ssh" however, we restart assigning "s_disk_id" from zero > near the end of the "find_hdds" function. > > The problem is that "find_hdds" is separately called from > "find_scsi_disks", "find_nvme_disks", and "find_ide_disks". Thus, if we > have at least two hard disks in the source on different controller types, > we'll output two disks with a zero-value "s_disk_id". > > "input/input_vmx.ml" starts up nbdkit with "in%d" socket names that are > based on list position, not "s_disk_id", so the nbdkit invocations are > correct. However, in "convert/convert.ml", the "server" argument for > "g#add_drive_opts" is based on "s_disk_id"; therefore, one of the disks > will be multiply referenced, and the others will be ignored. > > Assign "s_disk_id" in "find_disks" [input/parse_domain_from_vmx.ml] once > all the disks have been found and concatenated into the common list. > > Note that the global sorting order remains well-defined. Both before and > after this patch, the sorting occurs *within* "find_hdds", but that's OK: > the concatenation order between the type-specific disk-lists is > well-defined (albeit arbitrary) in "find_disks". > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1883802 > Signed-off-by: Laszlo Ersek <[email protected]> > --- > input/parse_domain_from_vmx.ml | 16 +++++++--------- > 1 file changed, 7 insertions(+), 9 deletions(-) > > diff --git a/input/parse_domain_from_vmx.ml b/input/parse_domain_from_vmx.ml > index b812edeb81e2..5f5946f01a27 100644 > --- a/input/parse_domain_from_vmx.ml > +++ b/input/parse_domain_from_vmx.ml > @@ -102,9 +102,12 @@ let remote_file_exists uri path = > Sys.command cmd = 0 > > let rec find_disks vmx vmx_source = > - find_scsi_disks vmx vmx_source > - @ find_nvme_disks vmx vmx_source > - @ find_ide_disks vmx vmx_source > + (* Set the s_disk_id field to an incrementing number. *) > + List.mapi > + (fun i (source, filename) -> { source with s_disk_id = i }, filename) > + (find_scsi_disks vmx vmx_source @ > + find_nvme_disks vmx vmx_source @ > + find_ide_disks vmx vmx_source) > > (* Find all SCSI hard disks. > * > @@ -209,12 +212,7 @@ and find_hdds vmx vmx_source > * won't return them in any particular order. > *) > let hdds = List.sort compare hdds in > - let hdds = List.map (fun (_, _, source, filename) -> source, filename) > hdds in > - > - (* Set the s_disk_id field to an incrementing number. *) > - let hdds = List.mapi (fun i (source, filename) -> > - { source with s_disk_id = i }, filename) hdds in > - hdds > + List.map (fun (_, _, source, filename) -> source, filename) hdds > > (* Find all removable disks. > * > --
Reviewed-by: Richard W.M. Jones <[email protected]> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW _______________________________________________ Libguestfs mailing list [email protected] https://listman.redhat.com/mailman/listinfo/libguestfs
