qemu: fix libvirtd crash when querying halted cpus info

It was introduced by commit 7a51d9ebb, which started to use
monitor commands without job acquiring, which is unsafe and leads
to simultaneous access to vm->mon structure by different threads.

Crash backtrace is the following (shortened):

Program received signal SIGSEGV, Segmentation fault.
qemuMonitorSend (mon=mon@entry=0x7f4ef4000d20, msg=msg@entry=0x7f4f18e78640) at qemu/qemu_monitor.c:1011
1011        while (!mon->msg->finished) {

0  qemuMonitorSend () at qemu/qemu_monitor.c:1011
1  0x00007f691abdc720 in qemuMonitorJSONCommandWithFd () at qemu/qemu_monitor_json.c:298
2  0x00007f691abde64a in qemuMonitorJSONCommand at qemu/qemu_monitor_json.c:328
3  qemuMonitorJSONQueryCPUs at qemu/qemu_monitor_json.c:1408
4  0x00007f691abcaebd in qemuMonitorGetCPUInfo g@entry=false) at qemu/qemu_monitor.c:1931
5  0x00007f691ab96863 in qemuDomainRefreshVcpuHalted at qemu/qemu_domain.c:6309
6  0x00007f691ac0af99 in qemuDomainGetStatsVcpu at qemu/qemu_driver.c:18945
7  0x00007f691abef921 in qemuDomainGetStats  at qemu/qemu_driver.c:19469
8  qemuConnectGetAllDomainStats at qemu/qemu_driver.c:19559
9  0x00007f693382e806 in virConnectGetAllDomainStats at libvirt-domain.c:11546
10 0x00007f6934470c40 in remoteDispatchConnectGetAllDomainStats at remote.c:6267

(gdb) p mon->msg
$1 = (qemuMonitorMessagePtr) 0x0

This change fixes it by calling qemuDomainRefreshVcpuHalted only when job is acquired.

Signed-off-by: Maxim Nestratov <mnestratov@virtuozzo.com>

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a82e58b29f293baad74df661ca55c9f45cfdebe2..05a88c2887bad6e1420b1f4ea0d79e179ee39115 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18978,7 +18978,7 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
                        virDomainObjPtr dom,
                        virDomainStatsRecordPtr record,
                        int *maxparams,
-                       unsigned int privflags ATTRIBUTE_UNUSED)
+                       unsigned int privflags)
 {
     size_t i;
     int ret = -1;
@@ -19005,10 +19005,16 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
         VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0)
         goto cleanup;
 
-    if (qemuDomainRefreshVcpuHalted(driver, dom,
-                                    QEMU_ASYNC_JOB_NONE) == 0 &&
-        VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0)
-        goto cleanup;
+    if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
+        if (qemuDomainRefreshVcpuHalted(driver, dom,
+                                        QEMU_ASYNC_JOB_NONE) < 0) {
+            /* it's ok to be silent and go ahead, because halted vcpu info
+             * wasn't here from the beginning */
+            virResetLastError();
+        } else if (VIR_ALLOC_N(cpuhalted, virDomainDefGetVcpus(dom->def)) < 0) {
+            goto cleanup;
+        }
+    }
 
     if (qemuDomainHelperGetVcpus(dom, cpuinfo, cpuwait,
                                  virDomainDefGetVcpus(dom->def),
@@ -19462,7 +19468,7 @@ static struct qemuDomainGetStatsWorker qemuDomainGetStatsWorkers[] = {
     { qemuDomainGetStatsState, VIR_DOMAIN_STATS_STATE, false },
     { qemuDomainGetStatsCpu, VIR_DOMAIN_STATS_CPU_TOTAL, false },
     { qemuDomainGetStatsBalloon, VIR_DOMAIN_STATS_BALLOON, true },
-    { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, false },
+    { qemuDomainGetStatsVcpu, VIR_DOMAIN_STATS_VCPU, true },
     { qemuDomainGetStatsInterface, VIR_DOMAIN_STATS_INTERFACE, false },
     { qemuDomainGetStatsBlock, VIR_DOMAIN_STATS_BLOCK, true },
     { qemuDomainGetStatsPerf, VIR_DOMAIN_STATS_PERF, false },