[libvirt] [PATCH] qemu_hotplug: Remove virQEMUDriverPtr arguments

Suyang Chen posted 1 patch 3 weeks ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20190324004132.118974-1-dawson0xff@gmail.com
src/qemu/qemu_driver.c  |  5 ++---
src/qemu/qemu_hotplug.c | 29 ++++++++++++++---------------
src/qemu/qemu_hotplug.h |  3 +--
3 files changed, 17 insertions(+), 20 deletions(-)

[libvirt] [PATCH] qemu_hotplug: Remove virQEMUDriverPtr arguments

Posted by Suyang Chen 3 weeks ago
Since commit 2e6ecba1bcac, the pointer to the qemu driver is saved in
domain object's private data and hence does not have to be passed as
yet another parameter if domain object is already one of them.

This just changed qemuDomainChangeDiskLive and
qemuDomainChangeEjectableMedia functions

Signed-off-by: Suyang Chen <dawson0xff@gmail.com>
---
 src/qemu/qemu_driver.c  |  5 ++---
 src/qemu/qemu_hotplug.c | 29 ++++++++++++++---------------
 src/qemu/qemu_hotplug.h |  3 +--
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b27c6ce98e..5d064030e9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8100,7 +8100,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 static int
 qemuDomainChangeDiskLive(virDomainObjPtr vm,
                          virDomainDeviceDefPtr dev,
-                         virQEMUDriverPtr driver,
                          bool force)
 {
     virDomainDiskDefPtr disk = dev->data.disk;
@@ -8136,7 +8135,7 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
             goto cleanup;
         }
 
-        if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+        if (qemuDomainChangeEjectableMedia(vm, orig_disk,
                                            dev->data.disk->src, force) < 0)
             goto cleanup;
 
@@ -8165,7 +8164,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
     switch ((virDomainDeviceType)dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
         qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, NULL);
-        ret = qemuDomainChangeDiskLive(vm, dev, driver, force);
+        ret = qemuDomainChangeDiskLive(vm, dev, force);
         break;
 
     case VIR_DOMAIN_DEVICE_GRAPHICS:
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 41d60277d1..4bcec0e982 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -844,14 +844,13 @@ qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
  * Returns 0 on success, -1 on error and reports libvirt error
  */
 int
-qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
-                               virDomainObjPtr vm,
+qemuDomainChangeEjectableMedia(virDomainObjPtr vm,
                                virDomainDiskDefPtr disk,
                                virStorageSourcePtr newsrc,
                                bool force)
 {
-    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
     qemuDomainObjPrivatePtr priv = vm->privateData;
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(priv->driver);
     virStorageSourcePtr oldsrc = disk->src;
     bool sharedAdded = false;
     int ret = -1;
@@ -862,27 +861,27 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     if (virDomainDiskTranslateSourcePool(disk) < 0)
         goto cleanup;
 
-    if (qemuAddSharedDisk(driver, disk, vm->def->name) < 0)
+    if (qemuAddSharedDisk(priv->driver, disk, vm->def->name) < 0)
         goto cleanup;
 
     sharedAdded = true;
 
-    if (qemuDomainDetermineDiskChain(driver, vm, disk, NULL, true) < 0)
+    if (qemuDomainDetermineDiskChain(priv->driver, vm, disk, NULL, true) < 0)
         goto cleanup;
 
     if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0)
         goto cleanup;
 
-    if (qemuHotplugPrepareDiskSourceAccess(driver, vm, newsrc, false) < 0)
+    if (qemuHotplugPrepareDiskSourceAccess(priv->driver, vm, newsrc, false) < 0)
         goto cleanup;
 
-    if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0)
+    if (qemuHotplugAttachManagedPR(priv->driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) < 0)
         goto cleanup;
 
     if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
-        rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, oldsrc, newsrc, force);
+        rc = qemuDomainChangeMediaBlockdev(priv->driver, vm, disk, oldsrc, newsrc, force);
     else
-        rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
+        rc = qemuDomainChangeMediaLegacy(priv->driver, vm, disk, newsrc, force);
 
     virDomainAuditDisk(vm, oldsrc, newsrc, "update", rc >= 0);
 
@@ -891,8 +890,8 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
 
     /* remove the old source from shared device list */
     disk->src = oldsrc;
-    ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
-    ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, oldsrc, true));
+    ignore_value(qemuRemoveSharedDisk(priv->driver, disk, vm->def->name));
+    ignore_value(qemuHotplugPrepareDiskSourceAccess(priv->driver, vm, oldsrc, true));
 
     /* media was changed, so we can remove the old media definition now */
     virObjectUnref(oldsrc);
@@ -905,13 +904,13 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     /* undo changes to the new disk */
     if (ret < 0) {
         if (sharedAdded)
-            ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
+            ignore_value(qemuRemoveSharedDisk(priv->driver, disk, vm->def->name));
 
-        ignore_value(qemuHotplugPrepareDiskSourceAccess(driver, vm, newsrc, true));
+        ignore_value(qemuHotplugPrepareDiskSourceAccess(priv->driver, vm, newsrc, true));
     }
 
     /* remove PR manager object if unneeded */
-    ignore_value(qemuHotplugRemoveManagedPR(driver, vm, QEMU_ASYNC_JOB_NONE));
+    ignore_value(qemuHotplugRemoveManagedPR(priv->driver, vm, QEMU_ASYNC_JOB_NONE));
 
     /* revert old image do the disk definition */
     if (oldsrc)
@@ -1315,7 +1314,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
     if ((disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ||
          disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) &&
         (orig_disk = virDomainDiskFindByBusAndDst(vm->def, disk->bus, disk->dst))) {
-        if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
+        if (qemuDomainChangeEjectableMedia(vm, orig_disk,
                                            disk->src, false) < 0)
             return -1;
 
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 7ac03b7810..a60df36f74 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -26,8 +26,7 @@
 # include "qemu_domain.h"
 # include "domain_conf.h"
 
-int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
-                                   virDomainObjPtr vm,
+int qemuDomainChangeEjectableMedia(virDomainObjPtr vm,
                                    virDomainDiskDefPtr disk,
                                    virStorageSourcePtr newsrc,
                                    bool force);
-- 
2.20.1

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

Re: [libvirt] [PATCH] qemu_hotplug: Remove virQEMUDriverPtr arguments

Posted by Cole Robinson 2 weeks ago
On 3/23/19 8:41 PM, Suyang Chen wrote:
> Since commit 2e6ecba1bcac, the pointer to the qemu driver is saved in
> domain object's private data and hence does not have to be passed as
> yet another parameter if domain object is already one of them.
> 
> This just changed qemuDomainChangeDiskLive and
> qemuDomainChangeEjectableMedia functions
> 
> Signed-off-by: Suyang Chen <dawson0xff@gmail.com>

Thanks for the patch. Unfortunately a similar change prompted a
discussion over here:

https://www.redhat.com/archives/libvir-list/2019-March/msg01639.html

And not everyone is in agreement that these types of changes, in
isolation, are useful. So I don't think this patch will be applied. I
will remove the associated bitesizedtask entry from the wiki so this
doesn't cause additional confusion

Thanks,
Cole

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