[PATCH] libxl: Set path to console on domain startup

[PATCH] libxl: Set path to console on domain startup

[Anthony PERARD]

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

[PATCH] libxl: Set path to console on domain startup.

[Anthony PERARD]

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@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

Re: [PATCH] libxl: Set path to console on domain startup.

[Don Koch]

Not really familiar with libvirt, but...

On Fri, 5 Dec 2014 16:30:06 +0000
Anthony PERARD <anthony.perard@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@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@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH] libxl: Set path to console on domain startup.

[Ian Campbell]

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@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);

Re: [PATCH] libxl: Set path to console on domain startup.

[Anthony PERARD]

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@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

Re: [PATCH] libxl: Set path to console on domain startup.

[Ian Campbell]

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@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.

Re: [libvirt] [PATCH] libxl: Set path to console on domain startup

[Ján Tomko]

[-- Attachment #1.1: Type: text/plain, Size: 1197 bytes --]

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(+)
> 



[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

Re: [libvirt] [PATCH] libxl: Set path to console on domain startup

[Anthony PERARD]

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

Re: [libvirt] [PATCH] libxl: Set path to console on domain startup

[Jim Fehlig]

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

Re: [libvirt] [PATCH] libxl: Set path to console on domain startup.

[Jim Fehlig]

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@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);
>   

Re: [libvirt] [PATCH] libxl: Set path to console on domain startup.

[Anthony PERARD]

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@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