On 06/21/2016 04:20 AM, Joao Martins wrote:
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?
Yes, I think it helps clarify the problem being solved by the commit.
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>
I asked for an example after having problems testing your patch, but it turned out to be an issue which I think was introduced long ago by commit 482e5f15. I was testing with transient domains and noticed 'virsh create; 'virsh console' always failed with error: internal error: character device <null> is not using a PTY My config only contained <console type='pty'> <target port='0'/> </console> so the "really crazy backcompat stuff for consoles" in virDomainDefAddConsoleCompat() moved the config over to def->serials[0] and created a new def->consoles[0] without def->consoles[0].source.type set to VIR_DOMAIN_CHR_TYPE_PTY. Without source.type set, libxlConsoleCallback() would ignore the console and not copy the PTY path to source.data.file.path. 'virsh console' would then fail with the error noted above. I spent too much time debugging this, partly because I was confused by odd behavior with persistent domains. E.g. 'virsh define; virsh start; virsh console' would fail with the same error, but all subsequent 'virsh shutdown; virsh start; virsh console' sequences would work :-). That behavior was due to vm->newDef being populated with correct console config when calling virDomainObjSetDefTransient() in libxlDomainStart(). After the first shutdown of the domain, vm->newDef would be copied over to vm->def, allowing subsequent 'virsh start; virsh console' to work correctly. Long story short, I found the problem but not sure how to fix it :-). One approach would be the below patch. Another would be to loosen the restriction in libxlConsoleCallback, filing in source.data.file.path when the console source type is also set to VIR_DOMAIN_CHR_TYPE_NULL. Adding Cole and Peter (both of which have toiled in and likely loathe this code) to see if they have opinions on a proper fix. Regards, Jim
From a231636e8eec08152220e32af5f4026a50954f41 Mon Sep 17 00:00:00 2001 From: Jim Fehlig <jfehlig@suse.com> Date: Tue, 21 Jun 2016 20:25:23 -0600 Subject: [PATCH] domain conf: copy source type from stolen console
When domXML contains only <console type='pty'> and no corresponding <serial>, the console is "stolen" [1] and used as the first <serial> device. A new console is created as an alias to the first <serial> device, but missed copying some configuration such as the source 'type' attribute. [1] See comments associated with virDomainDefAddConsoleCompat() in $LIBVIRT-SRC/src/conf/domain_conf.c: Signed-off-by: Jim Fehlig <jfehlig@suse.com> --- src/conf/domain_conf.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 75ad03f..2fda4fe 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -3845,6 +3845,7 @@ virDomainDefAddConsoleCompat(virDomainDefPtr def) /* Create an console alias for the serial port */ def->consoles[0]->deviceType = VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE; def->consoles[0]->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL; + def->consoles[0]->source.type = def->serials[0]->source.type; } } else if (def->os.type == VIR_DOMAIN_OSTYPE_HVM && def->nserials > 0 && def->serials[0]->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && -- 2.1.4