Re: [Xen-devel] [PATCH] libxl: Set path to console on domain startup.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Dec 08, 2014 at 11:59:44AM +0000, Ian Campbell wrote:
> On Fri, 2014-12-05 at 16:30 +0000, Anthony PERARD wrote:
> 
> Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
> (I've done so here...)

Thanks.

> > The path to the pty of a Xen PV console is set only in
> > virDomainOpenConsole. But this is done too late. A call to
> > virDomainGetXMLDesc done before OpenConsole will not have the path to
> > the pty, but a call after OpenConsole will.
> > 
> > e.g. of the current issue.
> > Starting a domain with '<console type="pty"/>'
> > Then:
> > virDomainGetXMLDesc():
> >   <devices>
> >     <console type='pty'>
> >       <target type='xen' port='0'/>
> >     </console>
> >   </devices>
> > virDomainOpenConsole()
> > virDomainGetXMLDesc():
> >   <devices>
> >     <console type='pty' tty='/dev/pts/30'>
> >       <source path='/dev/pts/30'/>
> >       <target type='xen' port='0'/>
> >     </console>
> >   </devices>
> > 
> > The patch intend to get the tty path on the first call of GetXMLDesc.
> 
> Doesn't it actually do it on domain start (which makes more sense to me
> anyway).

Just a wording issue. I meant: Have GetXMLDesc always return the path to
the tty.

> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  src/libxl/libxl_domain.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..de56054 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,23 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > virDomainObjPtr vm,
> >      if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> >          goto cleanup_dom;
> >  
> > +    if (vm->def->nconsoles) {
> > +        virDomainChrDefPtr chr = NULL;
> > +        chr = vm->def->consoles[0];
> > +        if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) {
> > +            libxl_console_type console_type;
> > +            char *console = NULL;
> > +            console_type =
> > +                (chr->targetType == 
> > VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ?
> > +                 LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV);
> > +            ret = libxl_console_get_tty(priv->ctx, vm->def->id, 
> > chr->target.port,
> > +                                        console_type, &console);
> > +            if (!ret)
> > +                ignore_value(VIR_STRDUP(chr->source.data.file.path, 
> > console));
> 
> libxlDomainOpenConsole will strdup another (well, probably the same)
> value here, causing a leak I think, so you'll need some check there too
> I expect.

So, if OpenConsole is call twice, there will also be a leak? Or maybe it
can not be called twice.

Anyway, I though from the use of VIR_STRDUP there that it was safe to
call VIR_STRDUP several times and it well free the destination. I might
be wrong.

> > +            VIR_FREE(console);
> > +        }
> > +    }
> > +
> >      if (!start_paused) {
> >          libxl_domain_unpause(priv->ctx, domid);
> >          virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, 
> > VIR_DOMAIN_RUNNING_BOOTED);

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel