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

Re: [libvirt] [PATCH 2/2] qemu: Don't fail if the SCSI host device is shareable between domains




[ shrinked ]

---
 src/libvirt_private.syms |  2 +-
 src/qemu/qemu_hostdev.c  | 75 ++++++++++++++++++++++++++----------------------
 src/util/virscsi.c       | 48 +++++++++++++++++++++++++------
 src/util/virscsi.h       |  7 +++--
 4 files changed, 84 insertions(+), 48 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 65d1bde..bd5f466 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1677,7 +1677,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 86a463a..9d81e94 100644
--- 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;
+            }

It took some thinking, but I convinced myself that this path doesn't
need the shared check since it's only called from qemuProcessReconnect;
however, if something else did call it some day then that check may be
necessary. It may be wise to add it anyway... I have no strong opinion
about it being required for this change.


Missed reply for this one.

Actually it needs the checking.  Assuming there are 2 live domains, and
they are using the same SCSI generic device.  It's possible since the
device could be shared among domains.   And in that case, we should
update the "dev->used_by" array instead of adding the device to the list
("driver->activeScsiHostdevs") directly, during the reconnecting sequence.
I.E it needs to restore the internal data correctly to the state just as the
one before libvirtd getting shutdown.

Regards,
Osier






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