Re: [libvirt] [PATCH RFC] libxl: set serial source path for console type=serial

Joao Martins wrote: > Guests use a <console /> (and sometimes <serial /> pair) to represent > the console. On the callback that is called when console is brought up > (NB: before domain starts), we fill the path of the console element with > the appropriate "/dev/pts/X". For PV guests it all works fine, although > for HVM guests it doesn't. Consequently we end up seeing erronous > behaviour on HVM guests where console path is not displayed on the XML > but still can be accessed with virDomainOpenConsole after booting guest. > Consumers of this XML (e.g. Openstack) also won't be able to use the > console path. Can you provide example input domXML config that causes this problem?

Regards, Jim > Finally, the path set in consoles array won't persist > across daemon reboots, thus rendering admins/users with no access to > console with a message such as: > > "error: operation failed: PTY device is not yet assigned" > > This is because: for HVM guests, DefFormatInternal will ignore the > console element and instead write it with the content of the serial > element for target type = serial which isn't touched in our > libxlConsoleCallback. To address that we introduce a new helper routine > libxlConsoleSetSourcePath() that sets the source path and thus also > handling this case. That is by setting the source path on with serial > element akin to the one indexed by console "port". This way we keep > similar behaviour for PV and HVM guests. > > Signed-off-by: Joao Martins <joao.m.martins(a)oracle.com&gt; > --- > src/libxl/libxl_domain.c | 35 +++++++++++++++++++++++++++++++---- > 1 file changed, 31 insertions(+), 4 deletions(-) > > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index 221af87..4a46143 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -955,6 +955,35 @@ libxlNetworkPrepareDevices(virDomainDefPtr def) > return 0; > } > > +static int > +libxlConsoleSetSourcePath(virDomainDefPtr def, virDomainChrDefPtr console, > + char *path) > +{ > + int ret = -1; > + size_t i; > + > + if (!path || path[0] == '\0') > + return ret; > + > + if (VIR_STRDUP(console->source.data.file.path, path) < 0) > + return ret; > + > + if (console->targetType != VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) > + return 0; > + > + for (i = 0; i < def->nserials; i++) { > + virDomainChrDefPtr serial = def->serials[i]; > + > + if (serial->target.port == console->target.port && > + VIR_STRDUP(serial->source.data.file.path, path) >= 0) { > + ret = 0; > + break; > + } > + } > + > + return ret; > +} > + > static void > libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) > { > @@ -977,10 +1006,8 @@ libxlConsoleCallback(libxl_ctx *ctx, libxl_event *ev, void *for_callback) > &console); > if (!ret) { > VIR_FREE(chr->source.data.file.path); > - if (console && console[0] != '\0') { > - ignore_value(VIR_STRDUP(chr->source.data.file.path, > - console)); > - } > + if (libxlConsoleSetSourcePath(vm->def, chr, console) < 0) > + VIR_WARN("Failed to set console source path"); > } > VIR_FREE(console); > }