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

[Peter Krempa]

On Fri, May 06, 2016 at 14:52:02 +0200, Pavel Hrdina wrote:
> 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?

Not really. This is what I'd do.

ACK using that approach
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160506/62ec6b37/attachment-0001.sig>