12:37 p.m.
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