This makes sure that that the commandlines generated for devices and 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 (or worse, encoding exceptions to the standard ${controller}${index} into the logic) 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". Because the function that finds a controller alias based on a device def requires a pointer to the full domainDef in order to get the list of controllers, the arglist of a few functions had to have this added. --- Changes from V1: This is the Patches 07/13 08/13 and 09/13 of V1 merged together on John's suggestion. src/qemu/qemu_command.c | 85 +++++++++++----------- src/qemu/qemu_command.h | 1 - .../qemuxml2argv-disk-sata-device.args | 4 +- 3 files changed, 46 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a75664c..a3413ee 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; 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: @@ -4559,8 +4557,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, } else { virBufferAddLit(&buf, "virtio-serial"); } - virBufferAsprintf(&buf, ",id=" QEMU_VIRTIO_SERIAL_PREFIX "%d", - def->idx); + virBufferAsprintf(&buf, ",id=%s", def->info.alias); if (def->opts.vioserial.ports != -1) { virBufferAsprintf(&buf, ",max_ports=%d", def->opts.vioserial.ports); @@ -4572,11 +4569,11 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, break; case VIR_DOMAIN_CONTROLLER_TYPE_CCID: - virBufferAsprintf(&buf, "usb-ccid,id=ccid%d", def->idx); + virBufferAsprintf(&buf, "usb-ccid,id=%s", def->info.alias); break; case VIR_DOMAIN_CONTROLLER_TYPE_SATA: - virBufferAsprintf(&buf, "ahci,id=ahci%d", def->idx); + virBufferAsprintf(&buf, "ahci,id=%s", def->info.alias); break; case VIR_DOMAIN_CONTROLLER_TYPE_USB: @@ -4596,8 +4593,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, _("PCI bridge index should be > 0")); goto error; } - virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d", - def->idx, def->idx); + virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=%s", + def->idx, def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) { @@ -4611,7 +4608,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, _("dmi-to-pci-bridge index should be > 0")); goto error; } - virBufferAsprintf(&buf, "i82801b11-bridge,id=pci.%d", def->idx); + virBufferAsprintf(&buf, "i82801b11-bridge,id=%s", def->info.alias); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: @@ -6311,10 +6308,13 @@ qemuBuildChrArgStr(virDomainChrSourceDefPtr dev, const char *prefix) static char * -qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, +qemuBuildVirtioSerialPortDevStr(virDomainDefPtr def, + virDomainChrDefPtr dev, virQEMUCapsPtr qemuCaps) { virBuffer buf = VIR_BUFFER_INITIALIZER; + const char *contAlias; + switch (dev->deviceType) { case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: virBufferAddLit(&buf, "virtconsole"); @@ -6346,12 +6346,13 @@ qemuBuildVirtioSerialPortDevStr(virDomainChrDefPtr dev, goto error; } - virBufferAsprintf(&buf, - ",bus=" QEMU_VIRTIO_SERIAL_PREFIX "%d.%d", - dev->info.addr.vioserial.controller, - dev->info.addr.vioserial.bus); - virBufferAsprintf(&buf, - ",nr=%d", + contAlias = virDomainControllerAliasFind(def, VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, + dev->info.addr.vioserial.controller); + if (!contAlias) + goto error; + + virBufferAsprintf(&buf, ",bus=%s.%d,nr=%d", contAlias, + dev->info.addr.vioserial.bus, dev->info.addr.vioserial.port); } @@ -10912,6 +10913,7 @@ qemuBuildParallelChrDeviceStr(char **deviceStr, static int qemuBuildChannelChrDeviceStr(char **deviceStr, + virDomainDefPtr def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -10934,7 +10936,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO: - if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(chr, qemuCaps))) + if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(def, chr, qemuCaps))) goto cleanup; break; @@ -10951,6 +10953,7 @@ qemuBuildChannelChrDeviceStr(char **deviceStr, static int qemuBuildConsoleChrDeviceStr(char **deviceStr, + virDomainDefPtr def, virDomainChrDefPtr chr, virQEMUCapsPtr qemuCaps) { @@ -10964,7 +10967,7 @@ qemuBuildConsoleChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO: - if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(chr, qemuCaps))) + if (!(*deviceStr = qemuBuildVirtioSerialPortDevStr(def, chr, qemuCaps))) goto cleanup; break; @@ -11008,11 +11011,11 @@ qemuBuildChrDeviceStr(char **deviceStr, break; case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL: - ret = qemuBuildChannelChrDeviceStr(deviceStr, chr, qemuCaps); + ret = qemuBuildChannelChrDeviceStr(deviceStr, vmdef, chr, qemuCaps); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE: - ret = qemuBuildConsoleChrDeviceStr(deviceStr, chr, qemuCaps); + ret = qemuBuildConsoleChrDeviceStr(deviceStr, vmdef, chr, qemuCaps); break; case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST: diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 9ef7046..0fc59a8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -36,7 +36,6 @@ # define QEMU_CONFIG_FORMAT_ARGV "qemu-argv" # define QEMU_DRIVE_HOST_PREFIX "drive-" -# define QEMU_VIRTIO_SERIAL_PREFIX "virtio-serial" # define QEMU_FSDEV_HOST_PREFIX "fsdev-" /* These are only defaults, they can be changed now in qemu.conf and 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 -- 2.1.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list