On Wed, Feb 03, 2016 at 01:17:42PM +0100, Pino Toscano wrote: > Introduce a new read-only API to get a path where to store temporary > sockets: this is different from tmpdir, as we need short paths for > sockets (due to sockaddr_un::sun_path), and it is either > XDG_RUNTIME_DIR if set, or /tmp; adapt guestfs_int_create_socketname > to create sockets in that location. > > Furthermore, print sockdir and XDG_RUNTIME_DIR in test-tool for > debugging.
As you saw, there were a few problems with this patch. However I also found something more fundamental. On machines where XDG_RUNTIME_DIR is set to /run/user/$UID, it fails badly when run as root: Original error from libvirt: internal error: process exited while connecting to monitor: 2016-02-08T19:17:42.375986Z qemu-system-x86_64: -chardev socket,id=charserial0,path=/run/user/0/libguestfsdittS9/console.sock: Failed to connect socket: Permission denied [code=1 int1=-1] This is because libvirt runs the appliance as qemu.qemu, which cannot access /run/user/0 (mode 0700). This is the default configuration when accessing a remote machine using `ssh root@remote virt-tool ...' I think we should drop all references to XDG_RUNTIME_DIR (as I noted before, I don't have a high regard for freedesktop pseudo-standards, which I believe are just a way for some people to "policy launder" junk into Linux). The attached patch does that. Note the get-sockdir function now returns the hard-coded value "/tmp", which may or may not be a good idea. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://people.redhat.com/~rjones/virt-df/
>From 4a013c0fe6163acf0f5cc62a59dd93b8f5b80383 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" <[email protected]> Date: Mon, 8 Feb 2016 19:32:06 +0000 Subject: [PATCH] lib: Remove references to XDG_RUNTIME_DIR. When accessing a machine using `ssh root@remote virt-tool ...', XDG_RUNTIME_DIR is set to /run/user/0. This directory has mode 0755. Libvirt runs the appliance as qemu.qemu, and it cannot access any files created in this directory. Fixed commit 55202a4d49a101392148d79cb2e1591428db2681. --- fish/guestfish.pod | 11 ----------- generator/actions.ml | 4 +--- src/guestfs-internal.h | 2 -- src/guestfs.pod | 11 ----------- src/handle.c | 5 ----- src/tmpdirs.c | 11 +---------- test-tool/test-tool.c | 3 --- 7 files changed, 2 insertions(+), 45 deletions(-) diff --git a/fish/guestfish.pod b/fish/guestfish.pod index bbeea82..c6f5663 100644 --- a/fish/guestfish.pod +++ b/fish/guestfish.pod @@ -1519,17 +1519,6 @@ about kernel selection, see L<supermin(1)>. See L</LIBGUESTFS_CACHEDIR>, L</LIBGUESTFS_TMPDIR>. -=item XDG_RUNTIME_DIR - -This directory represents a user-specific directory for storing -non-essential runtime files. - -If it is set, then is used to store temporary sockets. Otherwise, -F</tmp> is used. - -See also L</get-sockdir>, -L<http://www.freedesktop.org/wiki/Specifications/basedir-spec/>. - =back =head1 FILES diff --git a/generator/actions.ml b/generator/actions.ml index eb45392..756a09d 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -3516,9 +3516,7 @@ This is different from C<guestfs_tmpdir>, as we need shorter paths for sockets (due to the limited buffers of filenames for UNIX sockets), and C<guestfs_tmpdir> may be too long for them. -The environment variable C<XDG_RUNTIME_DIR> controls the default -value: If C<XDG_RUNTIME_DIR> is set, then that is the default. -Else F</tmp> is the default." }; +F</tmp> is the default." }; ] diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 22b6c6c..f776b84 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -437,7 +437,6 @@ struct guestfs_h char *sockdir; /* Environment variables that affect tmpdir/cachedir/sockdir locations. */ char *env_tmpdir; /* $TMPDIR (NULL if not set) */ - char *env_runtimedir; /* $XDG_RUNTIME_DIR (NULL if not set)*/ char *int_tmpdir; /* $LIBGUESTFS_TMPDIR or guestfs_set_tmpdir or NULL */ char *int_cachedir; /* $LIBGUESTFS_CACHEDIR or guestfs_set_cachedir or NULL */ @@ -779,7 +778,6 @@ extern void guestfs_int_call_callbacks_array (guestfs_h *g, uint64_t event, cons /* tmpdirs.c */ extern int guestfs_int_set_env_tmpdir (guestfs_h *g, const char *tmpdir); -extern int guestfs_int_set_env_runtimedir (guestfs_h *g, const char *runtimedir); extern int guestfs_int_lazy_make_tmpdir (guestfs_h *g); extern int guestfs_int_lazy_make_sockdir (guestfs_h *g); extern void guestfs_int_remove_tmpdir (guestfs_h *g); diff --git a/src/guestfs.pod b/src/guestfs.pod index 2a199c0..a82d060 100644 --- a/src/guestfs.pod +++ b/src/guestfs.pod @@ -3482,17 +3482,6 @@ about kernel selection, see L<supermin(1)>. See L</LIBGUESTFS_CACHEDIR>, L</LIBGUESTFS_TMPDIR>. -=item XDG_RUNTIME_DIR - -This directory represents a user-specific directory for storing -non-essential runtime files. - -If it is set, then is used to store temporary sockets. Otherwise, -F</tmp> is used. - -See also L</get-sockdir>, -L<http://www.freedesktop.org/wiki/Specifications/basedir-spec/>. - =back =head1 SEE ALSO diff --git a/src/handle.c b/src/handle.c index 25d3c99..cf00f3b 100644 --- a/src/handle.c +++ b/src/handle.c @@ -273,10 +273,6 @@ parse_environment (guestfs_h *g, return -1; } - str = do_getenv (data, "XDG_RUNTIME_DIR"); - if (guestfs_int_set_env_runtimedir (g, str) == -1) - return -1; - return 0; } @@ -384,7 +380,6 @@ guestfs_close (guestfs_h *g) free (g->tmpdir); free (g->sockdir); free (g->env_tmpdir); - free (g->env_runtimedir); free (g->int_tmpdir); free (g->int_cachedir); free (g->last_error); diff --git a/src/tmpdirs.c b/src/tmpdirs.c index e66ab9c..46e9ca8 100644 --- a/src/tmpdirs.c +++ b/src/tmpdirs.c @@ -76,12 +76,6 @@ guestfs_int_set_env_tmpdir (guestfs_h *g, const char *tmpdir) } int -guestfs_int_set_env_runtimedir (guestfs_h *g, const char *runtimedir) -{ - return set_abs_path (g, runtimedir, &g->env_runtimedir); -} - -int guestfs_impl_set_tmpdir (guestfs_h *g, const char *tmpdir) { return set_abs_path (g, tmpdir, &g->int_tmpdir); @@ -131,10 +125,7 @@ guestfs_impl_get_sockdir (guestfs_h *g) { const char *str; - if (g->env_runtimedir) - str = g->env_runtimedir; - else - str = "/tmp"; + str = "/tmp"; return safe_strdup (g, str); } diff --git a/test-tool/test-tool.c b/test-tool/test-tool.c index 495316b..3e58cd6 100644 --- a/test-tool/test-tool.c +++ b/test-tool/test-tool.c @@ -210,9 +210,6 @@ main (int argc, char *argv[]) p = getenv ("PATH"); if (p) printf ("PATH=%s\n", p); - p = getenv ("XDG_RUNTIME_DIR"); - if (p) - printf ("XDG_RUNTIME_DIR=%s\n", p); /* Print SELinux mode (don't worry if this fails, or if the command * doesn't even exist). -- 2.7.0
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
