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&gt; > --- > 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&gt; 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); [...]