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

[Ján Tomko]

On 11/12/2013 05:59 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 |   52 ++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 29c6aea..f18e233 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 == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +        if (STRPREFIX(vm->def->os.machine, "s390-ccw") &&
> +            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;
> +    }
> +

These would be better inside the if below (if QEMU_CAPS_DEVICE is not set, we
don't probe for the VIRTIO_CCW and VIRTIO_S390 caps anyway).

>      if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> -        if (qemuDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0)
> -            goto cleanup;
> +        if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
> +            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;

> @@ -3031,18 +3046,27 @@ 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;
> +        }
>  
> -    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;
> +        }
> +    } else if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
> +        if (!virDomainDeviceAddressIsValid(&detach->info,
> +                                           VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) {
> +            virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> +                           _("device cannot be detached without a CCW address"));
> +            goto cleanup;
> +        }
>      }
>  
>      if (qemuDomainControllerIsBusy(vm, detach)) {
> 

This doesn't catch the case of ADDRESS_TYPE_NONE.
How about:

if (detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
    detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
    detach->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_S390) {
    virReportError(VIR_ERR_OPERATION_FAILED,
                   _("device with '%s' address cannot be detached"),
                   virDomainDeviceAddressTypeToString(detach->info.type));
    goto cleanup;
}

if (!virDomainDeviceAddressIsValid(&detach->info, detach->info.type)) {
    virReportError(VIR_ERR_OPERATION_FAILED,
                   _("device with invalid '%s' address cannot be detached"),
                   virDomainDeviceAddressTypeToString(detach->info.type));
    goto cleanup;
}

if (detach->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
    qemuIsMultiFunctionDevice(vm->def, &detach->info)) {
    virReportError(VIR_ERR_OPERATION_FAILED,
                   _("cannot hot unplug multifunction PCI device: %s"),
                   dev->data.disk->dst);
    goto cleanup;
}

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/20131120/baf22fc7/attachment-0001.sig>