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

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



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 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.


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