[libvirt] [PATCH 2/3] qemu: Separate calls based on controller bus type

[Ján Tomko]

On 10/08/2013 02:45 PM, Eric Farman wrote:
> For systems without a PCI bus, attaching a SCSI controller fails:
> 
>   [root at xxxxxxxx ~]# cat controller.xml
>   <controller type='scsi' model='virtio-scsi' index='0' />
>   [root at 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 at 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 at linux.vnet.ibm.com>
> ---
>  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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131112/fc0ada6d/attachment-0001.sig>