4:39 p.m.
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(a)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
4:56 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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.
5:16 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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
5:23 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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.
6:07 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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
6:07 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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
10:30 a.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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(a)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.
12:32 p.m.
New subject: [libvirt] [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:
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(a)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
1:36 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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.
1:37 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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.
2:15 p.m.
New subject: [libvirt] [Xen-devel] [PATCH V2] libxl: Set path to console on domain startup.
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.
3567
days inactive
3574
days old
10 comments
3 participants
participants (3)
-
Anthony PERARD -
Ian Campbell -
Ian Jackson