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