[PATCH 1/2] qemu: simplify monitor callbacks

[Eric Blake <eblake@xxxxxxxxxx>]

The next patch will change reference counting idioms; consolidating
this pattern now makes the next patch smaller (touch only the new
macro rather than every caller).

* src/qemu/qemu_monitor.c (QEMU_MONITOR_CALLBACK): New helper.
(qemuMonitorGetDiskSecret, qemuMonitorEmitShutdown)
(qemuMonitorEmitReset, qemuMonitorEmitPowerdown)
(qemuMonitorEmitStop, qemuMonitorEmitRTCChange)
(qemuMonitorEmitWatchdog, qemuMonitorEmitIOError)
(qemuMonitorEmitGraphics): Use it to reduce duplication.
---

> > We should probably annotate qemuMonitorUnref() (and similar functions)
> > with ATTRIBUTE_RETURN_CHECK too

> I'm working on that now...

And here we go; this exercise didn't find any more unsafe instances.

 src/qemu/qemu_monitor.c |   80 +++++++++++++----------------------------------
 1 files changed, 22 insertions(+), 58 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index dc08594..fd02ca0 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -754,6 +754,15 @@ int qemuMonitorHMPCommandWithFd(qemuMonitorPtr mon,
         return qemuMonitorTextCommandWithFd(mon, cmd, scm_fd, reply);
 }

+/* Ensure proper locking around callbacks; assumes mon and ret are in
+ * scope.  */
+#define QEMU_MONITOR_CALLBACK(callback, ...)         \
+    qemuMonitorRef(mon);                             \
+    qemuMonitorUnlock(mon);                          \
+    if (mon->cb && mon->cb->callback)                \
+        ret = (mon->cb->callback)(mon, __VA_ARGS__); \
+    qemuMonitorLock(mon);                            \
+    qemuMonitorUnref(mon)

 int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
                              virConnectPtr conn,
@@ -765,12 +774,8 @@ int qemuMonitorGetDiskSecret(qemuMonitorPtr mon,
     *secret = NULL;
     *secretLen = 0;

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->diskSecretLookup)
-        ret = mon->cb->diskSecretLookup(mon, conn, mon->vm, path, secret, secretLen);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(diskSecretLookup, conn, mon->vm,
+                          path, secret, secretLen);
     return ret;
 }

@@ -780,12 +785,7 @@ int qemuMonitorEmitShutdown(qemuMonitorPtr mon)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainShutdown)
-        ret = mon->cb->domainShutdown(mon, mon->vm);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainShutdown, mon->vm);
     return ret;
 }

@@ -795,12 +795,7 @@ int qemuMonitorEmitReset(qemuMonitorPtr mon)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainReset)
-        ret = mon->cb->domainReset(mon, mon->vm);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainReset, mon->vm);
     return ret;
 }

@@ -810,12 +805,7 @@ int qemuMonitorEmitPowerdown(qemuMonitorPtr mon)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainPowerdown)
-        ret = mon->cb->domainPowerdown(mon, mon->vm);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainPowerdown, mon->vm);
     return ret;
 }

@@ -825,12 +815,7 @@ int qemuMonitorEmitStop(qemuMonitorPtr mon)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainStop)
-        ret = mon->cb->domainStop(mon, mon->vm);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainStop, mon->vm);
     return ret;
 }

@@ -840,12 +825,7 @@ int qemuMonitorEmitRTCChange(qemuMonitorPtr mon, long long offset)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainRTCChange)
-        ret = mon->cb->domainRTCChange(mon, mon->vm, offset);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainRTCChange, mon->vm, offset);
     return ret;
 }

@@ -855,12 +835,7 @@ int qemuMonitorEmitWatchdog(qemuMonitorPtr mon, int action)
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainWatchdog)
-        ret = mon->cb->domainWatchdog(mon, mon->vm, action);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainWatchdog, mon->vm, action);
     return ret;
 }

@@ -873,12 +848,7 @@ int qemuMonitorEmitIOError(qemuMonitorPtr mon,
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainIOError)
-        ret = mon->cb->domainIOError(mon, mon->vm, diskAlias, action, reason);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainIOError, mon->vm, diskAlias, action, reason);
     return ret;
 }

@@ -898,16 +868,10 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon,
     int ret = -1;
     VIR_DEBUG("mon=%p", mon);

-    qemuMonitorRef(mon);
-    qemuMonitorUnlock(mon);
-    if (mon->cb && mon->cb->domainGraphics)
-        ret = mon->cb->domainGraphics(mon, mon->vm,
-                                      phase,
-                                      localFamily, localNode, localService,
-                                      remoteFamily, remoteNode, remoteService,
-                                      authScheme, x509dname, saslUsername);
-    qemuMonitorLock(mon);
-    qemuMonitorUnref(mon);
+    QEMU_MONITOR_CALLBACK(domainGraphics, mon->vm, phase,
+                          localFamily, localNode, localService,
+                          remoteFamily, remoteNode, remoteService,
+                          authScheme, x509dname, saslUsername);
     return ret;
 }

-- 
1.7.4

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