On Fri, Jul 26, 2024 at 02:39:46PM +0200, Adam Julis wrote:
Disk targets are generated in virVMXParseConfig() with virVMXGenerateDiskTarget(). It works on combination of controller, fix offset, unit and prefix. While SCSI and SATA have same prefix "sd", function virVMXGenerateDiskTarget() could returned in some cases same targets. In this patch, after loaded SCSI and SATA disks to the def, indexes are regenerated, now simply from position of the disk in array of disks (def). With this, required uniqueness is guaranteed. Because assigned addresses of disks are generated from their indexes, for every changed SATA disk is called virDomainDiskDefAssignAddress() with the updated value. The corresponding tests have been modified to match the index changes. Signed-off-by: Adam Julis <ajulis@xxxxxxxxxx> --- Since previous version in mailing list was complicated for trying to preserve the indexes of SCSI and previous tests, this one going to straightforward, although it changes all (SCSI and SATA) indexes. It's not a bug, since we cannot guarantee the same naming inside the guest anyway.
I know I suggested this, but looking at the last test case (scsi-driver vmx2xml) made me a bit nervous. The controllers will be added, but unused and the disks are going to get plugged elsewhere. It _probably_ won't be an issue, since it does not change after re-parsing the XML (apart of other things that do and we don't account for them in the VMWare driver, let's just say it has issues) I think that's fine. I would still like to also pull some information from the biggest user of our VMWare driver(s) - virt-v2v. @Rich, is it OK if we don't report the proper address of the disks? Right now we can generate wrong targets (multiple disks being declared as "sda", albeit only in the domain XML) which would subsequently not parse properly. Is this fix OK for your use cases?
src/vmx/vmx.c | 19 +++++++++++++++++++
tests/vmx2xmldata/esx-in-the-wild-11.xml | 4 ++--
tests/vmx2xmldata/esx-in-the-wild-12.xml | 4 ++--
tests/vmx2xmldata/esx-in-the-wild-2.xml | 4 ++--
tests/vmx2xmldata/esx-in-the-wild-8.xml | 4 ++--
tests/vmx2xmldata/scsi-driver.xml | 12 ++++++------
6 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 227744d062..22e59726c8 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1400,6 +1400,7 @@ virVMXParseConfig(virVMXContext *ctx,
virCPUDef *cpu = NULL;
char *firmware = NULL;
size_t saved_ndisks = 0;
+ size_t i;
if (ctx->parseFileName == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -1805,6 +1806,24 @@ virVMXParseConfig(virVMXContext *ctx,
}
}
+ /* now disks contain only SCSI and SATA, SATA could have same name (dst) as SCSI
+ * so replace all their names with new ones to guarantee their uniqueness
+ * finally, regenerate correct addresses, while it depends on the index */
+
+ for (i = 0; i < def->ndisks; i++) {
+ virDomainDiskDef *disk = def->disks[i];
+
+ VIR_FREE(disk->dst);
+ disk->dst = virIndexToDiskName(i, "sd");
+
+ if (virDomainDiskDefAssignAddress(NULL, disk, def) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not assign address to disk '%1$s'"),
+ virDomainDiskGetSource(disk));
+ goto cleanup;
+ }
+ }
+
/* def:disks (ide) */
for (bus = 0; bus < 2; ++bus) {
for (unit = 0; unit < 2; ++unit) {
diff --git a/tests/vmx2xmldata/esx-in-the-wild-11.xml b/tests/vmx2xmldata/esx-in-the-wild-11.xml
index 8807a057d7..d9522c1be2 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-11.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-11.xml
@@ -22,8 +22,8 @@
</disk>
<disk type='file' device='disk'>
<source file='[datastore] directory/esx6.7-rhel7.7-x86_64_3.vmdk'/>
- <target dev='sdp' bus='scsi'/>
- <address type='drive' controller='0' bus='0' target='0' unit='16'/>
+ <target dev='sdb' bus='scsi'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
<controller type='scsi' index='0' model='vmpvscsi'/>
<interface type='bridge'>
diff --git a/tests/vmx2xmldata/esx-in-the-wild-12.xml b/tests/vmx2xmldata/esx-in-the-wild-12.xml
index 42184501d0..a7730845ee 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-12.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-12.xml
@@ -21,9 +21,9 @@
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<disk type='file' device='cdrom'>
- <target dev='sda' bus='sata'/>
+ <target dev='sdb' bus='sata'/>
<readonly/>
- <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
<controller type='scsi' index='0' model='vmpvscsi'/>
<controller type='sata' index='0'/>
diff --git a/tests/vmx2xmldata/esx-in-the-wild-2.xml b/tests/vmx2xmldata/esx-in-the-wild-2.xml
index 59071b5d3a..1a66f5e9c7 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-2.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-2.xml
@@ -20,9 +20,9 @@
</disk>
<disk type='file' device='cdrom'>
<source file='[datastore] directory/Debian1-cdrom.iso'/>
- <target dev='sdp' bus='scsi'/>
+ <target dev='sdb' bus='scsi'/>
<readonly/>
- <address type='drive' controller='1' bus='0' target='0' unit='0'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
<disk type='file' device='cdrom'>
<source file='/vmimages/tools-isoimages/linux.iso'/>
diff --git a/tests/vmx2xmldata/esx-in-the-wild-8.xml b/tests/vmx2xmldata/esx-in-the-wild-8.xml
index 47d22ced2a..d5356bda34 100644
--- a/tests/vmx2xmldata/esx-in-the-wild-8.xml
+++ b/tests/vmx2xmldata/esx-in-the-wild-8.xml
@@ -36,9 +36,9 @@
</disk>
<disk type='file' device='cdrom'>
<source file='[692eb778-2d4937fe] CentOS-4.7.ServerCD-x86_64.iso'/>
- <target dev='sda' bus='sata'/>
+ <target dev='sdd' bus='sata'/>
<readonly/>
- <address type='drive' controller='0' bus='0' target='0' unit='0'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='3'/>
</disk>
<controller type='scsi' index='0' model='vmpvscsi'/>
<controller type='sata' index='0'/>
diff --git a/tests/vmx2xmldata/scsi-driver.xml b/tests/vmx2xmldata/scsi-driver.xml
index e5b73420c3..42b6fffe24 100644
--- a/tests/vmx2xmldata/scsi-driver.xml
+++ b/tests/vmx2xmldata/scsi-driver.xml
@@ -19,18 +19,18 @@
</disk>
<disk type='file' device='disk'>
<source file='[datastore] directory/harddisk2.vmdk'/>
- <target dev='sdp' bus='scsi'/>
- <address type='drive' controller='1' bus='0' target='0' unit='0'/>
+ <target dev='sdb' bus='scsi'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
<disk type='file' device='disk'>
<source file='[datastore] directory/harddisk3.vmdk'/>
- <target dev='sdae' bus='scsi'/>
- <address type='drive' controller='2' bus='0' target='0' unit='0'/>
+ <target dev='sdc' bus='scsi'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='2'/>
</disk>
<disk type='file' device='disk'>
<source file='[datastore] directory/harddisk4.vmdk'/>
- <target dev='sdat' bus='scsi'/>
- <address type='drive' controller='3' bus='0' target='0' unit='0'/>
+ <target dev='sdd' bus='scsi'/>
+ <address type='drive' controller='0' bus='0' target='0' unit='3'/>
</disk>
<controller type='scsi' index='0' model='buslogic'/>
<controller type='scsi' index='1' model='lsilogic'/>
--
2.45.2
Attachment:
signature.asc
Description: PGP signature
