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(-)
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@redhat.com>
---
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.
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.2On 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@redhat.com>
>---
>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
>
© 2016 - 2024 Red Hat, Inc.