Some, but not all, of the monitor event handlers check
the virObjectUnref return value to see if the domain
was disposed.

It should not be possible for this to happen, since
the functional ready holds a lock on the domain and
has only just acquired an extra reference on the
domain a few lines earlier.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_process.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index f7f6793113..51a086031d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -307,7 +307,7 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        ignore_value(virObjectUnref(vm));
+        virObjectUnref(vm);
         qemuProcessEventFree(processEvent);
         goto cleanup;
     }
@@ -840,15 +840,13 @@ qemuProcessHandleWatchdog(qemuMonitorPtr mon G_GNUC_UNUSED,
              */
             processEvent->vm = virObjectRef(vm);
             if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-                if (!virObjectUnref(vm))
-                    vm = NULL;
+                virObjectUnref(vm);
                 qemuProcessEventFree(processEvent);
             }
         }
     }
 
-    if (vm)
-        virObjectUnlock(vm);
+    virObjectUnlock(vm);
     virObjectEventStateQueue(driver->domainEventState, watchdogEvent);
     virObjectEventStateQueue(driver->domainEventState, lifecycleEvent);
 
@@ -977,7 +975,7 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon G_GNUC_UNUSED,
         processEvent->status = status;
 
         if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-            ignore_value(virObjectUnref(vm));
+            virObjectUnref(vm);
             goto cleanup;
         }
 
@@ -1039,7 +1037,7 @@ qemuProcessHandleJobStatusChange(qemuMonitorPtr mon G_GNUC_UNUSED,
         processEvent->data = virObjectRef(job);
 
         if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-            ignore_value(virObjectUnref(vm));
+            virObjectUnref(vm);
             goto cleanup;
         }
 
@@ -1342,14 +1340,12 @@ qemuProcessHandleGuestPanic(qemuMonitorPtr mon G_GNUC_UNUSED,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        if (!virObjectUnref(vm))
-            vm = NULL;
+        virObjectUnref(vm);
         qemuProcessEventFree(processEvent);
     }
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virObjectUnlock(vm);
 
     return 0;
 }
@@ -1383,7 +1379,7 @@ qemuProcessHandleDeviceDeleted(qemuMonitorPtr mon G_GNUC_UNUSED,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        ignore_value(virObjectUnref(vm));
+        virObjectUnref(vm);
         goto error;
     }
 
@@ -1554,7 +1550,7 @@ qemuProcessHandleNicRxFilterChanged(qemuMonitorPtr mon G_GNUC_UNUSED,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        ignore_value(virObjectUnref(vm));
+        virObjectUnref(vm);
         goto error;
     }
 
@@ -1593,7 +1589,7 @@ qemuProcessHandleSerialChanged(qemuMonitorPtr mon G_GNUC_UNUSED,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        ignore_value(virObjectUnref(vm));
+        virObjectUnref(vm);
         goto error;
     }
 
@@ -1873,14 +1869,12 @@ qemuProcessHandleGuestCrashloaded(qemuMonitorPtr mon G_GNUC_UNUSED,
     processEvent->vm = virObjectRef(vm);
 
     if (virThreadPoolSendJob(driver->workerPool, 0, processEvent) < 0) {
-        if (!virObjectUnref(vm))
-            vm = NULL;
+        virObjectUnref(vm);
         qemuProcessEventFree(processEvent);
     }
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virObjectUnlock(vm);
 
     return 0;
 }
-- 
2.24.1

On 5/19/20 12:41 PM, Daniel P. Berrangé wrote:
> Some, but not all, of the monitor event handlers check
> the virObjectUnref return value to see if the domain
> was disposed.
> 
> It should not be possible for this to happen, since
> the functional ready holds a lock on the domain and

s/functional ready/function already/ (not everyday that a misplaced 
space still results in two valid words which might still legitimately 
appear adjacent in some other sentence...)

> has only just acquired an extra reference on the
> domain a few lines earlier.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   src/qemu/qemu_process.c | 30 ++++++++++++------------------
>   1 file changed, 12 insertions(+), 18 deletions(-)
> 
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org