From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 458071FF0A7 for ; Wed, 30 Sep 2026 09:46:58 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 81F1821670; Wed, 30 Sep 2026 09:46:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dualfroz.com; s=dkim; t=1790689290; h=from:subject:date:message-id:to:mime-version: content-transfer-encoding; bh=k2Bq98l5BjPdlq5ikCWGZPRp2NteDyrWWW+FgCmfCsk=; b=MiSGhxnJr4Y81R9LZMw8z3bkOJ2JfoVF7rznZLY5tXyWtsIBWlkhEsL1s/D4tYuEUHhIb3 eYQkw/QW1d0JChq34M0X7Bc0AxtRtb3Bi+kayNEQwT/Hh2s2FZu/WlD8VxSb4QJG9ETdob wF1AdQyRcn24xzZa8B8uupqfSLguIT7aiAfxNOMunHkEIz7ejNpFiWnYE1OOI/AbF6AFW9 ZKoEQ+0BUOeJxBX+zef3zf+ThwPnZgVBwPxwmGSu522v3T8B4S+UbpEa2RC+mLePabuyO8 2NBT+sxHCSL9DhaEUBE/VGCfelrZATFzon6RsafDlkEAeJnYovTxa7qq8w8e0w== From: Michal Fox To: pve-devel@lists.proxmox.com Subject: [PATCH storage] fix #5998: import: avoid duplicate disk keys with multiple controllers Date: Tue, 29 Sep 2026 15:41:29 +0200 Message-ID: <20260929134129.2585883-1-me@dualfroz.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Last-TLS-Session-Version: TLSv1.3 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.047 Adjusted score from AWL reputation of From: address DKIM_INVALID 0.1 DKIM or DK signature exists, but is not valid DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DMARC_PASS -0.1 DMARC pass policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_SHORT 0.001 Use of a URL Shortener for very short URL SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record X-MailFrom: me@dualfroz.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation Message-ID-Hash: VCCLRLXGBOZX6OTFZT4ZXE4JG3N3TKKV X-Message-ID-Hash: VCCLRLXGBOZX6OTFZT4ZXE4JG3N3TKKV X-Mailman-Approved-At: Wed, 30 Sep 2026 09:46:15 +0200 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 100755 --- 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'); -- 2.43.0