On Thu, Dec 11, 2014 at 04:23:15PM +0000, Ian Campbell wrote: > On Thu, 2014-12-11 at 16:16 +0000, Anthony PERARD wrote: > > 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). > > The aop_console_how option to libxl_domain_create_new and > libxl_domain_create_restore is documented as: > > /* A progress report will be made via ao_console_how, of type > * domain_create_console_available, when the domain's primary > * console is available and can be connected to. > */ > > Which sounds like exactly what is needed? It looks like the progress is reported before the main function finish, from the description of the type libxl_asyncprogress_how (in libxl.h). Also, I tryied to use it, it works if xenconsoled is running. But if xenconsoled is not running, then the callback is also called and libxl_console_get_tty() return an empty string. Or, maybe my test case is not relevant, so another question: Will libxl wait for xenconsoled to process the new domain before calling the callback? Or, should I set the callback to NULL and have the domain_create_console_available event sent through the callback set by libxl_event_register_callbacks()? -- Anthony PERARD -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list