Following the same idea of avoid code repetition from the
previous patch, this commit introduces a new function that
aggregates the functions of qemuAddSharedDisk() and
qemuRemoveSharedDisk() into a single place, using a flag to
switch between add/remove operations.

Both qemuAddSharedDisk() and qemuRemoveSharedDisk() are
public, so keep them around to avoid changing other files
due to an internal qemu_conf.c refactory.

No functional change was made.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 src/qemu/qemu_conf.c | 130 +++++++++++++++++++++----------------------
 1 file changed, 62 insertions(+), 68 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a583440807..422f0d743d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1646,19 +1646,35 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver,
 }
 
 
-/* qemuAddSharedDisk:
- * @driver: Pointer to qemu driver struct
- * @src: disk source
- * @name: The domain name
- *
- * Increase ref count and add the domain name into the list which
- * records all the domains that use the shared device if the entry
- * already exists, otherwise add a new entry.
- */
-int
-qemuAddSharedDisk(virQEMUDriverPtr driver,
-                  virDomainDiskDefPtr disk,
-                  const char *name)
+static int
+qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver,
+                            const char *key,
+                            const char *name)
+{
+    qemuSharedDeviceEntryPtr entry = NULL;
+    int idx;
+
+    if (!(entry = virHashLookup(driver->sharedDevices, key)))
+        return -1;
+
+    /* Nothing to do if the shared disk is not recored in the table. */
+    if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx))
+        return 0;
+
+    if (entry->ref != 1)
+        VIR_DELETE_ELEMENT(entry->domains, idx, entry->ref);
+    else
+        ignore_value(virHashRemoveEntry(driver->sharedDevices, key));
+
+    return 0;
+}
+
+
+static int
+qemuAddRemoveSharedDiskInternal(virQEMUDriverPtr driver,
+                                virDomainDiskDefPtr disk,
+                                const char *name,
+                                bool addDisk)
 {
     char *key = NULL;
     int ret = -1;
@@ -1670,17 +1686,21 @@ qemuAddSharedDisk(virQEMUDriverPtr driver,
 
     qemuDriverLock(driver);
 
-    if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0)
-        goto cleanup;
-
     if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk))))
         goto cleanup;
 
-    if (qemuSharedDeviceEntryInsert(driver, key, name) < 0)
-        goto cleanup;
+    if (addDisk) {
+        if (qemuCheckSharedDisk(driver->sharedDevices, disk) < 0)
+            goto cleanup;
 
-    ret = 0;
+        if (qemuSharedDeviceEntryInsert(driver, key, name) < 0)
+            goto cleanup;
+    } else {
+        if (qemuSharedDeviceEntryRemove(driver, key, name) < 0)
+            goto cleanup;
+    }
 
+    ret = 0;
  cleanup:
     qemuDriverUnlock(driver);
     VIR_FREE(key);
@@ -1688,6 +1708,24 @@ qemuAddSharedDisk(virQEMUDriverPtr driver,
 }
 
 
+/* qemuAddSharedDisk:
+ * @driver: Pointer to qemu driver struct
+ * @src: disk source
+ * @name: The domain name
+ *
+ * Increase ref count and add the domain name into the list which
+ * records all the domains that use the shared device if the entry
+ * already exists, otherwise add a new entry.
+ */
+int
+qemuAddSharedDisk(virQEMUDriverPtr driver,
+                  virDomainDiskDefPtr disk,
+                  const char *name)
+{
+    return qemuAddRemoveSharedDiskInternal(driver, disk, name, true);
+}
+
+
 static bool
 qemuIsSharedHostdev(virDomainHostdevDefPtr hostdev)
 {
@@ -1721,30 +1759,6 @@ qemuGetHostdevPath(virDomainHostdevDefPtr hostdev)
 }
 
 
