15-Nov-16 15:38, Viktor Mihajlovski пишет:
On 15.11.2016 13:19, Peter Krempa wrote:
On Wed, Nov 02, 2016 at 18:56:49 +0300, Maxim Nestratov wrote:
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@xxxxxxxxxxxxx> ---
src/qemu/qemu_driver.c | 20 +++++++++++++------- 1 file changed,
13 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index dd1907b..43a546d 100644 --- a/src/qemu/qemu_driver.c +++
b/src/qemu/qemu_driver.c @@ -18915,7 +18915,7 @@
qemuDomainGetStatsVcpu(virQEMUDriverPtr driver, virDomainObjPtr
dom, virDomainStatsRecordPtr record, int *maxparams, -
unsigned int privflags ATTRIBUTE_UNUSED) +
unsigned int privflags) { size_t i; int ret = -1; @@ -18939,14
+18939,20 @@ qemuDomainGetStatsVcpu(virQEMUDriverPtr driver,
return -1;
if (VIR_ALLOC_N(cpuinfo, virDomainDefGetVcpus(dom->def)) < 0 || -
VIR_ALLOC_N(cpuwait, virDomainDefGetVcpus(dom->def)) < 0) -
goto cleanup; - - if (qemuDomainRefreshVcpuHalted(driver,
dom, - QEMU_ASYNC_JOB_NONE) ==
0 && + VIR_ALLOC_N(cpuwait,
virDomainDefGetVcpus(dom->def)) < 0 || VIR_ALLOC_N(cpuhalted,
virDomainDefGetVcpus(dom->def)) < 0) goto cleanup;
+ if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) { +
Spurious empty line.
+ 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(); +
} + } +
Not visible in this context: The output of the parameter needs to
be suppressed if the parameter can't be requested.
Peter
right, the VIR_ALLOC_N(cpuhalted...) should be conditional on the
success of qemuDomainRefreshVcpuHalted, as it was originally...
Agree. Sent a new version.
Maxim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list