Re: [libvirt] [PATCH 3/3] qemu: don't retry connect() if doing FD passing
On 03/14/2018 01:33 PM, Daniel P. Berrangé wrote:
Since libvirt called bind() and listen() on the UNIX socket, it is
guaranteed that connect() will immediately succeed, if QEMU is running
normally. It will only fail if QEMU has closed the monitor socket by
mistake or if QEMU has exited, letting the kernel close it.
With this in mind we can remove the retry loop and timeout when
connecting to the QEMU monitor if we are doing FD passing. Libvirt can
go straight to sending the QMP greeting and will simply block waiting
for a reply until QEMU is ready.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 2 +-
src/qemu/qemu_monitor.c | 54 ++++++++++++++++++++++++++------------------
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_process.c | 27 ++++++++++++++++------
tests/qemumonitortestutils.c | 1 +
5 files changed, 55 insertions(+), 30 deletions(-)
So just doing the monitor for now... Eventually the agent too?
How about having a news.xml patch - is this a newsworthy change?
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 57c06c7c15..9950461810 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1771,7 +1771,7 @@ qemuProcessInitMonitor(virQEMUDriverPtr driver,
static int
qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int asyncJob,
- qemuDomainLogContextPtr logCtxt)
+ bool retry, qemuDomainLogContextPtr logCtxt)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
qemuMonitorPtr mon = NULL;
@@ -1799,6 +1799,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int
asyncJob,
mon = qemuMonitorOpen(vm,
priv->monConfig,
priv->monJSON,
+ retry,
timeout,
&monitorCallbacks,
driver);
@@ -2174,17 +2175,23 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver,
{
int ret = -1;
virHashTablePtr info = NULL;
- qemuDomainObjPrivatePtr priv;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ bool retry = true;
This is also called from qemuProcessAttach where it wouldn't seem
there'd be necessarily be a chardev on the command line with the
pre-opened fd, but then again since it's being used for an already
running qemu instance, that would "I hope" work properly...
With that assumption in place,
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
+
+ if (priv->qemuCaps &&
+ virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS))
+ retry = false;
- VIR_DEBUG("Connect monitor to %p '%s'", vm, vm->def->name);
- if (qemuConnectMonitor(driver, vm, asyncJob, logCtxt) < 0)
+ VIR_DEBUG("Connect monitor to vm=%p name='%s' retry=%d",
+ vm, vm->def->name, retry);
+
+ if (qemuConnectMonitor(driver, vm, asyncJob, retry, logCtxt) < 0)
goto cleanup;
/* Try to get the pty path mappings again via the monitor. This is much more
* reliable if it's available.
* Note that the monitor itself can be on a pty, so we still need to try the
* log output method. */
- priv = vm->privateData;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
goto cleanup;
ret = qemuMonitorGetChardevInfo(priv->mon, &info);
[...]