public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH storage] fix #5998: import: avoid duplicate disk keys with multiple controllers
@ 2026-09-29 13:41 Michal Fox
  0 siblings, 0 replies; only message in thread
From: Michal Fox @ 2026-09-29 13:41 UTC (permalink / raw)
  To: pve-devel

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 <me@dualfroz.com>
---
 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 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Envelope vmw:buildId="build-2982904" xmlns="http://schemas.dmtf.org/ovf/envelope/1" xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common" xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1" xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData" xmlns:vmw="http://www.vmware.com/schema/ovf" xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+  <References>
+    <File ovf:href="disk1.vmdk" ovf:id="file1" ovf:size="3481968640"/>
+    <File ovf:href="disk2.vmdk" ovf:id="file2" ovf:size="68096"/>
+    <File ovf:href="Win10-Liz-disk1.vmdk" ovf:id="file3" ovf:size="68096"/>
+  </References>
+  <DiskSection>
+    <Info>Virtual disk information</Info>
+    <Disk ovf:capacity="40" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk1" ovf:fileRef="file1" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="7684882432"/>
+    <Disk ovf:capacity="1" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk2" ovf:fileRef="file2" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="0"/>
+    <Disk ovf:capacity="1" ovf:capacityAllocationUnits="byte * 2^30" ovf:diskId="vmdisk3" ovf:fileRef="file3" ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized" ovf:populatedSize="0"/>
+  </DiskSection>
+  <NetworkSection>
+    <Info>The list of logical networks</Info>
+    <Network ovf:name="bridged">
+      <Description>The bridged network</Description>
+    </Network>
+  </NetworkSection>
+  <VirtualSystem ovf:id="vm">
+    <Info>A virtual machine</Info>
+    <Name>two-scsi-controllers</Name>
+    <OperatingSystemSection ovf:id="103" vmw:osType="windows7Server64Guest">
+      <Info>The kind of installed guest operating system</Info>
+    </OperatingSystemSection>
+    <VirtualHardwareSection>
+      <Info>Virtual hardware requirements</Info>
+      <System>
+        <vssd:ElementName>Virtual Hardware Family</vssd:ElementName>
+        <vssd:InstanceID>0</vssd:InstanceID>
+        <vssd:VirtualSystemIdentifier>Win_2008-R2x64</vssd:VirtualSystemIdentifier>
+        <vssd:VirtualSystemType>vmx-11</vssd:VirtualSystemType>
+      </System>
+      <Item>
+        <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
+        <rasd:Description>Number of Virtual CPUs</rasd:Description>
+        <rasd:ElementName>1 virtual CPU(s)</rasd:ElementName>
+        <rasd:InstanceID>1</rasd:InstanceID>
+        <rasd:ResourceType>3</rasd:ResourceType>
+        <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
+      </Item>
+      <Item>
+        <rasd:AllocationUnits>byte * 2^20</rasd:AllocationUnits>
+        <rasd:Description>Memory Size</rasd:Description>
+        <rasd:ElementName>2048MB of memory</rasd:ElementName>
+        <rasd:InstanceID>2</rasd:InstanceID>
+        <rasd:ResourceType>4</rasd:ResourceType>
+        <rasd:VirtualQuantity>2048</rasd:VirtualQuantity>
+      </Item>
+      <Item>
+        <rasd:Address>0</rasd:Address>
+        <rasd:Description>SATA Controller</rasd:Description>
+        <rasd:ElementName>sataController0</rasd:ElementName>
+        <rasd:InstanceID>3</rasd:InstanceID>
+        <rasd:ResourceSubType>vmware.sata.ahci</rasd:ResourceSubType>
+        <rasd:ResourceType>20</rasd:ResourceType>
+      </Item>
+      <Item ovf:required="false">
+        <rasd:Address>0</rasd:Address>
+        <rasd:Description>USB Controller (EHCI)</rasd:Description>
+        <rasd:ElementName>usb</rasd:ElementName>
+        <rasd:InstanceID>4</rasd:InstanceID>
+        <rasd:ResourceSubType>vmware.usb.ehci</rasd:ResourceSubType>
+        <rasd:ResourceType>23</rasd:ResourceType>
+        <vmw:Config ovf:required="false" vmw:key="ehciEnabled" vmw:value="true"/>
+      </Item>
+      <Item>
+        <rasd:Address>0</rasd:Address>
+        <rasd:Description>SCSI Controller</rasd:Description>
+        <rasd:ElementName>scsiController0</rasd:ElementName>
+        <rasd:InstanceID>5</rasd:InstanceID>
+        <rasd:ResourceSubType>lsilogicsas</rasd:ResourceSubType>
+        <rasd:ResourceType>6</rasd:ResourceType>
+      </Item>
+      <Item>
+        <rasd:Address>1</rasd:Address>
+        <rasd:Description>SCSI Controller</rasd:Description>
+        <rasd:ElementName>scsiController1</rasd:ElementName>
+        <rasd:InstanceID>6</rasd:InstanceID>
+        <rasd:ResourceSubType>lsilogicsas</rasd:ResourceSubType>
+        <rasd:ResourceType>6</rasd:ResourceType>
+      </Item>
+      <Item>
+        <rasd:AddressOnParent>0</rasd:AddressOnParent>
+        <rasd:ElementName>disk0</rasd:ElementName>
+        <rasd:HostResource>ovf:/disk/vmdisk1</rasd:HostResource>
+        <rasd:InstanceID>7</rasd:InstanceID>
+        <rasd:Parent>5</rasd:Parent>
+        <rasd:ResourceType>17</rasd:ResourceType>
+      </Item>
+      <Item>
+        <rasd:AddressOnParent>0</rasd:AddressOnParent>
+        <rasd:ElementName>disk2</rasd:ElementName>
+        <rasd:HostResource>ovf:/disk/vmdisk3</rasd:HostResource>
+        <rasd:InstanceID>14</rasd:InstanceID>
+        <rasd:Parent>6</rasd:Parent>
+        <rasd:ResourceType>17</rasd:ResourceType>
+      </Item>
+      <Item>
+        <rasd:AddressOnParent>1</rasd:AddressOnParent>
+        <rasd:ElementName>disk1</rasd:ElementName>
+        <rasd:HostResource>ovf:/disk/vmdisk2</rasd:HostResource>
+        <rasd:InstanceID>8</rasd:InstanceID>
+        <rasd:Parent>5</rasd:Parent>
+        <rasd:ResourceType>17</rasd:ResourceType>
+      </Item>
+      <Item>
+        <rasd:AddressOnParent>2</rasd:AddressOnParent>
+        <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
+        <rasd:Connection>bridged</rasd:Connection>
+        <rasd:Description>E1000 ethernet adapter on &quot;bridged&quot;</rasd:Description>
+        <rasd:ElementName>ethernet0</rasd:ElementName>
+        <rasd:InstanceID>9</rasd:InstanceID>
+        <rasd:ResourceSubType>E1000</rasd:ResourceSubType>
+        <rasd:ResourceType>10</rasd:ResourceType>
+        <vmw:Config ovf:required="false" vmw:key="wakeOnLanEnabled" vmw:value="false"/>
+      </Item>
+      <Item ovf:required="false">
+        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+        <rasd:ElementName>sound</rasd:ElementName>
+        <rasd:InstanceID>10</rasd:InstanceID>
+        <rasd:ResourceSubType>vmware.soundcard.hdaudio</rasd:ResourceSubType>
+        <rasd:ResourceType>1</rasd:ResourceType>
+      </Item>
+      <Item ovf:required="false">
+        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+        <rasd:ElementName>video</rasd:ElementName>
+        <rasd:InstanceID>11</rasd:InstanceID>
+        <rasd:ResourceType>24</rasd:ResourceType>
+        <vmw:Config ovf:required="false" vmw:key="enable3DSupport" vmw:value="true"/>
+      </Item>
+      <Item ovf:required="false">
+        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+        <rasd:ElementName>vmci</rasd:ElementName>
+        <rasd:InstanceID>12</rasd:InstanceID>
+        <rasd:ResourceSubType>vmware.vmci</rasd:ResourceSubType>
+        <rasd:ResourceType>1</rasd:ResourceType>
+      </Item>
+      <Item ovf:required="false">
+        <rasd:AddressOnParent>1</rasd:AddressOnParent>
+        <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+        <rasd:ElementName>cdrom0</rasd:ElementName>
+        <rasd:InstanceID>13</rasd:InstanceID>
+        <rasd:Parent>3</rasd:Parent>
+        <rasd:ResourceType>15</rasd:ResourceType>
+      </Item>
+      <vmw:Config ovf:required="false" vmw:key="cpuHotAddEnabled" vmw:value="true"/>
+      <vmw:Config ovf:required="false" vmw:key="memoryHotAddEnabled" vmw:value="true"/>
+      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.powerOffType" vmw:value="soft"/>
+      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
+      <vmw:Config ovf:required="false" vmw:key="powerOpInfo.suspendType" vmw:value="soft"/>
+      <vmw:Config ovf:required="false" vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
+    </VirtualHardwareSection>
+  </VirtualSystem>
+</Envelope>
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




^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-09-30  7:46 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-29 13:41 [PATCH storage] fix #5998: import: avoid duplicate disk keys with multiple controllers Michal Fox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal