qemu: Don't fail if the SCSI host device is shareable between domains

It doesn't make sense to fail if the SCSI host device is specified
as "shareable" explicitly between domains (NB, it works if and only
if the device is specified as "shareable" for *all* domains,
otherwise it fails).

To fix the problem, this patch introduces an array for virSCSIDevice
struct, which records all the names of domain which are using the
device (note that the recorded domains must specify the device as
shareable).  And the change on the data struct brings on many
subsequent changes in the code.

Prior to this patch, the "shareable" tag didn't work as expected,
it actually work like "non-shareable".  So this patch also added notes
in formatdomain.html to declare the fact.

* src/util/virscsi.h:
  - Remove virSCSIDeviceGetUsedBy
  - Change definition of virSCSIDeviceGetUsedBy and virSCSIDeviceListDel
  - Add virSCSIDeviceIsAvailable

* src/util/virscsi.c:
  - struct virSCSIDevice: Change "used_by" to be an array; Add
    "n_used_by" as the array count
  - virSCSIDeviceGetUsedBy: Removed
  - virSCSIDeviceFree: frees the "used_by" array
  - virSCSIDeviceSetUsedBy: Copy the domain name to avoid potential
    memory corruption
  - virSCSIDeviceIsAvailable: New
  - virSCSIDeviceListDel: Change the logic, for device which is already
    in the list, just remove the corresponding entry in "used_by". And
    since it's only used in one place, we can safely removing the code
    to find out the dev in the list first.
  - Copyright updating

* src/libvirt_private.sys:
  - virSCSIDeviceGetUsedBy: Remove
  - virSCSIDeviceIsAvailable: New

* src/qemu/qemu_hostdev.c:
  - qemuUpdateActiveScsiHostdevs: Check if the device existing before
    adding it to the list;
  - qemuPrepareHostdevSCSIDevices: Error out if the not all domains
    use the device as "shareable"; Also don't try to add the device
    to the activeScsiHostdevs list if it already there; And make
    more sensible error w.r.t the current "shareable" value in
    driver->activeScsiHostdevs.
  - qemuDomainReAttachHostScsiDevices: Change the logic according
    to the changes on helpers.

Signed-off-by: Osier Yang <jyang@redhat.com>

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ff50214123deae18a806578e34ba60ba82bc4f7a..fd02864d6374de7cf4c7099fc44526e0fe620dea 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2798,6 +2798,11 @@
         between domains (assuming the hypervisor and OS support this).
         Only supported by SCSI host device.
         <span class="since">Since 1.0.6</span>
+        <p>
+          Note: Although <code>shareable</code> was introduced
+          <span class="since">in 1.0.6</span>, it did not work as
+          as expected until <span class="since">1.2.2</span>.
+        </p>
       </dd>
     </dl>
 

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 45f31171cc40484db9902bf80434d84d3b760dda..f890c6ecee9a9d022647500370581d7879b7e12e 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1687,7 +1687,7 @@ virSCSIDeviceGetSgName;
 virSCSIDeviceGetShareable;
 virSCSIDeviceGetTarget;
 virSCSIDeviceGetUnit;
-virSCSIDeviceGetUsedBy;
+virSCSIDeviceIsAvailable;
 virSCSIDeviceListAdd;
 virSCSIDeviceListCount;
 virSCSIDeviceListDel;

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 86a463a65c69f592e257176ca522b596c1e4b382..2b9d274279e3722f0202e106654c34044d9674a3 100644 (file)
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -250,13 +250,14 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver,
     virDomainHostdevDefPtr hostdev = NULL;
     size_t i;
     int ret = -1;
+    virSCSIDevicePtr scsi = NULL;
+    virSCSIDevicePtr tmp = NULL;
 
     if (!def->nhostdevs)
         return 0;
 
     virObjectLock(driver->activeScsiHostdevs);
     for (i = 0; i < def->nhostdevs; i++) {
-        virSCSIDevicePtr scsi = NULL;
         hostdev = def->hostdevs[i];
 
         if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS ||
@@ -271,11 +272,18 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver,
                                       hostdev->shareable)))
             goto cleanup;
 
-        virSCSIDeviceSetUsedBy(scsi, def->name);
-
-        if (virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0) {
+        if ((tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) {
+            if (virSCSIDeviceSetUsedBy(tmp, def->name) < 0) {
+                virSCSIDeviceFree(scsi);
+                goto cleanup;
+            }
             virSCSIDeviceFree(scsi);
-            goto cleanup;
+        } else {
+            if (virSCSIDeviceSetUsedBy(scsi, def->name) < 0 ||
+                virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0) {
+                virSCSIDeviceFree(scsi);
+                goto cleanup;
+            }
         }
     }
     ret = 0;
@@ -1118,24 +1126,29 @@ qemuPrepareHostdevSCSIDevices(virQEMUDriverPtr driver,
     for (i = 0; i < count; i++) {
         virSCSIDevicePtr scsi = virSCSIDeviceListGet(list, i);
         if ((tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) {
-            const char *other_name = virSCSIDeviceGetUsedBy(tmp);
+            bool scsi_shareable = virSCSIDeviceGetShareable(scsi);
+            bool tmp_shareable = virSCSIDeviceGetShareable(tmp);
 
-            if (other_name)
-                virReportError(VIR_ERR_OPERATION_INVALID,
-                               _("SCSI device %s is in use by domain %s"),
-                               virSCSIDeviceGetName(tmp), other_name);
-            else
+            if (!(scsi_shareable && tmp_shareable)) {
                 virReportError(VIR_ERR_OPERATION_INVALID,
-                               _("SCSI device %s is already in use"),
+                               _("SCSI device %s is already in use by "
+                                 "other domain(s) as '%s'"),
+                               tmp_shareable ? "shareable" : "non-shareable",
                                virSCSIDeviceGetName(tmp));
-            goto error;
-        }
+                goto error;
+            }
 
-        virSCSIDeviceSetUsedBy(scsi, name);
-        VIR_DEBUG("Adding %s to activeScsiHostdevs", virSCSIDeviceGetName(scsi));
+            if (virSCSIDeviceSetUsedBy(tmp, name) < 0)
+                goto error;
+        } else {
+            if (virSCSIDeviceSetUsedBy(scsi, name) < 0)
+                goto error;
 
-        if (virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0)
-            goto error;
+            VIR_DEBUG("Adding %s to activeScsiHostdevs", virSCSIDeviceGetName(scsi));
+
+            if (virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0)
+                goto error;
+        }
     }
 
     virObjectUnlock(driver->activeScsiHostdevs);
@@ -1380,8 +1393,8 @@ qemuDomainReAttachHostScsiDevices(virQEMUDriverPtr driver,
     virObjectLock(driver->activeScsiHostdevs);
     for (i = 0; i < nhostdevs; i++) {
         virDomainHostdevDefPtr hostdev = hostdevs[i];
-        virSCSIDevicePtr scsi, tmp;
-        const char *used_by = NULL;
+        virSCSIDevicePtr scsi;
+        virSCSIDevicePtr tmp;
         virDomainDeviceDef dev;
 
         dev.type = VIR_DOMAIN_DEVICE_HOSTDEV;
@@ -1411,30 +1424,26 @@ qemuDomainReAttachHostScsiDevices(virQEMUDriverPtr driver,
         /* Only delete the devices which are marked as being used by @name,
          * because qemuProcessStart could fail on the half way. */
 
-        tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi);
-        virSCSIDeviceFree(scsi);
-
-        if (!tmp) {
+        if (!(tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) {
             VIR_WARN("Unable to find device %s:%d:%d:%d "
                      "in list of active SCSI devices",
                      hostdev->source.subsys.u.scsi.adapter,
                      hostdev->source.subsys.u.scsi.bus,
                      hostdev->source.subsys.u.scsi.target,
                      hostdev->source.subsys.u.scsi.unit);
+            virSCSIDeviceFree(scsi);
             continue;
         }
 
-        used_by = virSCSIDeviceGetUsedBy(tmp);
-        if (STREQ_NULLABLE(used_by, name)) {
-            VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeScsiHostdevs",
-                      hostdev->source.subsys.u.scsi.adapter,
-                      hostdev->source.subsys.u.scsi.bus,
-                      hostdev->source.subsys.u.scsi.target,
-                      hostdev->source.subsys.u.scsi.unit,
-                      name);
+        VIR_DEBUG("Removing %s:%d:%d:%d dom=%s from activeScsiHostdevs",
+                   hostdev->source.subsys.u.scsi.adapter,
+                   hostdev->source.subsys.u.scsi.bus,
+                   hostdev->source.subsys.u.scsi.target,
+                   hostdev->source.subsys.u.scsi.unit,
+                   name);
 
-            virSCSIDeviceListDel(driver->activeScsiHostdevs, tmp);
-        }
+        virSCSIDeviceListDel(driver->activeScsiHostdevs, tmp, name);
+        virSCSIDeviceFree(scsi);
     }
     virObjectUnlock(driver->activeScsiHostdevs);
 }

diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 58d9e259a483248c3af671c8e14d331ab51ae477..9aa7f350492f0d5da182ebc9aa0967977e20c09c 100644 (file)
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -1,6 +1,7 @@
 /*
  * virscsi.c: helper APIs for managing host SCSI devices
  *
+ * Copyright (C) 2013-2014 Red Hat, Inc.
  * Copyright (C) 2013 Fujitsu, Inc.
  *
  * This library is free software; you can redistribute it and/or
@@ -19,6 +20,7 @@
  *
  * Authors:
  *     Han Cheng <hanc.fnst@cn.fujitsu.com>
+ *     Osier Yang <jyang@redhat.com>
  */
 
 #include <config.h>
@@ -55,7 +57,8 @@ struct _virSCSIDevice {
     char *name; /* adapter:bus:target:unit */
     char *id;   /* model:vendor */
     char *sg_path; /* e.g. /dev/sg2 */
-    const char *used_by; /* name of the domain using this dev */
+    char **used_by; /* name of the domains using this dev */
+    size_t n_used_by; /* how many domains are using this dev */
 
     bool readonly;
     bool shareable;
@@ -256,26 +259,36 @@ cleanup:
 void
 virSCSIDeviceFree(virSCSIDevicePtr dev)
 {
+    size_t i;
+
     if (!dev)
         return;
 
     VIR_FREE(dev->id);
     VIR_FREE(dev->name);
     VIR_FREE(dev->sg_path);
+    for (i = 0; i < dev->n_used_by; i++)
+        VIR_FREE(dev->used_by[i]);
+    VIR_FREE(dev->used_by);
     VIR_FREE(dev);
 }
 
-void
+int
 virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev,
                        const char *name)
 {
-    dev->used_by = name;
+    char *copy = NULL;
+
+    if (VIR_STRDUP(copy, name) < 0)
+        return -1;
+
+    return VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy);
 }
 
-const char *
-virSCSIDeviceGetUsedBy(virSCSIDevicePtr dev)
+bool
+virSCSIDeviceIsAvailable(virSCSIDevicePtr dev)
 {
-    return dev->used_by;
+    return dev->n_used_by == 0;
 }
 
 const char *
@@ -406,10 +419,23 @@ virSCSIDeviceListSteal(virSCSIDeviceListPtr list,
 
 void
 virSCSIDeviceListDel(virSCSIDeviceListPtr list,
-                     virSCSIDevicePtr dev)
+                     virSCSIDevicePtr dev,
+                     const char *name)
 {
-    virSCSIDevicePtr ret = virSCSIDeviceListSteal(list, dev);
-    virSCSIDeviceFree(ret);
+    virSCSIDevicePtr tmp = NULL;
+    size_t i;
+
+    for (i = 0; i < dev->n_used_by; i++) {
+        if (STREQ_NULLABLE(dev->used_by[i], name)) {
+            if (dev->n_used_by > 1) {
+                VIR_DELETE_ELEMENT(dev->used_by, i, dev->n_used_by);
+            } else {
+                tmp = virSCSIDeviceListSteal(list, dev);
+                virSCSIDeviceFree(tmp);
+            }
+            break;
+        }
+    }
 }
 
 virSCSIDevicePtr

diff --git a/src/util/virscsi.h b/src/util/virscsi.h
index b2e98cac6857deaaee9a6868d2ddcc7be87cd469..aff7e5a3925c110fc75eac6df4a1918a8b060fd1 100644 (file)
--- a/src/util/virscsi.h
+++ b/src/util/virscsi.h
@@ -50,8 +50,8 @@ virSCSIDevicePtr virSCSIDeviceNew(const char *adapter,
                                   bool shareable);
 
 void virSCSIDeviceFree(virSCSIDevicePtr dev);
-void virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, const char *name);
-const char *virSCSIDeviceGetUsedBy(virSCSIDevicePtr dev);
+int virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, const char *name);
+bool virSCSIDeviceIsAvailable(virSCSIDevicePtr dev);
 const char *virSCSIDeviceGetName(virSCSIDevicePtr dev);
 unsigned int virSCSIDeviceGetAdapter(virSCSIDevicePtr dev);
 unsigned int virSCSIDeviceGetBus(virSCSIDevicePtr dev);
@@ -83,7 +83,8 @@ size_t virSCSIDeviceListCount(virSCSIDeviceListPtr list);
 virSCSIDevicePtr virSCSIDeviceListSteal(virSCSIDeviceListPtr list,
                                         virSCSIDevicePtr dev);
 void virSCSIDeviceListDel(virSCSIDeviceListPtr list,
-                          virSCSIDevicePtr dev);
+                          virSCSIDevicePtr dev,
+                          const char *name);
 virSCSIDevicePtr virSCSIDeviceListFind(virSCSIDeviceListPtr list,
                                        virSCSIDevicePtr dev);