On 10/08/2013 02:45 PM, Eric Farman wrote:
> For systems without a PCI bus, attaching a SCSI controller fails:
>
> [root@xxxxxxxx ~]# cat controller.xml
> <controller type='scsi' model='virtio-scsi' index='0' />
> [root@xxxxxxxx ~]# virsh attach-device guest01 controller.xml
> error: Failed to attach device from controller.xml
> error: XML error: No PCI buses available
>
> A similar problem occurs with the detach of a controller:
>
> [root@xxxxxxxx ~]# virsh detach-device guest01 controller.xml
> error: Failed to detach device from controller.xml
> error: operation failed: controller scsi:0 not found
>
> The qemuDomainXXtachPciControllerDevice routines made assumptions
> that any caller had a PCI bus. These routines now selectively calls
> PCI functions where necessary, and assigns the device information
> type to one appropriate for the bus in use.
>
> Signed-off-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
> ---
> src/qemu/qemu_hotplug.c | 45 +++++++++++++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 14 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index bc0f57a..70a1b3d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -365,9 +365,23 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
> return -1;
> }
>
> + if (!controller->info.type) {
> + if (STREQLEN(vm->def->os.machine, "s390-ccw", 8) &&
STRPREFIX(vm->def->os.machine, "s390-ccw") should do the same
> + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW))
> + controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
> + else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390))
> + controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390;
> + }
> +
> if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> - if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0)
> - goto cleanup;
> + if (!controller->info.type ||
I'd prefer a comparison against VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE instead.
> + controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0)
> + goto cleanup;
> + } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> + if (qemuDomainCCWAddressAssign(&controller->info, priv->ccwaddrs, true) < 0)
> + goto cleanup;
> + }
> releaseaddr = true;
> if (qemuAssignDeviceControllerAlias(controller) < 0)
> goto cleanup;
> @@ -399,7 +413,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver,
> qemuDomainObjExitMonitor(driver, vm);
>
> if (ret == 0) {
> - controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> + if (!controller->info.type)
Here too.
> + controller->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
> virDomainControllerInsertPreAlloced(vm->def, controller);
> }
>
> @@ -3024,18 +3039,20 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
>
> detach = vm->def->controllers[idx];
>
> - if (!virDomainDeviceAddressIsValid(&detach->info,
> - VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> - virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> - _("device cannot be detached without a PCI address"));
> - goto cleanup;
> - }
> + if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> + if (!virDomainDeviceAddressIsValid(&detach->info,
> + VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("device cannot be detached without a PCI address"));
> + goto cleanup;
> + }
Shouldn't we check if there is either a VIRTIO_S390, VIRTIO_CCW or a PCI address?
>
> - if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
> - virReportError(VIR_ERR_OPERATION_FAILED,
> - _("cannot hot unplug multifunction PCI device: %s"),
> - dev->data.disk->dst);
> - goto cleanup;
> + if (qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("cannot hot unplug multifunction PCI device: %s"),
> + dev->data.disk->dst);
> + goto cleanup;
> + }
> }
>
> if (qemuDomainControllerIsBusy(vm, detach)) {
>
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list