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

[Osier Yang]

[ 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