[libvirt] [PATCH 3/3] qemu: don't retry connect() if doing FD passing

[John Ferlan]

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 at 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 at 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);

[...]