10:30 a.m.
Hi,
I'm trying to fix an issue when using OpenStack with libvirt+xen (libxenlight).
OpenStack cannot access the console output of a Xen PV guest, because the XML
generated by libvirt for a domain is missing the path to the pty. The path
actually appear in the XML once one call virDomainOpenConsole().
The patch intend to get the path to the pty without having to call
virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a
few question:
Is libxlDomainStart will be called on restore/migrate/reboot?
I guest the function libxlDomainOpenConsole() would not need to do the same
work if the console path is settup properly.
There is a bug report about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1170743
Regards,
Anthony PERARD (1):
libxl: Set path to console on domain startup.
src/libxl/libxl_domain.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
--
Anthony PERARD
10:30 a.m.
New subject: [libvirt] [PATCH] 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 get the tty path on the first call of GetXMLDesc.
Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
---
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));
+ VIR_FREE(console);
+ }
+ }
+
if (!start_paused) {
libxl_domain_unpause(priv->ctx, domid);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
--
Anthony PERARD
1:08 p.m.
New subject: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
Not really familiar with libvirt, but...
On Fri, 5 Dec 2014 16:30:06 +0000
Anthony PERARD <anthony.perard(a)citrix.com> 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 get the tty path on the first call of GetXMLDesc.
Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
---
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;
Pointless initializer. Possibly combine with following statement.
-d
+ 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));
+ VIR_FREE(console);
+ }
+ }
+
if (!start_paused) {
libxl_domain_unpause(priv->ctx, domid);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel(a)lists.xen.org
http://lists.xen.org/xen-devel
5:59 a.m.
New subject: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
On Fri, 2014-12-05 at 16:30 +0000, Anthony PERARD wrote:
Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
(I've done so here...)
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 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).
Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
---
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.
+ VIR_FREE(console);
+ }
+ }
+
if (!start_paused) {
libxl_domain_unpause(priv->ctx, domid);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
9:11 a.m.
New subject: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
On Mon, Dec 08, 2014 at 11:59:44AM +0000, Ian Campbell wrote:
On Fri, 2014-12-05 at 16:30 +0000, Anthony PERARD wrote:
Jim Fehlig maintains the libxl driver in libvirt, so you should CC him
(I've done so here...)
Thanks.
> 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 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.
>
> Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
> ---
> 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.
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.
> + VIR_FREE(console);
> + }
> + }
> +
> if (!start_paused) {
> libxl_domain_unpause(priv->ctx, domid);
> virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
--
Anthony PERARD
9:14 a.m.
New subject: [libvirt] [Xen-devel] [PATCH] libxl: Set path to console on domain startup.
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(a)citrix.com>
> > ---
> > 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.
1:03 p.m.
New subject: [libvirt] [PATCH] libxl: Set path to console on domain startup.
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.
Hi Anthony,
Thanks for the patch. Can you address the comments made by others, my
comments below, and provide a V2?
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 get the tty path on the first call of GetXMLDesc.
Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
---
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));
VIR_STRDUP will not free an existing value. You should VIR_FREE first,
which btw can handle a NULL argument. And since you're initializing
source.data.file.path when starting the domain, I think we can drop the
similar code in libxlDomainOpenConsole().
Regards,
Jim
+ VIR_FREE(console);
+ }
+ }
+
if (!start_paused) {
libxl_domain_unpause(priv->ctx, domid);
virDomainObjSetState(vm, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_BOOTED);
5:47 a.m.
New subject: [libvirt] [PATCH] libxl: Set path to console on domain startup.
On Mon, Dec 08, 2014 at 12:03:36PM -0700, Jim Fehlig wrote:
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.
>
Hi Anthony,
Thanks for the patch. Can you address the comments made by others, my
comments below, and provide a V2?
Will do.
> 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 get the tty path on the first call of GetXMLDesc.
>
> Signed-off-by: Anthony PERARD <anthony.perard(a)citrix.com>
> ---
> 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));
>
VIR_STRDUP will not free an existing value. You should VIR_FREE first,
which btw can handle a NULL argument. And since you're initializing
source.data.file.path when starting the domain, I think we can drop the
similar code in libxlDomainOpenConsole().
I will do that.
Thanks,
--
Anthony PERARD
9:40 a.m.
On 12/05/2014 05:30 PM, Anthony PERARD wrote:
Hi,
I'm trying to fix an issue when using OpenStack with libvirt+xen (libxenlight).
OpenStack cannot access the console output of a Xen PV guest, because the XML
generated by libvirt for a domain is missing the path to the pty. The path
actually appear in the XML once one call virDomainOpenConsole().
The patch intend to get the path to the pty without having to call
virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a
few question:
Is libxlDomainStart will be called on restore/migrate/reboot?
I guest the function libxlDomainOpenConsole() would not need to do the same
work if the console path is settup properly.
There is a bug report about this:
https://bugzilla.redhat.com/show_bug.cgi?id=1170743
Hi,
you can leave the bugzilla link in the commit message, if somebody ever needs
more context.
(And the patch looks good to me, but I'm no libxl expert)
Jan
Regards,
Anthony PERARD (1):
libxl: Set path to console on domain startup.
src/libxl/libxl_domain.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
10:41 a.m.
On Mon, Dec 08, 2014 at 04:40:04PM +0100, Ján Tomko wrote:
On 12/05/2014 05:30 PM, Anthony PERARD wrote:
> Hi,
>
> I'm trying to fix an issue when using OpenStack with libvirt+xen (libxenlight).
> OpenStack cannot access the console output of a Xen PV guest, because the XML
> generated by libvirt for a domain is missing the path to the pty. The path
> actually appear in the XML once one call virDomainOpenConsole().
>
> The patch intend to get the path to the pty without having to call
> virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a
> few question:
> Is libxlDomainStart will be called on restore/migrate/reboot?
> I guest the function libxlDomainOpenConsole() would not need to do the same
> work if the console path is settup properly.
>
> There is a bug report about this:
> https://bugzilla.redhat.com/show_bug.cgi?id=1170743
>
Hi,
you can leave the bugzilla link in the commit message, if somebody ever needs
more context.
Ok.
(And the patch looks good to me, but I'm no libxl expert)
Thanks,
--
Anthony PERARD
12:55 p.m.
Anthony PERARD wrote:
Hi,
I'm trying to fix an issue when using OpenStack with libvirt+xen (libxenlight).
OpenStack cannot access the console output of a Xen PV guest, because the XML
generated by libvirt for a domain is missing the path to the pty. The path
actually appear in the XML once one call virDomainOpenConsole().
The patch intend to get the path to the pty without having to call
virDomainOpenConsole, so I've done the work in libxlDomainStart(). So I have a
few question:
Is libxlDomainStart will be called on restore/migrate/reboot?
Yes.
I guest the function libxlDomainOpenConsole() would not need to do
the same
work if the console path is settup properly.
Yes, agreed.
Regards,
Jim
3889
days inactive
3893
days old
10 comments
5 participants
participants (5)
-
Anthony PERARD -
Don Koch -
Ian Campbell -
Jim Fehlig -
Ján Tomko