Re: [PATCH 4/4] vmx: Ensure unique disk targets when parsing
On 1/25/24 15:48, Ján Tomko wrote:
On a Friday in 2023, Michal Privoznik wrote:
> When parsing disks from a vmx file, the target name is generated
> based on disk bus, controller the disk is attached to, and its
> unit. But in case of SCSI and SATA attached disks this does not
> guarantee the target name uniqueness. If there are two disks, one
> attached to scsi.0 and the other to sata.0 both end up with the
> same "sda" target name. And because of the way their drive
> address is derived, they end up with the same address too.
>
> Try harder to generate an unique disk target.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/vmx/vmx.c | 189 +++++++++++++----------
> tests/vmx2xmldata/esx-in-the-wild-12.xml | 4 +-
> tests/vmx2xmldata/esx-in-the-wild-8.xml | 4 +-
> 3 files changed, 109 insertions(+), 88 deletions(-)
>
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index 399f03b419..7c752c72f9 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -2142,105 +2142,126 @@ virXMXGenerateDiskTarget(virDomainDiskDef *def,
> int controllerOrBus,
> int unit)
> {
> - const char *prefix = NULL;
> - unsigned int idx = 0;
> -
> - switch (def->bus) {
> - case VIR_DOMAIN_DISK_BUS_SCSI:
> - if (controllerOrBus < 0 || controllerOrBus > 3) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("SCSI controller index %1$d out of
> [0..3] range"),
> - controllerOrBus);
> - return -1;
> - }
> + unsigned int tries = 0;
>
> - if (unit < 0 || unit > vmdef->scsiBusMaxUnit || unit == 7) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("SCSI unit index %1$d out of
> [0..6,8..%2$u] range"),
> - unit, vmdef->scsiBusMaxUnit);
> - return -1;
> - }
> + for (tries = 0; tries < 10; tries++) {
It seems strange to me to use 'tries' to try to compute something that
is already known.
Wouldn't generating the indexes in two passes work here?
First pass takes cares of only SATA disks, for example,
and the second pass will take care of all the other disks,
using the highest index used for a SATA disk as an offset.
That might work. Let me see if I can write such patch.
Michal