Re: [PATCH v2] vmx: Ensure unique disk targets when parsing

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(a)redhat.com&gt; --- 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.2