On 27/08/15 19:03, Ian Jackson wrote:
Wei Liu writes ("Re: [Xen-devel] [PATCH v2] libxenstore: prefer using the character
device"):
On Thu, Aug 27, 2015 at 09:04:38AM -0500, Jonathan Creekmore wrote:
With the addition of FMODE_ATOMIC_POS in the Linux 3.14 kernel,
concurrent blocking file accesses to a single open file descriptor can
cause a deadlock trying to grab the file position lock. If a watch has
been set up, causing a read_thread to blocking read on the file
descriptor, then future writes that would cause the background read to
complete will block waiting on the file position lock before they can
execute. This race condition only occurs when libxenstore is accessing
the xenstore daemon through the /proc/xen/xenbus file and not through
the unix domain socket, which is the case when the xenstore daemon is
running as a stub domain or when oxenstored is passed
--disable-socket. Accessing the daemon from the true character device
also does not exhibit this problem.
On Linux, prefer using the character device file over the proc file if
the character device exists.
I confess I still see this as working around a kernel bug. Only this
time we are switching from a buggy to non-buggy kernel interface.
/proc/xen/xenbus is deprecated. The tools should use the non-deprecated
interface.
Why don't we have the kernel provide only non-buggy interfaces ?
Fixing /proc/xen/xenbus is non-trival and since there's a fully working
non-deprecated interface (/dev/xen/xenbus), it's unlikely that anyone is
going to be inspired to fix it.
diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c
index af4f75a..0c7744e 100644
--- a/tools/xenstore/xs_lib.c
+++ b/tools/xenstore/xs_lib.c
@@ -81,6 +81,8 @@ const char *xs_domain_dev(void)
#if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__)
return "/dev/xen/xenbus";
#elif defined(__linux__)
+ if (access("/dev/xen/xenbus", F_OK) == 0)
+ return "/dev/xen/xenbus";
Also, previously xs_domain_dev was a function which simply returned a
static value. I feel vaguely uneasy at putting this kind of
autodetection logic here.
"Vaguely uneasy"? Are we engineers or witchdoctors?
xs_domain_dev() already does a system call to query the environment so
it did not just "return a static value":
const char *xs_domain_dev(void)
{
char *s = getenv("XENSTORED_PATH");
if (s)
return s;
...
David
