[libvirt] [PATCH 02/17] qemu_process: move listen code out of qemuProcessSetupGraphics

[Pavel Hrdina]

On Fri, May 06, 2016 at 02:10:10PM +0200, Peter Krempa wrote:
> On Thu, May 05, 2016 at 18:20:21 +0200, Pavel Hrdina wrote:
> > Move adding the config listen type=address if there is none in
> > qemuProcessPrepareDomain and move check for multiple listens to
> > qemuProcessStartValidate.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  src/qemu/qemu_process.c | 75 ++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 55 insertions(+), 20 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index 348b392..17fd566 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> 
> > @@ -5138,6 +5143,35 @@ qemuProcessPrepareDomain(virConnectPtr conn,
> >      if (qemuAssignDeviceAliases(vm->def, priv->qemuCaps) < 0)
> >          goto cleanup;
> >  
> > +    /* Fill in run-time values for graphics devices. */
> > +    for (i = 0; i < vm->def->ngraphics; i++) {
> > +        virDomainGraphicsDefPtr graphics = vm->def->graphics[i];
> > +        char *listenAddr = NULL;
> > +
> > +        switch (graphics->type) {
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_VNC:
> > +            listenAddr = cfg->vncListen;
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
> > +            if (!listenAddr)
> 
> This fallthrough case with checking whether this was set is weird.
> Looking at the caps code it looks like its guaranteed that both
> vncListen and spiceListen are always allocated, but still it looks like
> both types should either have individual calls to
> virDomainGraphicsListenAppendAddress or the call should happen after the
> switch so that we don't obscure it using the fallthrough case.

Right, it can be easily updated that the listenAddr is set only for vnc or spice
and the virDomainGraphicsListenAppendAddress could be moved out of the switch
with this condition: if (graphics->nListens == 0 && listeAddr).  Do you require
v2 for this?

> 
> > +                listenAddr = cfg->spiceListen;
> > +
> > +            if (graphics->nListens == 0) {
> > +                if (virDomainGraphicsListenAppendAddress(graphics,
> > +                                                         listenAddr) < 0)
> > +                    goto cleanup;
> > +
> > +                graphics->listens[0].fromConfig = true;
> > +            }
> > +            break;
> > +
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_SDL:
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_RDP:
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP:
> > +        case VIR_DOMAIN_GRAPHICS_TYPE_LAST:
> > +            break;
> > +        }
> > +    }
> > +
> >      /* "volume" type disk's source must be translated before
> >       * cgroup and security setting.
> >       */