[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 05/14] 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).
>
> Signed-off-by: Marek Marczykowski-Górecki <marmarek invisiblethingslab com>
> ---
>  src/libxl/libxl_conf.c | 114 +++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 82 insertions(+), 32 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 623956e..e5d8dc5 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -332,7 +332,56 @@ error:
>  }
>  
>  static int
> -libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
> +libxlMakeVNCInfo(libxlDriverPrivatePtr driver,
> +                 virDomainGraphicsDefPtr l_vfb,
> +                 libxl_vnc_info *x_vnc)
>   

For HVM guests, I noticed this function is called twice, first from
libxlMakeDomBuildInfo, then again via libxlMakeVfbList.

> +{
> +    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(driver->reservedVNCPorts, &port) < 0)
>   

Another port gets allocated on the second invocation

> +            return -1;
> +        if (port == 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                    "%s", _("Unable to find an unused VNC port"));
> +            return -1;
> +        }
> +        l_vfb->data.vnc.port = port;
>   

which updates libvirt's port info with incorrect info, since qemu gets
launched with the port info in the b_info struct.

> +    }
> +    x_vnc->display = l_vfb->data.vnc.port - LIBXL_VNC_PORT_MIN;
> +
> +    listenAddr = virDomainGraphicsListenGetAddress(l_vfb, 0);
> +    if (listenAddr) {
> +        /* libxl_device_vfb_init() does strdup("127.0.0.1") */
>   

You'll need to change the strdup to VIR_STRDUP, even in the comment, to
satisfy make syntax-check.  Or tweak the syntax-check rule :).

> +        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(libxlDriverPrivatePtr driver,
> +                      virDomainDefPtr def,
> +                      libxl_domain_config *d_config)
>  {
>      libxl_domain_build_info *b_info = &d_config->b_info;
>      int hvm = STREQ(def->os.type, "hvm");
> @@ -404,6 +453,34 @@ libxlMakeDomBuildInfo(virDomainDefPtr def, libxl_domain_config *d_config)
>          if (VIR_STRDUP(b_info->u.hvm.boot, bootorder) < 0)
>              goto error;
>  
> +        /* 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(driver, def->graphics[i],
> +                                &b_info->u.hvm.vnc) < 0)
> +                        goto error;
> +                    if (def->graphics[i]->data.vnc.keymap &&
> +                            (b_info->u.hvm.keymap =
> +                             strdup(def->graphics[i]->data.vnc.keymap)) == NULL) {
> +                        virReportOOMError();
>   

Needs to be converted to VIR_STRDUP.

Regards,
Jim

> +                        goto error;
> +                    }
> +                    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)
> +                        goto error;
> +                    break;
> +            }
> +        }
> +
>          /*
>           * The following comment and calculation were taken directly from
>           * libxenlight's internal function libxl_get_required_shadow_memory():
> @@ -679,41 +756,14 @@ libxlMakeVfb(libxlDriverPrivatePtr driver,
>               virDomainGraphicsDefPtr l_vfb,
>               libxl_device_vfb *x_vfb)
>  {
> -    unsigned short port;
> -    const char *listenAddr;
> -
>      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(driver->reservedVNCPorts, &port) < 0)
> -                    return -1;
> -                if (port == 0) {
> -                    virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                   "%s", _("Unable to find an unused VNC port"));
> -                    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(driver, l_vfb, &x_vfb->vnc) < 0)
> +                return -1;
>              if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0)
>                  return -1;
>              break;
> @@ -812,7 +862,7 @@ libxlBuildDomainConfig(libxlDriverPrivatePtr driver,
>      if (libxlMakeDomCreateInfo(driver, def, &d_config->c_info) < 0)
>          return -1;
>  
> -    if (libxlMakeDomBuildInfo(def, d_config) < 0) {
> +    if (libxlMakeDomBuildInfo(driver, def, d_config) < 0) {
>          return -1;
>      }
>  
>   


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]