[libvirt] [PATCH v2 09/10] qemu: hotplug support for scsi hostdev

Osier Yang jyang at redhat.com
Wed Apr 3 10:20:25 UTC 2013


On 01/04/13 20:01, Han Cheng wrote:
> This patch add hotplug for scsi hostdev.

s/add/adds/

> And user should hotplug a virtio-scsi controller if doesn't exist.

I'm wondering if it could be implicitly added.


> Usb hostdev related codes are in qemuDomainAttachHostDevice, push down to
> qemuDomainAttachHostUsbDevice.
>
> Signed-off-by: Han Cheng <hanc.fnst at cn.fujitsu.com>
> ---
>   src/qemu/qemu_hotplug.c |  211 ++++++++++++++++++++++++++++++++++++++---------
>   1 files changed, 173 insertions(+), 38 deletions(-)
>
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b978b97..a78b410 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1116,6 +1116,98 @@ error:
>   
>   }
>   
> +int qemuDomainAttachHostScsiDevice(virQEMUDriverPtr driver,
> +                                   virDomainObjPtr vm,
> +                                   virDomainHostdevDefPtr hostdev)
> +{
> +    int ret;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char *devstr = NULL;
> +    char *drvstr = NULL;
> +
> +    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DRIVE) ||
> +        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE) ||
> +        !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SCSI_GENERIC)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("SCSI passthrough is not supported by this version of qemu"));
> +        return -1;
> +    }
> +
> +    if (qemuPrepareHostdevSCSIDevices(driver, vm->def->name,
> +                                      &hostdev, 1)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Unable to prepare scsi hostdev"));

Giving a prompt for what the device is should be better.

> +        return -1;
> +    }
> +
> +    if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, 0) < 0)
> +        goto error;
> +    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
> +        goto error;
> +    if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
> +        goto error;
> +
> +    if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0) {

vm->def->nhostdevs + 1

> +        virReportOOMError();
> +        goto error;
> +    }

vm->def->hostdevs[def->nhostdevs] is leaked if it errors out later.


> +
> +    if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
> +        virCgroupPtr cgroup = NULL;
> +        virSCSIDevicePtr scsi;
> +        qemuCgroupData data;
> +
> +        if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unable to find cgroup for %s"),
> +                           vm->def->name);
> +            goto error;
> +        }
> +
> +        if ((scsi = virSCSIDeviceNew(hostdev->source.subsys.u.scsi.adapter,
> +                                     hostdev->source.subsys.u.scsi.bus,
> +                                     hostdev->source.subsys.u.scsi.target,
> +                                     hostdev->source.subsys.u.scsi.unit,
> +                                     hostdev->readonly)) == NULL)
> +            goto error;
> +
> +        data.vm = vm;
> +        data.cgroup = cgroup;
> +        if (virSCSIDeviceFileIterate(scsi, qemuSetupHostScsiDeviceCgroup,
> +                                     &data) < 0) {
> +            virSCSIDeviceFree(scsi);
> +            goto error;
> +        }
> +        virSCSIDeviceFree(scsi);
> +    }
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    if ((ret = qemuMonitorAddDrive(priv->mon, drvstr)) == 0) {
> +        if ((ret = qemuMonitorAddDevice(priv->mon, devstr)) < 0) {
> +            VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
> +                     drvstr, devstr);

Indention.

> +            qemuMonitorDriveDel(priv->mon, drvstr);
> +        }
> +    }
> +    qemuDomainObjExitMonitor(driver, vm);
> +    virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);
> +    if (ret < 0)
> +        goto error;
> +

You can reallocate the vm->def->hostdevs right here to avoid the leak.

> +    vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +
> +    VIR_FREE(drvstr);
> +    VIR_FREE(devstr);

As I commented in 5/10, these frees can be destroyed.

