From 9f20cd4f752be8216556227f35a0675ae3715acc Mon Sep 17 00:00:00 2001 From: Michal Fox Date: Fri, 25 Sep 2026 11:38:09 +0200 Subject: [PATCH] fix #5998: import: avoid duplicate disk keys with multiple controllers The disk key is built from the controller type and the address of the disk on that controller. The address is only unique per controller, so with more than one controller of the same type, like a VM with more than 16 disks spread over two SCSI controllers, several disks got the same key. 'qm importovf' then failed with "cowardly refusing to overwrite existing entry: scsi0", and the import wizard silently dropped all but one of those disks, as they are stored in a hash by key. Keep the address for the first disk using it and move any further disk with the same address to the lowest address of that bus that is not used by any other disk. Add a test manifest with a second SCSI controller. Signed-off-by: Michal Fox --- src/PVE/GuestImport/OVF.pm | 18 ++ .../ovf_manifests/two-scsi-controllers.ovf | 155 ++++++++++++++++++ src/test/run_ovf_tests.pl | 26 +++ 3 files changed, 199 insertions(+) create mode 100644 src/test/ovf_manifests/two-scsi-controllers.ovf diff --git a/src/PVE/GuestImport/OVF.pm b/src/PVE/GuestImport/OVF.pm index a228cd2..70df604 100644 --- a/src/PVE/GuestImport/OVF.pm +++ b/src/PVE/GuestImport/OVF.pm @@ -369,6 +369,24 @@ ovf:Item[rasd:InstanceID='%s']/rasd:ResourceType", $controller_id, }; } + # the address is only unique per controller, so disks on different controllers of the same + # type can end up with the same address, move those to the next free one of that bus + my $used_addresses = { map { $_->{disk_address} => 1 } @disks }; + my $assigned_addresses = {}; + for my $disk (@disks) { + if ( + $assigned_addresses->{ $disk->{disk_address} } + && $disk->{disk_address} =~ m/^([a-z]+)\d+$/ + ) { + my $bus = $1; + my $index = 0; + $index++ while $used_addresses->{"$bus$index"}; + $disk->{disk_address} = "$bus$index"; + $used_addresses->{ $disk->{disk_address} } = 1; + } + $assigned_addresses->{ $disk->{disk_address} } = 1; + } + my $nic_id = dtmf_name_to_id('Ethernet Adapter'); my $xpath_find_nics = "/ovf:Envelope/ovf:VirtualSystem/ovf:VirtualHardwareSection/ovf:Item[rasd:ResourceType=${nic_id}]"; diff --git a/src/test/ovf_manifests/two-scsi-controllers.ovf b/src/test/ovf_manifests/two-scsi-controllers.ovf new file mode 100644 index 0000000..263e99c --- /dev/null +++ b/src/test/ovf_manifests/two-scsi-controllers.ovf @@ -0,0 +1,155 @@ + + + + + + + + + Virtual disk information + + + + + + The list of logical networks + + The bridged network + + + + A virtual machine + two-scsi-controllers + + The kind of installed guest operating system + + + Virtual hardware requirements + + Virtual Hardware Family + 0 + Win_2008-R2x64 + vmx-11 + + + hertz * 10^6 + Number of Virtual CPUs + 1 virtual CPU(s) + 1 + 3 + 1 + + + byte * 2^20 + Memory Size + 2048MB of memory + 2 + 4 + 2048 + + + 0 + SATA Controller + sataController0 + 3 + vmware.sata.ahci + 20 + + + 0 + USB Controller (EHCI) + usb + 4 + vmware.usb.ehci + 23 + + + + 0 + SCSI Controller + scsiController0 + 5 + lsilogicsas + 6 + + + 1 + SCSI Controller + scsiController1 + 6 + lsilogicsas + 6 + + + 0 + disk0 + ovf:/disk/vmdisk1 + 7 + 5 + 17 + + + 0 + disk2 + ovf:/disk/vmdisk3 + 14 + 6 + 17 + + + 1 + disk1 + ovf:/disk/vmdisk2 + 8 + 5 + 17 + + + 2 + true + bridged + E1000 ethernet adapter on "bridged" + ethernet0 + 9 + E1000 + 10 + + + + false + sound + 10 + vmware.soundcard.hdaudio + 1 + + + false + video + 11 + 24 + + + + false + vmci + 12 + vmware.vmci + 1 + + + 1 + false + cdrom0 + 13 + 3 + 15 + + + + + + + + + + diff --git a/src/test/run_ovf_tests.pl b/src/test/run_ovf_tests.pl index 23f0aa6..8d89bc1 100644 --- a/src/test/run_ovf_tests.pl +++ b/src/test/run_ovf_tests.pl @@ -38,6 +38,15 @@ if (my $err = $@) { ok('parse win10 no default rasd NS'); } +my $two_controllers = + eval { PVE::GuestImport::OVF::parse_ovf("$test_manifests/two-scsi-controllers.ovf") }; +if (my $err = $@) { + fail('parse two scsi controllers'); + warn("error: $err\n"); +} else { + ok('parse two scsi controllers'); +} + my $xee = eval { PVE::GuestImport::OVF::parse_ovf("$test_manifests/XEE.ovf") }; if (my $err = $@) { ok("parsing XEE.ovf failed as expected"); @@ -95,6 +104,17 @@ is( 'single disk vm (no default rasd NS) has the correct size', ); +is_deeply( + [map { $_->{disk_address} } $two_controllers->{disks}->@*], + ['scsi0', 'scsi2', 'scsi1'], + 'disks with the same address on different controllers get unique addresses', +); +is( + $two_controllers->{disks}->[1]->{backing_file}, + "$test_manifests/Win10-Liz-disk1.vmdk", + 'disk on the second controller has the correct backing device', +); + print "testing nics\n"; is($win2008->{net}->{net0}->{model}, 'e1000', 'win2008 has correct nic model'); is($win10->{net}->{net0}->{model}, 'e1000e', 'win10 has correct nic model'); @@ -109,6 +129,12 @@ is($win2008->{qm}->{memory}, '2048', 'win2008 VM memory is correct'); is($win2008->{qm}->{cores}, '1', 'win2008 VM cores are correct'); is($win2008->{qm}->{ostype}, 'win7', 'win2008 VM ostype is correcty'); +is( + $two_controllers->{qm}->{boot}, + 'order=scsi0;scsi2;scsi1', + 'VM with two scsi controllers boot is correct', +); + is($win10->{qm}->{boot}, 'order=scsi0', 'win10 VM boot is correct'); is($win10->{qm}->{name}, 'Win10-Liz', 'win10 VM name is correct'); is($win10->{qm}->{memory}, '6144', 'win10 VM memory is correct');