-static int
-qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver,
-                            const char *key,
-                            const char *name)
-{
-    qemuSharedDeviceEntryPtr entry = NULL;
-    int idx;
-
-    if (!(entry = virHashLookup(driver->sharedDevices, key)))
-        return -1;
-
-    /* Nothing to do if the shared disk is not recored in the table. */
-    if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx))
-        return 0;
-
-    if (entry->ref != 1)
-        VIR_DELETE_ELEMENT(entry->domains, idx, entry->ref);
-    else
-        ignore_value(virHashRemoveEntry(driver->sharedDevices, key));
-
-    return 0;
-}
-
-
 static int
 qemuAddRemoveSharedHostdevInternal(virQEMUDriverPtr driver,
                                    virDomainHostdevDefPtr hostdev,
@@ -1799,7 +1813,8 @@ qemuAddSharedDevice(virQEMUDriverPtr driver,
      * which is only valid for block disk and scsi host device.
      */
     if (dev->type == VIR_DOMAIN_DEVICE_DISK)
-        return qemuAddSharedDisk(driver, dev->data.disk, name);
+        return qemuAddRemoveSharedDiskInternal(driver, dev->data.disk,
+                                               name, true);
     else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV)
         return qemuAddRemoveSharedHostdevInternal(driver, dev->data.hostdev,
                                                   name, true);
@@ -1813,32 +1828,10 @@ qemuRemoveSharedDisk(virQEMUDriverPtr driver,
                      virDomainDiskDefPtr disk,
                      const char *name)
 {
-    char *key = NULL;
-    int ret = -1;
-
-    if (virStorageSourceIsEmpty(disk->src) ||
-        !disk->src->shared ||
-        !virStorageSourceIsBlockLocal(disk->src))
-        return 0;
-
-    qemuDriverLock(driver);
-
-    if (!(key = qemuGetSharedDeviceKey(virDomainDiskGetSource(disk))))
-        goto cleanup;
-
-    if (qemuSharedDeviceEntryRemove(driver, key, name) < 0)
-        goto cleanup;
-
-    ret = 0;
- cleanup:
-    qemuDriverUnlock(driver);
-    VIR_FREE(key);
-    return ret;
+    return qemuAddRemoveSharedDiskInternal(driver, disk, name, false);
 }
 
 
-
-
 /* qemuRemoveSharedDevice:
  * @driver: Pointer to qemu driver struct
  * @device: The device def
@@ -1854,7 +1847,8 @@ qemuRemoveSharedDevice(virQEMUDriverPtr driver,
                        const char *name)
 {
     if (dev->type == VIR_DOMAIN_DEVICE_DISK)
-        return qemuRemoveSharedDisk(driver, dev->data.disk, name);
+        return qemuAddRemoveSharedDiskInternal(driver, dev->data.disk,
+                                               name, false);
     else if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV)
         return qemuAddRemoveSharedHostdevInternal(driver, dev->data.hostdev,
                                                   name, false);
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 9/4/19 1:06 AM, Daniel Henrique Barboza wrote:
> Following the same idea of avoid code repetition from the
> previous patch, this commit introduces a new function that
> aggregates the functions of qemuAddSharedDisk() and
> qemuRemoveSharedDisk() into a single place, using a flag to
> switch between add/remove operations.
> 
> Both qemuAddSharedDisk() and qemuRemoveSharedDisk() are
> public, so keep them around to avoid changing other files
> due to an internal qemu_conf.c refactory.
> 
> No functional change was made.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>   src/qemu/qemu_conf.c | 130 +++++++++++++++++++++----------------------
>   1 file changed, 62 insertions(+), 68 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index a583440807..422f0d743d 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1646,19 +1646,35 @@ qemuSharedDeviceEntryInsert(virQEMUDriverPtr driver,
>   }
>   
>   
> -/* qemuAddSharedDisk:
> - * @driver: Pointer to qemu driver struct
> - * @src: disk source
> - * @name: The domain name
> - *
> - * Increase ref count and add the domain name into the list which
> - * records all the domains that use the shared device if the entry
> - * already exists, otherwise add a new entry.
> - */
> -int
> -qemuAddSharedDisk(virQEMUDriverPtr driver,
> -                  virDomainDiskDefPtr disk,
> -                  const char *name)
> +static int
> +qemuSharedDeviceEntryRemove(virQEMUDriverPtr driver,
> +                            const char *key,
> +                            const char *name)
> +{
> +    qemuSharedDeviceEntryPtr entry = NULL;
> +    int idx;
> +
> +    if (!(entry = virHashLookup(driver->sharedDevices, key)))
> +        return -1;
> +
> +    /* Nothing to do if the shared disk is not recored in the table. */
> +    if (!qemuSharedDeviceEntryDomainExists(entry, name, &idx))
> +        return 0;
> +
> +    if (entry->ref != 1)
> +        VIR_DELETE_ELEMENT(entry->domains, idx, entry->ref);
> +    else
> +        ignore_value(virHashRemoveEntry(driver->sharedDevices, key));
> +
> +    return 0;
> +}
> +
> +
> +static int
> +qemuAddRemoveSharedDiskInternal(virQEMUDriverPtr driver,
> +                                virDomainDiskDefPtr disk,
> +                                const char *name,
> +                                bool addDisk)

This can be named qemuSharedDiskAddRemoveInternal().

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list