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

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Dec 09, 2014 at 03:56:02PM +0000, Ian Campbell wrote:
> On Tue, 2014-12-09 at 15:39 +0000, Anthony PERARD wrote:
> > 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 have the TTY path on the first call of GetXMLDesc.
> > This is done by setting up the path at domain start up instead of in
> > OpenConsole.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1170743
> > 
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > 
> > ---
> > Change in V2:
> >   Adding bug report link.
> >   Reword the last part of the patch description.
> >   Cleanup the code.
> >   Use VIR_FREE before VIR_STRDUP.
> >   Remove the code from OpenConsole as it is now a duplicate.
> > ---
> >  src/libxl/libxl_domain.c | 20 ++++++++++++++++++++
> >  src/libxl/libxl_driver.c | 15 ---------------
> >  2 files changed, 20 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index 9c62291..325de79 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1290,6 +1290,26 @@ libxlDomainStart(libxlDriverPrivatePtr driver, 
> > virDomainObjPtr vm,
> >      if (libxlDomainSetVcpuAffinities(driver, vm) < 0)
> >          goto cleanup_dom;
> >  
> > +    if (vm->def->nconsoles) {
> > +        virDomainChrDefPtr chr = vm->def->consoles[0];
> 
> Given vm->def->nconsoles should we loop and do them all?

Maybe. libvirt it self will not be able to access any console that is
not the first one (virDomainOpenConsole only provide access to console
0), but a consumer of libvirt could.

> Also, and I really should know the answer to this (and sorry for not
> thinking of it earlier), but:
> 
> > +            ret = libxl_console_get_tty(priv->ctx, vm->def->id,
> > +                                        chr->target.port, console_type,
> > +                                        &console);
> 
> Might this race against xenconsoled writing the node to xenstore and
> therefore be prone to failing when starting multiple guests all at once?

I've look through out libxl, xenconsoled and libvirt, and I could not
find any synchronisation point. So I guest it's possible.

> Is there a hook which is called on virsh dumpxml which could update
> things on the fly (i.e. lookup the console iff it isn't already set)?

I guest we could modify libxlDomainGetXMLDesc and libxlDomainOpenConsole
to do the check, or having a xenstore watch on the path (if that is
possible).

-- 
Anthony PERARD

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