> +
> +    return 0;
> +
> +error:
> +    qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &hostdev, 1);
> +    VIR_FREE(drvstr);
> +    VIR_FREE(devstr);
> +    return -1;
> +}
> +
>   int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>                                     virDomainObjPtr vm,
>                                     virDomainHostdevDefPtr hostdev)
> @@ -1123,6 +1215,26 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>       int ret;
>       qemuDomainObjPrivatePtr priv = vm->privateData;
>       char *devstr = NULL;
> +    virUSBDeviceList *list;
> +    virUSBDevicePtr usb = NULL;
> +
> +
> +    if (!(list = virUSBDeviceListNew()))
> +        goto error;
> +
> +    if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
> +        goto error;
> +
> +    if (virUSBDeviceListAdd(list, usb) < 0) {
> +        virUSBDeviceFree(usb);
> +        usb = NULL;
> +        goto error;
> +    }
> +
> +    if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0) {
> +        usb = NULL;
> +        goto error;
> +    }
>   
>       if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
>           if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0)
> @@ -1138,7 +1250,6 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>   
>       if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
>           virCgroupPtr cgroup = NULL;
> -        virUSBDevicePtr usb;
>           qemuCgroupData data;
>   
>           if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) {
> @@ -1148,19 +1259,12 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>               goto error;
>           }
>   
> -        if ((usb = virUSBDeviceNew(hostdev->source.subsys.u.usb.bus,
> -                                hostdev->source.subsys.u.usb.device,
> -                                NULL)) == NULL)
> -            goto error;
> -
>           data.vm = vm;
>           data.cgroup = cgroup;
>           if (virUSBDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
>                                       &data) < 0) {
> -            virUSBDeviceFree(usb);
>               goto error;
>           }
> -        virUSBDeviceFree(usb);
>       }
>   
>       qemuDomainObjEnterMonitor(driver, vm);
> @@ -1177,11 +1281,16 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
>   
>       vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
>   
> +    virUSBDeviceListSteal(list, usb);
> +    virObjectUnref(list);
>       VIR_FREE(devstr);
>   
>       return 0;
>   
>   error:
> +    if (usb)
> +        virUSBDeviceListSteal(driver->activeUsbHostdevs, usb);
> +    virObjectUnref(list);
>       VIR_FREE(devstr);
>       return -1;
>   }
> @@ -1190,9 +1299,6 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
>                                  virDomainObjPtr vm,
>                                  virDomainHostdevDefPtr hostdev)
>   {
> -    virUSBDeviceList *list;
> -    virUSBDevicePtr usb = NULL;
> -
>       if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("hostdev mode '%s' not supported"),
> @@ -1200,30 +1306,10 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
>           return -1;
>       }
>   
> -    if (!(list = virUSBDeviceListNew()))
> -        goto cleanup;
> -
> -    if (hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) {
> -        if (qemuFindHostdevUSBDevice(hostdev, true, &usb) < 0)
> -            goto cleanup;
> -
> -        if (virUSBDeviceListAdd(list, usb) < 0) {
> -            virUSBDeviceFree(usb);
> -            usb = NULL;
> -            goto cleanup;
> -        }
> -
> -        if (qemuPrepareHostdevUSBDevices(driver, vm->def->name, list) < 0) {
> -            usb = NULL;
> -            goto cleanup;
> -        }
> -
> -        virUSBDeviceListSteal(list, usb);
> -    }
>   
>       if (virSecurityManagerSetHostdevLabel(driver->securityManager,
>                                             vm->def, hostdev, NULL) < 0)
> -        goto cleanup;
> +        return -1;
>   
>       switch (hostdev->source.subsys.type) {
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
> @@ -1238,6 +1324,12 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
>               goto error;
>           break;
>   
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> +        if (qemuDomainAttachHostScsiDevice(driver, vm,
> +                                           hostdev) < 0)
> +            goto error;
> +        break;
> +
>       default:
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("hostdev subsys type '%s' not supported"),
> @@ -1245,18 +1337,12 @@ int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
>           goto error;
>       }
>   
> -    virObjectUnref(list);
>       return 0;
>   
>   error:
>       if (virSecurityManagerRestoreHostdevLabel(driver->securityManager,
>                                                 vm->def, hostdev, NULL) < 0)
>           VIR_WARN("Unable to restore host device labelling on hotplug fail");
> -
> -cleanup:
> -    virObjectUnref(list);
> -    if (usb)
> -        virUSBDeviceListSteal(driver->activeUsbHostdevs, usb);
>       return -1;
>   }
>   
> @@ -2439,6 +2525,46 @@ cleanup:
>   }
>   
>   static int
> +qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainHostdevDefPtr detach)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char *drvstr = NULL;
> +    char *devstr = NULL;
> +    int ret;
> +
> +    if (!detach->info->alias) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       "%s", _("device cannot be detached without a device alias"));
> +        return -1;
> +    }
> +    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(detach, priv->qemuCaps)))
> +        goto error;
> +    if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, detach, priv->qemuCaps)))
> +        goto error;
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +    if ((ret = qemuMonitorDelDevice(priv->mon, detach->info->alias)) == 0) {
> +        if ((ret = qemuMonitorDriveDel(priv->mon, drvstr)) < 0) {
> +            VIR_WARN("qemuMonitorDriveDel failed on %s (%s)",
> +                     detach->info->alias, drvstr);
> +            qemuMonitorAddDevice(priv->mon, devstr);
> +        }
> +    }
> +    qemuDomainObjExitMonitor(driver, vm);
> +    virDomainAuditHostdev(vm, detach, "detach", ret == 0);
> +
> +    if (ret == 0)
> +        qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &detach, 1);
> +
> +error:
> +    VIR_FREE(drvstr);
> +    VIR_FREE(devstr);
> +    return ret;
> +}
> +
> +static int
>   qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver,
>                                 virDomainObjPtr vm,
>                                 virDomainHostdevDefPtr detach)
> @@ -2511,6 +2637,9 @@ int qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
>       case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
>           ret = qemuDomainDetachHostUsbDevice(driver, vm, detach);
>           break;
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> +        ret = qemuDomainDetachHostScsiDevice(driver, vm, detach);
> +        break;
>       default:
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                          _("hostdev subsys type '%s' not supported"),
> @@ -2567,6 +2696,12 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver,
>                                  subsys->u.usb.vendor, subsys->u.usb.product);
>               }
>               break;
> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
> +            virReportError(VIR_ERR_OPERATION_FAILED,
> +                           _("host pci device %s:%d:%d.%d not found"),

s/pci/scsi/, others look good.




More information about the libvir-list mailing list