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

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

 



On Thu, Feb 05, 2015 at 02:53:44PM -0700, Jim Fehlig wrote:
> 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().

Indeed, I've blindly rebased that patch from older version, but...

> Is there
> something missing in that function?

... yes, if there is no graphics defined, the driver will set
b_info->u.hvm.nographic to 1. Which isn't enough to disable graphic,
because actual condition in libxl is:
    if (info->nographic && (!info->sdl && !info->vnc)) {
        flexarray_append(dm_args, "-nographic");
    }

I'll think about some better solution (perhaps only part about initially setting
vnc and sdl to false?).

> Regards,
> Jim
> 
> > Signed-off-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >  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)
> >   

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Attachment: pgpxhCFdhLNnZ.pgp
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]