[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 07/05/13 20:42, John Ferlan wrote:
On 05/03/2013 02:07 PM, Osier Yang wrote:
From: Han Cheng <hanc fnst cn fujitsu com>

This adds both attachment and detachment support for scsi host
device.

Signed-off-by: Han Cheng <hanc fnst cn fujitsu com>
Signed-off-by: Osier Yang <jyang 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:

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);

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]