[libvirt] [PATCH 0/2] qemu: fix monitor object leak in qemu attach and allow attaching without JSON
Peter Krempa (2): qemu: Don't strictly require JSON monitor for vCPU detection qemu: attach: Close monitor socket on connection failure src/qemu/qemu_monitor.c | 8 ++++---- src/qemu/qemu_process.c | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) -- 2.10.0
Attaching to a existing qemu process allows to get us into a situation when qemu is new enough to have JSON monitor and new vCPU hotplug but the json monitor is not used. The vCPU detection code would require it though. This broke attaching to qemu processes. Make the condition less strict and just skip the vCPU hotplug detection if JSON monitor is not available. Resolves one of the symptoms in: https://bugzilla.redhat.com/show_bug.cgi?id=1378401 --- src/qemu/qemu_monitor.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8083a36..5175f4e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1898,14 +1898,14 @@ qemuMonitorGetCPUInfo(qemuMonitorPtr mon, int rc; qemuMonitorCPUInfoPtr info = NULL; - if (hotplug) - QEMU_CHECK_MONITOR_JSON(mon); - else - QEMU_CHECK_MONITOR(mon); + QEMU_CHECK_MONITOR(mon); if (VIR_ALLOC_N(info, maxvcpus) < 0) return -1; + if (!mon->json) + hotplug = false; + /* initialize a few non-zero defaults */ qemuMonitorCPUInfoClear(info, maxvcpus); -- 2.10.0
On Mon, Oct 03, 2016 at 01:32:46PM +0200, Peter Krempa wrote:
Attaching to a existing qemu process allows to get us into a situation when qemu is new enough to have JSON monitor and new vCPU hotplug but the json monitor is not used. The vCPU detection code would require it though. This broke attaching to qemu processes.
Make the condition less strict and just skip the vCPU hotplug detection if JSON monitor is not available.
Resolves one of the symptoms in: https://bugzilla.redhat.com/show_bug.cgi?id=1378401 --- src/qemu/qemu_monitor.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
ACK Jan
If attaching to a qemu process fails after opening the monitor socket libvirt does not clean up the monitor. As the monitor also holds a reference to the domain object the qemu attach API basically leaks it. QEMU also does not interact on a second monitor connection and thus a further attempt to attach to it would lock up. Prevent libvirt from leaking the monitor by explicitly closing it. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378401 --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 31c8453..a4bc082 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6386,6 +6386,9 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, if (active && virAtomicIntDecAndTest(&driver->nactive) && driver->inhibitCallback) driver->inhibitCallback(false, driver->inhibitOpaque); + + qemuMonitorClose(priv->mon); + priv->mon = NULL; qemuDomainLogContextFree(logCtxt); VIR_FREE(seclabel); VIR_FREE(sec_managers); -- 2.10.0
On Mon, Oct 03, 2016 at 01:32:47PM +0200, Peter Krempa wrote:
If attaching to a qemu process fails after opening the monitor socket libvirt does not clean up the monitor. As the monitor also holds a reference to the domain object the qemu attach API basically leaks it.
QEMU also does not interact on a second monitor connection and thus a further attempt to attach to it would lock up.
Prevent libvirt from leaking the monitor by explicitly closing it.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378401 --- src/qemu/qemu_process.c | 3 +++ 1 file changed, 3 insertions(+)
ACK Jan
participants (2)
-
Ján Tomko -
Peter Krempa