Re: [PATCH 07/13] qemu: use controller alias when constructing disk/controller args

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 05/05/2015 02:03 PM, Laine Stump wrote:
> This makes sure that that the commandlines generated for disk devices
> and disk controller devices are *all* using the alias that has been
> set in the controller's object as the id of the controller, rather
> than hardcoding a printf.
> 
> Since this "fixes" the controller name used for the sata controller,
> the commandline arg for the sata controller in the sata test case had
> to be adjusted to be "sata0" instead of "ahci0".
> ---
>  src/qemu/qemu_command.c                            | 46 +++++++++++-----------
>  .../qemuxml2argv-disk-sata-device.args             |  4 +-
>  2 files changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 89beeb3..b9f4c51 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4014,6 +4014,7 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>  {
>      virBuffer opt = VIR_BUFFER_INITIALIZER;
>      const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
> +    const char *contAlias;
>      int controllerModel;
>  
>      if (virDomainDiskDefDstDuplicates(def))
> @@ -4061,7 +4062,11 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>              virBufferAddLit(&opt, "ide-drive");
>          }
>  
> -        virBufferAsprintf(&opt, ",bus=ide.%d,unit=%d",
> +        if (!(contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_IDE,
> +                                                       disk->info.addr.drive.controller)))
> +           goto error;
> +        virBufferAsprintf(&opt, ",bus=%s.%d,unit=%d",
> +                          contAlias,
>                            disk->info.addr.drive.bus,
>                            disk->info.addr.drive.unit);
>          break;
> @@ -4114,6 +4119,10 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>              }
>          }
>  
> +        if (!(contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_SCSI,
> +                                                       disk->info.addr.drive.controller)))
> +           goto error;
> +
>          if (controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
>              if (disk->info.addr.drive.target != 0) {
>                  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -4122,8 +4131,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>                  goto error;
>              }
>  
> -            virBufferAsprintf(&opt, ",bus=scsi%d.%d,scsi-id=%d",
> -                              disk->info.addr.drive.controller,
> +            virBufferAsprintf(&opt, ",bus=%s.%d,scsi-id=%d",
> +                              contAlias,
>                                disk->info.addr.drive.bus,
>                                disk->info.addr.drive.unit);
>          } else {
> @@ -4144,8 +4153,8 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>                  }
>              }
>  
> -            virBufferAsprintf(&opt, ",bus=scsi%d.0,channel=%d,scsi-id=%d,lun=%d",
> -                              disk->info.addr.drive.controller,
> +            virBufferAsprintf(&opt, ",bus=%s.0,channel=%d,scsi-id=%d,lun=%d",
> +                              contAlias,
>                                disk->info.addr.drive.bus,
>                                disk->info.addr.drive.target,
>                                disk->info.addr.drive.unit);
> @@ -4173,23 +4182,12 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
>              virBufferAddLit(&opt, "ide-drive");
>          }
>  
> -        if (qemuDomainMachineIsQ35(def) &&
> -            disk->info.addr.drive.controller == 0) {
> -            /* Q35 machines have an implicit ahci (sata) controller at
> -             * 00:1F.2 which for some reason is hardcoded with the id
> -             * "ide" instead of the seemingly more reasonable "ahci0"
> -             * or "sata0".
> -             */
> -            virBufferAsprintf(&opt, ",bus=ide.%d", disk->info.addr.drive.unit);
> -        } else {
> -            /* All other ahci controllers have been created by
> -             * libvirt, and we gave them the id "ahci${n}" where ${n}
> -             * is the controller number. So we identify them that way.
> -             */
> -            virBufferAsprintf(&opt, ",bus=ahci%d.%d",
> -                              disk->info.addr.drive.controller,
> -                              disk->info.addr.drive.unit);
> -        }
> +        if (!(contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_SATA,
> +                                                      disk->info.addr.drive.controller)))
> +           goto error;
> +        virBufferAsprintf(&opt, ",bus=%s.%d",
> +                          contAlias,
> +                          disk->info.addr.drive.unit);
>          break;

through here - no issues...
>  
>      case VIR_DOMAIN_DISK_BUS_VIRTIO:
> @@ -4541,7 +4539,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>                             _("Unsupported controller model: %s"),
>                             virDomainControllerModelSCSITypeToString(def->model));
>          }
> -        virBufferAsprintf(&buf, ",id=scsi%d", def->idx);
> +        virBufferAsprintf(&buf, ",id=%s", def->info.alias);
>          break;
>  
>      case VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL:
> @@ -4576,7 +4574,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
>          break;
>  
>      case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
> -        virBufferAsprintf(&buf, "ahci,id=ahci%d", def->idx);
> +        virBufferAsprintf(&buf, "ahci,id=%s", def->info.alias);
>          break;
>  

But these two feel like they should go with patch 8 since that deals
with qemuBuildControllerDevStr

I guess the difference for me is these two go direct to def->info.alias
while the others use virDomainControllerAliasFind

ACK - either way to handle this.

John
>      case VIR_DOMAIN_CONTROLLER_TYPE_USB:
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-device.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-device.args
> index 475a0b1..7ae610f 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-device.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-sata-device.args
> @@ -1,7 +1,7 @@
>  LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
>  /usr/bin/qemu -S -M \
>  pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -monitor \
> -unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device ahci,id=ahci0,\
> +unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -device ahci,id=sata0,\
>  bus=pci.0,addr=0x3 -usb -drive file=/dev/HostVG/QEMUGuest1,if=none,\
> -id=drive-sata0-0-0 -device ide-drive,bus=ahci0.0,drive=drive-sata0-0-0,\
> +id=drive-sata0-0-0 -device ide-drive,bus=sata0.0,drive=drive-sata0-0-0,\
>  id=sata0-0-0 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]