[libvirt] [PATCH v2] qemu: fix libvirtd crash when querying halted cpus info

[Maxim Nestratov]

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 at entry=0x7f4ef4000d20, msg=msg at 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 at 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 at virtuozzo.com>
---
 src/qemu/qemu_driver.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a82e58b..55b9566 100644
--- 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,15 @@ 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 +19467,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 },
-- 
2.4.11