[libvirt] [PATCH 12/25] qemu: Add hotplug support for scsi host device

[John Ferlan]

On 05/03/2013 02:07 PM, Osier Yang wrote:
> From: Han Cheng <hanc.fnst at cn.fujitsu.com>
> 
> This adds both attachment and detachment support for scsi host
> device.
> 
> Signed-off-by: Han Cheng <hanc.fnst at cn.fujitsu.com>
> Signed-off-by: Osier Yang <jyang at redhat>
> ---
>  src/qemu/qemu_hotplug.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 136 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 422d336..e6bc3a2 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1194,6 +1194,74 @@ cleanup:
>      return ret;
>  }
>  
> +static int
> +qemuDomainAttachHostScsiDevice(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainHostdevDefPtr hostdev)
> +{
> +    int ret = -1;
> +    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_DEVICE_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,
> +                       _("Unable to prepare scsi hostdev: %s:%d:%d:%d"),
> +                       hostdev->source.subsys.u.scsi.adapter,
> +                       hostdev->source.subsys.u.scsi.bus,
> +                       hostdev->source.subsys.u.scsi.target,
> +                       hostdev->source.subsys.u.scsi.unit);
> +        return -1;
> +    }
> +
> +    if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, 0) < 0)
> +        goto cleanup;
> +
> +    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(hostdev, priv->qemuCaps)))
> +        goto cleanup;
> +
> +    if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
> +        goto cleanup;
> +
> +    if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    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);
> +            qemuMonitorDriveDel(priv->mon, drvstr);
> +        }
> +    }
> +    qemuDomainObjExitMonitor(driver, vm);
> +
> +    virDomainAuditHostdev(vm, hostdev, "attach", ret == 0);

It may be better to more closely follow code flow of other modules as I
think you could be missing a nuance of a failure mode by trying to be
more generic.  Just check all callers of AddDrive and AddDevice...

> +    if (ret < 0)
> +        goto cleanup;
> +
> +    vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
> +
> +    ret = 0;
> +cleanup:
> +    if (ret < 0)
> +        qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &hostdev, 1);
> +    VIR_FREE(drvstr);
> +    VIR_FREE(devstr);
> +    return ret;
> +}
> +
>  int qemuDomainAttachHostDevice(virQEMUDriverPtr driver,
>                                 virDomainObjPtr vm,
>                                 virDomainHostdevDefPtr hostdev)
> @@ -1225,6 +1293,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"),
> @@ -2441,11 +2515,59 @@ qemuDomainDetachHostUsbDevice(virQEMUDriverPtr driver,
>      return ret;
>  }
>  
> -static
> -int qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
> -                                   virDomainObjPtr vm,
> -                                   virDomainHostdevDefPtr detach,
> -                                   int idx)
> +static int
> +qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainHostdevDefPtr detach)
> +{
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    char *drvstr = NULL;
> +    char *devstr = NULL;
> +    int ret = -1;
> +
> +    if (!detach->info->alias) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       "%s", _("device cannot be detached without a device alias"));
> +        return -1;
> +    }
> +
> +    if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       "%s", _("device cannot be detached with this QEMU version"));
> +        return -1;
> +    }
> +
> +    if (!(drvstr = qemuBuildSCSIHostdevDrvStr(detach, priv->qemuCaps)))
> +        goto cleanup;
> +    if (!(devstr = qemuBuildSCSIHostdevDevStr(vm->def, detach, priv->qemuCaps)))
> +        goto cleanup;
> +
> +    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);

Coverity complains right here about no checking of return status:

(9) Event check_return: 	Calling function
"qemuMonitorAddDevice(qemuMonitorPtr, char const *)" without checking
return value (as is done elsewhere 8 out of 9 times).


As with attach, the flow of this code is a bit different than other
places where DelDevice() and DriveDel() are called - I would think you
may want to follow those other models more closely...

John


> +        }
> +    }
> +    qemuDomainObjExitMonitor(driver, vm);
> +
> +    virDomainAuditHostdev(vm, detach, "detach", ret == 0);
> +
> +    if (ret == 0)
> +        qemuDomainReAttachHostScsiDevices(driver, vm->def->name, &detach, 1);
> +
> +cleanup:
> +    VIR_FREE(drvstr);
> +    VIR_FREE(devstr);
> +    return ret;
> +}
> +
> +static int
> +qemuDomainDetachThisHostDevice(virQEMUDriverPtr driver,
> +                               virDomainObjPtr vm,
> +                               virDomainHostdevDefPtr detach,
> +                               int idx)
>  {
>      int ret = -1;
>  
> @@ -2472,6 +2594,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"),
> @@ -2531,6 +2656,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 scsi device %s:%d:%d.%d not found"),
> +                           subsys->u.scsi.adapter, subsys->u.scsi.bus,
> +                           subsys->u.scsi.target, subsys->u.scsi.unit);
> +            break;
>          default:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("unexpected hostdev type %d"), subsys->type);
>