On 06/21/2016 01:38 AM, Jim Fehlig wrote:
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?
Ah yes - I probably should include that in the commit description? So, for PV guests for an input console XML element: <console type='pty'> <target type='xen' port='0'/> </console> Which then on domain start gets filled like: <console type='pty' tty='/dev/pts/3'> <source path='/dev/pts/3'/> <target type='xen' port='0'/> </console> Although for HVM guests we have: <serial type='pty'> <target port='0'/> </serial> <console type='pty'> <target type='serial' port='0'/> </console> And no changes happen i.e. source path isn't written there _even though_ it's set on the console element - being the reason why we can access the console in the first place. IIUC The expected output should be: <serial type='pty'> <source path='/dev/pts/4'/> <target port='0'/> </serial> <console type='pty' tty='/dev/pts/4'> <source path='/dev/pts/4'/> <target type='serial' port='0'/> </console> Joao
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@oracle.com> --- 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); }