Re: [Xen-devel] Race condition in console_available callback? (libvirt + libxl + xenconsoled)

From: Ian Jackson <ian.jackson@citrix.com>
To: Pawel Marczewski <pawel@invisiblethingslab.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	"Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Subject: Re: [Xen-devel] Race condition in console_available callback? (libvirt + libxl + xenconsoled)
Date: Tue, 18 Feb 2020 15:12:41 +0000	[thread overview]
Message-ID: <24139.65257.426159.314189@mariner.uk.xensource.com> (raw)
In-Reply-To: <fdc00be9-11c9-ffbf-af4f-fa22baed9d42@invisiblethingslab.com>

Pawel Marczewski writes ("[Xen-devel] Race condition in console_available callback? (libvirt + libxl + xenconsoled)"):
> I am trying to debug an issue in QubesOS where a domain created by
> libvirt often does not have information stored about the console TTY path.

Hi.  Marek drew my attention to this.

> The relevant part of libvirt creates a domain using
> libxl_domain_create_new() and registers a callback (aop_console_how)
> that is supposed to fire when the console is available. The callback
> then calls libxl_console_get_tty(), but that fails with:
> 
> 2020-01-06 11:52:30.952+0000: libxl: libxl.c:1853:libxl_console_get_tty:
> unable to read console tty path `/local/domain/4/console/tty': Resource
> temporarily unavailable

Hmmm.

> Based on my reading of the libxl code, it's supposed to set the path in
> xenstore and then call the console_available callback, but only if the
> bootloader is configured. Otherwise, we call console_available at a
> later point (in domcreate_attach_devices()) and the path in xenstore is
> being set by xenconsoled independently.

Yes.

> However, there is no guarantee that xenconsoled will do that before we
> call console_available. And indeed, looking at the traces from
> xenstored, the read and write of `.../console/tty` are ordered randomly
> depending on the machine.

Oh dear.

> Should libxl wait for the information appearing in '.../console/tty' at
> this point? Perhaps similar as the code I see in xenconsoled client
> (xen/tools/console/client.c)?

Yes, I think it should.

Looking at my copy of libxl_bootloader.c, I wonder if the same race
exists, in some form, in the with-bootloader case ?  I find the code
here confusing (and yes, I wrote the libxl code, although I didn't
design the xenconsoled protocol, which is quite odd).

> I would be happy to work on a patch but I'm unfamiliar with the project
> so I want to check my assumptions.

That makes sense.  I hope what I have said is helpful.  Feel free to
come back with more questions (or to bother me on irc).

Regards,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel