From 007fb4388f046460c72836fa7236eb18db49dfb1 Mon Sep 17 00:00:00 2001 From: Maxim Nestratov Date: Wed, 2 Nov 2016 18:56:49 +0300 Subject: [PATCH] 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 --- src/qemu/qemu_driver.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a82e58b29f..05a88c2887 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,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 }, -- GitLab