[libvirt] [PATCH 12/25] qemu: Add hotplug support for scsi host device
Osier Yang
jyang at redhat.com
Mon May 13 15:48:11 UTC 2013
On 07/05/13 20:42, John Ferlan wrote:
> 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...
>
>
I think you meant keeping the original error? here is the diff:
-------------- next part --------------
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b6e5640..d037c9d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1240,9 +1240,15 @@ qemuDomainAttachHostScsiDevice(virQEMUDriverPtr driver,
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);
+ virErrorPtr orig_err = virSaveLastError();
+ if (qemuMonitorDriveDel(priv->mon, drvstr) < 0)
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice",
+ drvstr, devstr);
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
}
}
qemuDomainObjExitMonitor(driver, vm);
@@ -2545,9 +2551,15 @@ qemuDomainDetachHostScsiDevice(virQEMUDriverPtr driver,
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);
+ virErrorPtr orig_err = virSaveLastError();
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ VIR_WARN("Unable to add device %s (%s) after failed "
+ "qemuMonitorDriveDel",
+ drvstr, devstr);
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
}
}
qemuDomainObjExitMonitor(driver, vm);
More information about the libvir-list
mailing list