[PATCH 2/5] qemu: minor monitor lock cleanups

[Eric Blake <eblake@xxxxxxxxxx>]

If virCondInit fails (okay, so that's unlikely), then we end up
attempting a virObjectUnlock() on the cleanup path, even though
we don't hold a lock.  This is not guaranteed to be safe.  While
at it, I noticed a couple places where we were referencing mon->fd
outside locks.

* src/qemu/qemu_monitor.c (qemuMonitorOpenInternal): Minimize lock
duration.  mon-watch doesn't need clean up on error.
(qemuMonitorGetBlockExtent, qemuMonitorBlockResize): Don't
dereference fd outside of lock.
---
 src/qemu/qemu_monitor.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 7f4a7a0..40eea75 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -717,7 +717,6 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
     if (json)
         mon->wait_greeting = 1;
     mon->cb = cb;
-    virObjectLock(mon);

     if (virSetCloseExec(mon->fd) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -731,6 +730,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
     }


+    virObjectLock(mon);
     virObjectRef(mon);
     if ((mon->watch = virEventAddHandle(mon->fd,
                                         VIR_EVENT_HANDLE_HANGUP |
@@ -740,6 +740,7 @@ qemuMonitorOpenInternal(virDomainObjPtr vm,
                                         mon,
                                         virObjectFreeCallback)) < 0) {
         virObjectUnref(mon);
+        virObjectUnlock(mon);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        _("unable to register monitor events"));
         goto cleanup;
@@ -759,11 +760,8 @@ cleanup:
      * so kill the callbacks now.
      */
     mon->cb = NULL;
-    virObjectUnlock(mon);
     /* The caller owns 'fd' on failure */
     mon->fd = -1;
-    if (mon->watch)
-        virEventRemoveHandle(mon->watch);
     qemuMonitorClose(mon);
     return NULL;
 }
@@ -1508,8 +1506,7 @@ int qemuMonitorGetBlockExtent(qemuMonitorPtr mon,
                               unsigned long long *extent)
 {
     int ret;
-    VIR_DEBUG("mon=%p, fd=%d, dev_name=%p",
-          mon, mon->fd, dev_name);
+    VIR_DEBUG("mon=%p, dev_name=%p", mon, dev_name);

     if (mon->json)
         ret = qemuMonitorJSONGetBlockExtent(mon, dev_name, extent);
@@ -1524,8 +1521,7 @@ int qemuMonitorBlockResize(qemuMonitorPtr mon,
                            unsigned long long size)
 {
     int ret;
-    VIR_DEBUG("mon=%p, fd=%d, devname=%p size=%llu",
-              mon, mon->fd, device, size);
+    VIR_DEBUG("mon=%p, devname=%p size=%llu", mon, device, size);

     if (mon->json)
         ret = qemuMonitorJSONBlockResize(mon, device, size);
-- 
1.8.1.2

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