[libvirt] [PATCH v3] 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(a)virtuozzo.com>
---
v1-v2: don't output halted cpu info if it wasn't rathered
v2-v3: syntax-check recommendation
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 a82e58b..05a88c2 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 },
--
2.4.11