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@xxxxxxxxxx>
> > ---
> > 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list