[libvirt] [PATCH V2] libxl: Set path to console on domain startup.
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@citrix.com> --- 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]; + 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) { + VIR_FREE(chr->source.data.file.path); + ignore_value(VIR_STRDUP(chr->source.data.file.path, console)); + } + VIR_FREE(console); + } + } + if (!start_paused) { libxl_domain_unpause(priv->ctx, domid); virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 53c87ce..e79afac 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3957,10 +3957,8 @@ libxlDomainOpenConsole(virDomainPtr dom, { virDomainObjPtr vm = NULL; int ret = -1; - libxl_console_type console_type; virDomainChrDefPtr chr = NULL; libxlDomainObjPrivatePtr priv; - char *console = NULL; virCheckFlags(VIR_DOMAIN_CONSOLE_FORCE, -1); @@ -4002,18 +4000,6 @@ libxlDomainOpenConsole(virDomainPtr dom, goto cleanup; } - 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) - goto cleanup; - - if (VIR_STRDUP(chr->source.data.file.path, console) < 0) - goto cleanup; - /* handle mutually exclusive access to console devices */ ret = virChrdevOpen(priv->devs, &chr->source, @@ -4027,7 +4013,6 @@ libxlDomainOpenConsole(virDomainPtr dom, } cleanup: - VIR_FREE(console); if (vm) virObjectUnlock(vm); return ret; -- Anthony PERARD
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@citrix.com>
--- 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? 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? 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)? Ian.
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@citrix.com>
--- 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
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@citrix.com>
--- 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? Ian.
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@citrix.com>
--- 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 look like it. If I find how to use that. -- Anthony PERARD
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@citrix.com>
--- 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
On Mon, 2014-12-15 at 17:07 +0000, Anthony PERARD wrote:
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@citrix.com>
--- 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).
Correct.
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.
I'm not sure xenconsoled not running is a configuration we need to worry about, or at least it could be expected not to get a console in that case. Unless by "not running" you meant bottlenecked or not keeping up perhaps.
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?
I don't see an explicit wait in the code, but I don't know if it has arranged the sequencing some other way.
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()?
That would make sense, except I don't see libxl_evdisable_foo for these events, so I'm not sure it is possible. Ian.
On Tue, Dec 16, 2014 at 09:30:28AM +0000, Ian Campbell wrote:
On Mon, 2014-12-15 at 17:07 +0000, Anthony PERARD wrote:
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@citrix.com>
--- 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).
Correct.
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.
I'm not sure xenconsoled not running is a configuration we need to worry about, or at least it could be expected not to get a console in that case.
Unless by "not running" you meant bottlenecked or not keeping up perhaps.
Well, I did meant no xenconsoled process. But after, I also tried `kill -STOP`, but the same things is happening.
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?
I don't see an explicit wait in the code, but I don't know if it has arranged the sequencing some other way.
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()?
That would make sense, except I don't see libxl_evdisable_foo for these events, so I'm not sure it is possible.
Well, the domain_create_console_available event is report by libxl__ao_progress_report which will either callback() or call libxl__event_occurred(). So does not seams better to set callback to NULL. So, from this, I think I'm going to stick with the original patch and add some hooks in GetXMLDesc and OpenConsole. -- Anthony PERARD
Anthony PERARD writes ("Re: [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup."):
On Tue, Dec 16, 2014 at 09:30:28AM +0000, Ian Campbell wrote:
Unless by "not running" you meant bottlenecked or not keeping up perhaps.
Well, I did meant no xenconsoled process. But after, I also tried `kill -STOP`, but the same things is happening.
I think this is a bug. I imagine that you can cause `xl create -c' to malfunction too.
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()?
That would make sense, except I don't see libxl_evdisable_foo for these events, so I'm not sure it is possible.
Well, the domain_create_console_available event is report by libxl__ao_progress_report which will either callback() or call libxl__event_occurred(). So does not seams better to set callback to NULL.
The console available notification is only available via the ao progress mechanism. So you have to pass a non-NULL aop_console_how, and the notification will always be generated during the create. Your options for having the notification delivered are: I. aop_console_how->callback != NULL: libxl will call aop_console_how->callback(aop_console_how->for_callback) II. aop_console_how->callback == NULL: libxl will construct an libxl_event *event with event->domid = the domid of the domain being constructed event->domuuid = its uuid event->for_user = aop_console_how->for_event event->type = LIBXL_EVENT_TYPE_DOMAIN_CREATE_CONSOLE_AVAILABLE What happens to that event depends on whether and how libxl_event_register_callbacks has been called. II(a): If libxl_event_register_callbacks(,hooks,user) was called and also the bit is set ie !!(hooks->event_occurs_mask & (1UL << LIBXL_EVENT_TYPE_DOMAIN_CREATE_CONSOLE_AVAILABLE)): libxl will call hooks->event_occurs(user,event) II(b): If libxl_event_register_callbacks(,hooks,user) was NOT called or if the bit is clear: libxl will queue the event for retrieval by libxl_event_check or libxl_event-wait. (And if the application doesn't call those, the application will never be notified; the event will be retained until the libxl ctx is destroyed and then discarded.) I mention all of these for completeness, and for future reference for anyone reading the archives later. In libxl you want option I. Ian.
Ian Jackson writes ("Re: [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup."):
I mention all of these for completeness, and for future reference for anyone reading the archives later.
In libxl you want option I.
I mean `in xl you want option I'. In libvirt you probably want that too. Ian.
On Tue, 2014-12-16 at 12:36 +0000, Ian Jackson wrote:
Anthony PERARD writes ("Re: [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup."):
On Tue, Dec 16, 2014 at 09:30:28AM +0000, Ian Campbell wrote:
Unless by "not running" you meant bottlenecked or not keeping up perhaps.
Well, I did meant no xenconsoled process. But after, I also tried `kill -STOP`, but the same things is happening.
I think this is a bug. I imagine that you can cause `xl create -c' to malfunction too.
Presumably the libxl side fix is to register an ev_xswatch (and an ev_timeout) on the console path (which differs by guest type, see guts of libxl_console_get_tty) and call the callback out of that instead of directly at the end of domcreate_.... Depending on how racy this is (not very, I suspect) libvirt could ignore this possibility and assume the event works (and we'll independently fix it so it always does), or it could be a bit belt and braces and in addition to handling the callback also check for NULL and-recheck on the libvirt internal accessors. Anthony, you should also note that the callback can happen more than once, e.g. if you are using pygrub you get it once for the bootloader pty and then again for the actual guests pty. Ian.
participants (3)
-
Anthony PERARD -
Ian Campbell -
Ian Jackson