Re: [libvirt] [PATCH 04/10] libxl: fill HVM SDL and VNC settings based on <graphics/> entries

Marek Marczykowski-Górecki wrote: > Vfb entries in domain config are used only by PV drivers. Qemu > parameters are build based on b_info struct. So fill it with the same > data as vfb entries (actually the first one). > This will additionally allow graphic-less domain, when no <graphics/> > entries are present in domain XML (previously VNC was always enabled). > This should already be handled in libxlMakeVfbList().

Is there something missing in that function?

Regards, Jim > Signed-off-by: Marek Marczykowski-Górecki <marmarek(a)invisiblethingslab.com&gt; > --- > src/libxl/libxl_conf.c | 100 ++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 74 insertions(+), 26 deletions(-) > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index 1811a83..c9f8ad5 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -618,12 +618,53 @@ libxlMakeChrdevStr(virDomainChrDefPtr def, char **buf) > _("unsupported chardev '%s'"), type); > return -1; > } > + return 0; > +} > + > +static int > +libxlMakeVNCInfo(virPortAllocatorPtr graphicsports, > + virDomainGraphicsDefPtr l_vfb, > + libxl_vnc_info *x_vnc) > +{ > + unsigned short port; > + const char *listenAddr; > + > + libxl_defbool_set(&x_vnc->enable, 1); > + /* driver handles selection of free port */ > + libxl_defbool_set(&x_vnc->findunused, 0); > + if (l_vfb->data.vnc.autoport) { > + > + if (virPortAllocatorAcquire(graphicsports, &port) < 0) > + return -1; > + l_vfb->data.vnc.port = port; > + } > + x_vnc->display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; > > + listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0); > + if (listenAddr) { > + /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */ > + VIR_FREE(x_vnc->listen); > + if (VIR_STRDUP(x_vnc->listen, listenAddr) < 0) > + return -1; > + } > + return 0; > +} > + > +static int > +libxlMakeSDLInfo(virDomainGraphicsDefPtr l_vfb, > + libxl_sdl_info *x_sdl) > +{ > + libxl_defbool_set(&x_sdl->enable, 1); > + if (VIR_STRDUP(x_sdl->display, l_vfb->data.sdl.display) < 0) > + return -1; > + if (VIR_STRDUP(x_sdl->xauthority, l_vfb->data.sdl.xauth) < 0) > + return -1; > return 0; > } > > static int > libxlMakeDomBuildInfo(virDomainDefPtr def, > + virPortAllocatorPtr graphicsports, > libxl_ctx *ctx, > libxl_domain_config *d_config) > { > @@ -745,6 +786,35 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, > return -1; > } > > + /* Disable VNC and SDL until explicitly enabled */ > + libxl_defbool_set(&b_info->u.hvm.vnc.enable, 0); > + libxl_defbool_set(&b_info->u.hvm.sdl.enable, 0); > + > + for (i = 0; i < def->ngraphics; i++) { > + switch (def->graphics[i]->type) { > + case VIR_DOMAIN_GRAPHICS_TYPE_VNC: > + if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) > + continue; > + if (libxlMakeVNCInfo(graphicsports, > + def->graphics[i], > + &b_info->u.hvm.vnc) < 0) > + return -1; > + if (def->graphics[i]->data.vnc.keymap && > + VIR_STRDUP(b_info->u.hvm.keymap, > + def->graphics[i]->data.vnc.keymap) < 0) { > + virReportOOMError(); > + return -1; > + } > + break; > + case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > + if (libxl_defbool_val(b_info->u.hvm.sdl.enable)) > + continue; > + if (libxlMakeSDLInfo(def->graphics[i], &b_info->u.hvm.sdl) < 0) > + return -1; > + break; > + } > + } > + > /* > * The following comment and calculation were taken directly from > * libxenlight's internal function libxl_get_required_shadow_memory(): > @@ -1171,38 +1241,16 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, > virDomainGraphicsDefPtr l_vfb, > libxl_device_vfb *x_vfb) > { > - unsigned short port; > - const char *listenAddr; > - > libxl_device_vfb_init(x_vfb); > > switch (l_vfb->type) { > case VIR_DOMAIN_GRAPHICS_TYPE_SDL: > - libxl_defbool_set(&x_vfb->sdl.enable, 1); > - if (VIR_STRDUP(x_vfb->sdl.display, l_vfb->data.sdl.display) < 0) > - return -1; > - if (VIR_STRDUP(x_vfb->sdl.xauthority, l_vfb->data.sdl.xauth) < 0) > + if (libxlMakeSDLInfo(l_vfb, &x_vfb->sdl) < 0) > return -1; > break; > case VIR_DOMAIN_GRAPHICS_TYPE_VNC: > - libxl_defbool_set(&x_vfb->vnc.enable, 1); > - /* driver handles selection of free port */ > - libxl_defbool_set(&x_vfb->vnc.findunused, 0); > - if (l_vfb->data.vnc.autoport) { > - > - if (virPortAllocatorAcquire(graphicsports, &port) < 0) > - return -1; > - l_vfb->data.vnc.port = port; > - } > - x_vfb->vnc.display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN; > - > - listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0); > - if (listenAddr) { > - /* libxl_device_vfb_init() does VIR_STRDUP("127.0.0.1") */ > - VIR_FREE(x_vfb->vnc.listen); > - if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0) > - return -1; > - } > + if (libxlMakeVNCInfo(graphicsports, l_vfb, &x_vfb->vnc) < 0) > + return -1; > if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0) > return -1; > break; > @@ -1611,7 +1659,7 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > if (libxlMakeDomCreateInfo(ctx, def, &d_config->c_info) < 0) > return -1; > > - if (libxlMakeDomBuildInfo(def, ctx, d_config) < 0) > + if (libxlMakeDomBuildInfo(def, graphicsports, ctx, d_config) < 0) > return -1; > > if (libxlMakeDiskList(def, d_config) < 0) >