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

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(a)redhat.com&gt; > > --- > > 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?