On 06/22/2016 03:45 PM, Jim Fehlig wrote: > When domXML contains only <console type='pty'> and no corresponding > <serial>, the console is "stolen" [1] and used as the first <serial> > device. When this "stolen" console is accessed from the libxl driver > (in libxlConsoleCallback and libxlDomainOpenConsole), check if the > targetType is VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL, and use the > "stolen" device in def->serials[0] instead. Prior to this change, > creating a domain with input XML containing only a <console> device > and subsequently attempting to access its console with > 'virsh console' would fail > > error: internal error: character device <null> is not using a PTY > > [1] See comments associated with virDomainDefAddConsoleCompat() in > $LIBVIRT-SRC/src/conf/domain_conf.c: > > Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; > --- > src/libxl/libxl_domain.c | 8 ++++++-- > src/libxl/libxl_driver.c | 5 ++++- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index 221af87..6bcb4d9 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -964,13 +964,17 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) > virObjectLock(vm); > for (i = 0; i < vm->def->nconsoles; i++) { > virDomainChrDefPtr chr = vm->def->consoles[i]; > - if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { > + if (i == 0 && > + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) > + chr = vm->def->serials[0]; > + > + if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { > libxl_console_type console_type; > char *console = NULL; > int ret; > > console_type = > - (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ? > + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ? > LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV); > ret = libxl_console_get_tty(ctx, ev->domid, > chr->target.port, console_type, > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index c8c1f07..3189f1c 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -4443,8 +4443,11 @@ libxlDomainOpenConsole(virDomainPtr dom, > > priv = vm->privateData; > > - if (vm->def->nconsoles) > + if (vm->def->nconsoles) { > chr = vm->def->consoles[0]; > + if (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) > + chr = vm->def->serials[0]; > + } > > if (!chr) { > virReportError(VIR_ERR_INTERNAL_ERROR, > ACK (if my ACK is sufficient for functional libxl changes :) )

On 06/22/2016 10:18 PM, Jim Fehlig wrote: > On 06/22/2016 02:03 PM, Cole Robinson wrote: >> On 06/22/2016 03:45 PM, Jim Fehlig wrote: >>> When domXML contains only <console type='pty'> and no corresponding >>> <serial>, the console is "stolen" [1] and used as the first <serial> >>> device. When this "stolen" console is accessed from the libxl driver >>> (in libxlConsoleCallback and libxlDomainOpenConsole), check if the >>> targetType is VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL, and use the >>> "stolen" device in def->serials[0] instead. Prior to this change, >>> creating a domain with input XML containing only a <console> device >>> and subsequently attempting to access its console with >>> 'virsh console' would fail >>> >>> error: internal error: character device <null> is not using a PTY >>> >>> [1] See comments associated with virDomainDefAddConsoleCompat() in >>> $LIBVIRT-SRC/src/conf/domain_conf.c: >>> >>> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com&gt; >>> --- >>> src/libxl/libxl_domain.c | 8 ++++++-- >>> src/libxl/libxl_driver.c | 5 ++++- >>> 2 files changed, 10 insertions(+), 3 deletions(-) >>> >>> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c >>> index 221af87..6bcb4d9 100644 >>> --- a/src/libxl/libxl_domain.c >>> +++ b/src/libxl/libxl_domain.c >>> @@ -964,13 +964,17 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) >>> virObjectLock(vm); >>> for (i = 0; i < vm->def->nconsoles; i++) { >>> virDomainChrDefPtr chr = vm->def->consoles[i]; >>> - if (chr && chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { >>> + if (i == 0 && >>> + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) >>> + chr = vm->def->serials[0]; >>> + >>> + if (chr->source.type == VIR_DOMAIN_CHR_TYPE_PTY) { >>> libxl_console_type console_type; >>> char *console = NULL; >>> int ret; >>> >>> console_type = >>> - (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL ? >>> + (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL ? >>> LIBXL_CONSOLE_TYPE_SERIAL : LIBXL_CONSOLE_TYPE_PV); >>> ret = libxl_console_get_tty(ctx, ev->domid, >>> chr->target.port, console_type, >>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >>> index c8c1f07..3189f1c 100644 >>> --- a/src/libxl/libxl_driver.c >>> +++ b/src/libxl/libxl_driver.c >>> @@ -4443,8 +4443,11 @@ libxlDomainOpenConsole(virDomainPtr dom, >>> >>> priv = vm->privateData; >>> >>> - if (vm->def->nconsoles) >>> + if (vm->def->nconsoles) { >>> chr = vm->def->consoles[0]; >>> + if (chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) >>> + chr = vm->def->serials[0]; >>> + } >>> >>> if (!chr) { >>> virReportError(VIR_ERR_INTERNAL_ERROR, >>> >> ACK (if my ACK is sufficient for functional libxl changes :) ) > Thanks, I've pushed this. You recognized the convention and provided half the > patch, so I think it is sufficient :). Saw this a little late - but it seems that you shot two birds with this single patch. Since we're now effectively changing the serials as opposed the consoles def (as per Cole mentioned convention) you also ended up fixing the bug about the source path not being in the XML. So I am dropping my patch [0] :) Thanks!

Not sure about this, but it appears this patch could be -maint material? IIUC, these bugs have been here for a fair amount of releases potentially dating back more than one year.

Joao [0] https://www.redhat.com/archives/libvir-list/2016-June/msg01312.html