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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-12-08 at 15:11 +0000, Anthony PERARD wrote:
> > > 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.
> 

Ah, yes, I get what you meant now.

> > > 
> > > 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.

I'm not sure, there may be an existing issue here I suppose.

> 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.

I wondered about that, but AFAICS VIR_STRDUP is a wrapper around
virStrdup and virStrdup doesn't free any existing destination.

Ian.


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