Libguestfs - Feb 2016 - Re: [PATCH v2 2/2] New API: get-sockdir

Introduce an internal helper to create paths for sockets -- will be
useful for changing later the logic for placing sockets.
Futhermore, check that the length of sockets won't overflow the buffer
for their filenames.
---
 src/guestfs-internal.h |  1 +
 src/launch-direct.c    |  4 +++-
 src/launch-libvirt.c   | 10 ++++++----
 src/launch.c           | 17 +++++++++++++++++
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 5ecd322..bff9f64 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -782,6 +782,7 @@ extern void guestfs_int_launch_send_progress (guestfs_h *g,
int perdozen);
 extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char
*appliance_dev, int flags);
 #define APPLIANCE_COMMAND_LINE_IS_TCG 1
 const char *guestfs_int_get_cpu_model (int kvm);
+int guestfs_int_create_socketname (guestfs_h *g, const char *filename, char
(*sockname)[UNIX_PATH_MAX]);
 extern void guestfs_int_register_backend (const char *name, const struct
backend_ops *);
 extern int guestfs_int_set_backend (guestfs_h *g, const char *method);
 
diff --git a/src/launch-direct.c b/src/launch-direct.c
index b8e453d..a81d4b3 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -295,7 +295,9 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   /* Using virtio-serial, we need to create a local Unix domain socket
    * for qemu to connect to.
    */
-  snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock,
"%s/guestfsd.sock", g->tmpdir);
+  if (guestfs_int_create_socketname (g, "guestfsd.sock",
+                                     &data->guestfsd_sock) == -1)
+    goto cleanup0;
 
   daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (daemon_accept_sock == -1) {
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 8a5d93e..376bd80 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -395,8 +395,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
   /* Using virtio-serial, we need to create a local Unix domain socket
    * for qemu to connect to.
    */
-  snprintf (data->guestfsd_path, sizeof data->guestfsd_path,
-            "%s/guestfsd.sock", g->tmpdir);
+  if (guestfs_int_create_socketname (g, "guestfsd.sock",
+                                     &data->guestfsd_path) == -1)
+    goto cleanup;
 
   set_socket_create_context (g);
 
@@ -421,8 +422,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
   }
 
   /* For the serial console. */
-  snprintf (data->console_path, sizeof data->console_path,
-            "%s/console.sock", g->tmpdir);
+  if (guestfs_int_create_socketname (g, "console.sock",
+                                     &data->console_path) == -1)
+    goto cleanup;
 
   console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (console_sock == -1) {
diff --git a/src/launch.c b/src/launch.c
index f59818f..60f02a7 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -418,6 +418,23 @@ guestfs_int_get_cpu_model (int kvm)
 #endif
 }
 
+/* Create the path for a socket with the selected filename in the
+ * tmpdir.
+ */
+int
+guestfs_int_create_socketname (guestfs_h *g, const char *filename,
+                               char (*sockpath)[UNIX_PATH_MAX])
+{
+  if (strlen (g->tmpdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
+    error (g, _("socket path too long: %s/%s"), g->tmpdir,
filename);
+    return -1;
+  }
+
+  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->tmpdir,
filename);
+
+  return 0;
+}
+
 /* glibc documents, but does not actually implement, a 'getumask(3)'
  * call.  This implements a thread-safe way to get the umask.  Note
  * this is only called when g->verbose is true and after g->tmpdir
-- 
2.5.0

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.
---
 fish/guestfish.pod     | 11 +++++++++++
 generator/actions.ml   | 16 ++++++++++++++++
 src/guestfs-internal.h |  7 ++++++-
 src/guestfs.pod        | 11 +++++++++++
 src/handle.c           |  9 ++++++++-
 src/launch.c           |  9 ++++++---
 src/tmpdirs.c          | 33 +++++++++++++++++++++++++++++++++
 test-tool/test-tool.c  |  6 ++++++
 8 files changed, 97 insertions(+), 5 deletions(-)

diff --git a/fish/guestfish.pod b/fish/guestfish.pod
index c6f5663..bbeea82 100644
--- a/fish/guestfish.pod
+++ b/fish/guestfish.pod
@@ -1519,6 +1519,17 @@ 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 24c6eb7..4078082 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -3504,6 +3504,22 @@ call it returns a simple true/false boolean result,
instead
 of throwing an exception if a feature is not found.  For
 other documentation see C<guestfs_available>." };
 
+  { defaults with
+    name = "get_sockdir"; added = (1, 33, 8);
+    style = RString "sockdir", [], [];
+    blocking = false;
+    shortdesc = "get the temporary directory for sockets";
+    longdesc = "\
+Get the directory used by the handle to store temporary socket files.
+
+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." };
+
 ]
 
 (* daemon_functions are any functions which cause some action
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index bff9f64..6e441e4 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -434,8 +434,10 @@ struct guestfs_h
    * handle, you have to call guestfs_int_lazy_make_tmpdir.
    */
   char *tmpdir;
-  /* Environment variables that affect tmpdir/cachedir locations. */
+  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
*/
 
@@ -757,8 +759,11 @@ 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);
+extern void guestfs_int_remove_sockdir (guestfs_h *g);
 extern void guestfs_int_recursive_remove_dir (guestfs_h *g, const char *dir);
 
 /* whole-file.c */
diff --git a/src/guestfs.pod b/src/guestfs.pod
index a82d060..2a199c0 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -3482,6 +3482,17 @@ 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 947818c..25d3c99 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -273,6 +273,10 @@ 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;
 }
 
@@ -347,8 +351,9 @@ guestfs_close (guestfs_h *g)
   if (g->test_fp != NULL)
     fclose (g->test_fp);
 
-  /* Remove temporary directory. */
+  /* Remove temporary directories. */
   guestfs_int_remove_tmpdir (g);
+  guestfs_int_remove_sockdir (g);
 
   /* Mark the handle as dead and then free up all memory. */
   g->state = NO_HANDLE;
@@ -377,7 +382,9 @@ guestfs_close (guestfs_h *g)
   if (g->pda)
     hash_free (g->pda);
   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/launch.c b/src/launch.c
index 60f02a7..9273c58 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -425,12 +425,15 @@ int
 guestfs_int_create_socketname (guestfs_h *g, const char *filename,
                                char (*sockpath)[UNIX_PATH_MAX])
 {
-  if (strlen (g->tmpdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
-    error (g, _("socket path too long: %s/%s"), g->tmpdir,
filename);
+  if (guestfs_int_lazy_make_sockdir (g) == -1)
+    return -1;
+
+  if (strlen (g->sockdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
+    error (g, _("socket path too long: %s/%s"), g->sockdir,
filename);
     return -1;
   }
 
-  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->tmpdir,
filename);
+  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->sockdir,
filename);
 
   return 0;
 }
diff --git a/src/tmpdirs.c b/src/tmpdirs.c
index 2ae9c74..e66ab9c 100644
--- a/src/tmpdirs.c
+++ b/src/tmpdirs.c
@@ -76,6 +76,12 @@ 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);
@@ -119,6 +125,20 @@ guestfs_impl_get_cachedir (guestfs_h *g)
   return safe_strdup (g, str);
 }
 
+/* Note this actually calculates the sockdir, so it never returns NULL. */
+char *
+guestfs_impl_get_sockdir (guestfs_h *g)
+{
+  const char *str;
+
+  if (g->env_runtimedir)
+    str = g->env_runtimedir;
+  else
+    str = "/tmp";
+
+  return safe_strdup (g, str);
+}
+
 static int
 lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char **dest)
 {
@@ -145,6 +165,12 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g)
   return lazy_make_tmpdir (g, guestfs_get_tmpdir, &g->tmpdir);
 }
 
+int
+guestfs_int_lazy_make_sockdir (guestfs_h *g)
+{
+  return lazy_make_tmpdir (g, guestfs_get_sockdir, &g->sockdir);
+}
+
 /* Recursively remove a temporary directory.  If removal fails, just
  * return (it's a temporary directory so it'll eventually be cleaned
  * up by a temp cleaner).  This is done using "rm -rf" because
that's
@@ -167,3 +193,10 @@ guestfs_int_remove_tmpdir (guestfs_h *g)
   if (g->tmpdir)
     guestfs_int_recursive_remove_dir (g, g->tmpdir);
 }
+
+void
+guestfs_int_remove_sockdir (guestfs_h *g)
+{
+  if (g->sockdir)
+    guestfs_int_recursive_remove_dir (g, g->sockdir);
+}
diff --git a/test-tool/test-tool.c b/test-tool/test-tool.c
index 9d23d6d..495316b 100644
--- a/test-tool/test-tool.c
+++ b/test-tool/test-tool.c
@@ -210,6 +210,9 @@ 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).
@@ -255,6 +258,9 @@ main (int argc, char *argv[])
   printf ("guestfs_get_recovery_proc: %d\n",
guestfs_get_recovery_proc (g));
   printf ("guestfs_get_selinux: %d\n", guestfs_get_selinux (g));
   printf ("guestfs_get_smp: %d\n", guestfs_get_smp (g));
+  p = guestfs_get_sockdir (g);
+  printf ("guestfs_get_sockdir: %s\n", p ? : "(null)");
+  free (p);
   p = guestfs_get_tmpdir (g);
   printf ("guestfs_get_tmpdir: %s\n", p ? : "(null)");
   free (p);
-- 
2.5.0

On Wed, Feb 03, 2016 at 01:17:41PM +0100, Pino Toscano
wrote:
> Introduce an internal helper to create paths for sockets -- will be
> useful for changing later the logic for placing sockets.
> Futhermore, check that the length of sockets won't overflow the buffer
> for their filenames.
> ---
>  src/guestfs-internal.h |  1 +
>  src/launch-direct.c    |  4 +++-
>  src/launch-libvirt.c   | 10 ++++++----
>  src/launch.c           | 17 +++++++++++++++++
>  4 files changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index 5ecd322..bff9f64 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -782,6 +782,7 @@ extern void guestfs_int_launch_send_progress (guestfs_h
*g, int perdozen);
>  extern char *guestfs_int_appliance_command_line (guestfs_h *g, const char
*appliance_dev, int flags);
>  #define APPLIANCE_COMMAND_LINE_IS_TCG 1
>  const char *guestfs_int_get_cpu_model (int kvm);
> +int guestfs_int_create_socketname (guestfs_h *g, const char *filename,
char (*sockname)[UNIX_PATH_MAX]);
>  extern void guestfs_int_register_backend (const char *name, const struct
backend_ops *);
>  extern int guestfs_int_set_backend (guestfs_h *g, const char *method);
>  
> diff --git a/src/launch-direct.c b/src/launch-direct.c
> index b8e453d..a81d4b3 100644
> --- a/src/launch-direct.c
> +++ b/src/launch-direct.c
> @@ -295,7 +295,9 @@ launch_direct (guestfs_h *g, void *datav, const char
*arg)
>    /* Using virtio-serial, we need to create a local Unix domain socket
>     * for qemu to connect to.
>     */
> -  snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock,
"%s/guestfsd.sock", g->tmpdir);
> +  if (guestfs_int_create_socketname (g, "guestfsd.sock",
> +                                     &data->guestfsd_sock) == -1)
> +    goto cleanup0;
>  
>    daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
>    if (daemon_accept_sock == -1) {
> diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
> index 8a5d93e..376bd80 100644
> --- a/src/launch-libvirt.c
> +++ b/src/launch-libvirt.c
> @@ -395,8 +395,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
>    /* Using virtio-serial, we need to create a local Unix domain socket
>     * for qemu to connect to.
>     */
> -  snprintf (data->guestfsd_path, sizeof data->guestfsd_path,
> -            "%s/guestfsd.sock", g->tmpdir);
> +  if (guestfs_int_create_socketname (g, "guestfsd.sock",
> +                                     &data->guestfsd_path) == -1)
> +    goto cleanup;
>  
>    set_socket_create_context (g);
>  
> @@ -421,8 +422,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
>    }
>  
>    /* For the serial console. */
> -  snprintf (data->console_path, sizeof data->console_path,
> -            "%s/console.sock", g->tmpdir);
> +  if (guestfs_int_create_socketname (g, "console.sock",
> +                                     &data->console_path) == -1)
> +    goto cleanup;
>  
>    console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
>    if (console_sock == -1) {
> diff --git a/src/launch.c b/src/launch.c
> index f59818f..60f02a7 100644
> --- a/src/launch.c
> +++ b/src/launch.c
> @@ -418,6 +418,23 @@ guestfs_int_get_cpu_model (int kvm)
>  #endif
>  }
>  
> +/* Create the path for a socket with the selected filename in the
> + * tmpdir.
> + */
> +int
> +guestfs_int_create_socketname (guestfs_h *g, const char *filename,
> +                               char (*sockpath)[UNIX_PATH_MAX])
> +{
> +  if (strlen (g->tmpdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1)
{
> +    error (g, _("socket path too long: %s/%s"), g->tmpdir,
filename);
> +    return -1;
> +  }
> +
> +  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->tmpdir,
filename);
> +
> +  return 0;
> +}
> +
>  /* glibc documents, but does not actually implement, a
'getumask(3)'
>   * call.  This implements a thread-safe way to get the umask.  Note
>   * this is only called when g->verbose is true and after g->tmpdir
Looks good, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

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.
> ---
>  fish/guestfish.pod     | 11 +++++++++++
>  generator/actions.ml   | 16 ++++++++++++++++
>  src/guestfs-internal.h |  7 ++++++-
>  src/guestfs.pod        | 11 +++++++++++
>  src/handle.c           |  9 ++++++++-
>  src/launch.c           |  9 ++++++---
>  src/tmpdirs.c          | 33 +++++++++++++++++++++++++++++++++
>  test-tool/test-tool.c  |  6 ++++++
>  8 files changed, 97 insertions(+), 5 deletions(-)
> 
> diff --git a/fish/guestfish.pod b/fish/guestfish.pod
> index c6f5663..bbeea82 100644
> --- a/fish/guestfish.pod
> +++ b/fish/guestfish.pod
> @@ -1519,6 +1519,17 @@ 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 24c6eb7..4078082 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -3504,6 +3504,22 @@ call it returns a simple true/false boolean result,
instead
>  of throwing an exception if a feature is not found.  For
>  other documentation see C<guestfs_available>." };
>  
> +  { defaults with
> +    name = "get_sockdir"; added = (1, 33, 8);
> +    style = RString "sockdir", [], [];
> +    blocking = false;
> +    shortdesc = "get the temporary directory for sockets";
> +    longdesc = "\
> +Get the directory used by the handle to store temporary socket files.
> +
> +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." };
> +
>  ]
>  
>  (* daemon_functions are any functions which cause some action
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index bff9f64..6e441e4 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -434,8 +434,10 @@ struct guestfs_h
>     * handle, you have to call guestfs_int_lazy_make_tmpdir.
>     */
>    char *tmpdir;
> -  /* Environment variables that affect tmpdir/cachedir locations. */
> +  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 */
>  
> @@ -757,8 +759,11 @@ 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);
> +extern void guestfs_int_remove_sockdir (guestfs_h *g);
>  extern void guestfs_int_recursive_remove_dir (guestfs_h *g, const char
*dir);
>  
>  /* whole-file.c */
> diff --git a/src/guestfs.pod b/src/guestfs.pod
> index a82d060..2a199c0 100644
> --- a/src/guestfs.pod
> +++ b/src/guestfs.pod
> @@ -3482,6 +3482,17 @@ 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 947818c..25d3c99 100644
> --- a/src/handle.c
> +++ b/src/handle.c
> @@ -273,6 +273,10 @@ 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;
>  }
>  
> @@ -347,8 +351,9 @@ guestfs_close (guestfs_h *g)
>    if (g->test_fp != NULL)
>      fclose (g->test_fp);
>  
> -  /* Remove temporary directory. */
> +  /* Remove temporary directories. */
>    guestfs_int_remove_tmpdir (g);
> +  guestfs_int_remove_sockdir (g);
>  
>    /* Mark the handle as dead and then free up all memory. */
>    g->state = NO_HANDLE;
> @@ -377,7 +382,9 @@ guestfs_close (guestfs_h *g)
>    if (g->pda)
>      hash_free (g->pda);
>    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/launch.c b/src/launch.c
> index 60f02a7..9273c58 100644
> --- a/src/launch.c
> +++ b/src/launch.c
> @@ -425,12 +425,15 @@ int
>  guestfs_int_create_socketname (guestfs_h *g, const char *filename,
>                                 char (*sockpath)[UNIX_PATH_MAX])
>  {
> -  if (strlen (g->tmpdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1)
{
> -    error (g, _("socket path too long: %s/%s"), g->tmpdir,
filename);
> +  if (guestfs_int_lazy_make_sockdir (g) == -1)
> +    return -1;
> +
> +  if (strlen (g->sockdir) + 1 + strlen (filename) > UNIX_PATH_MAX-1)
{
> +    error (g, _("socket path too long: %s/%s"), g->sockdir,
filename);
>      return -1;
>    }
>  
> -  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->tmpdir,
filename);
> +  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", g->sockdir,
filename);
>  
>    return 0;
>  }
> diff --git a/src/tmpdirs.c b/src/tmpdirs.c
> index 2ae9c74..e66ab9c 100644
> --- a/src/tmpdirs.c
> +++ b/src/tmpdirs.c
> @@ -76,6 +76,12 @@ 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);
> @@ -119,6 +125,20 @@ guestfs_impl_get_cachedir (guestfs_h *g)
>    return safe_strdup (g, str);
>  }
>  
> +/* Note this actually calculates the sockdir, so it never returns NULL. */
> +char *
> +guestfs_impl_get_sockdir (guestfs_h *g)
> +{
> +  const char *str;
> +
> +  if (g->env_runtimedir)
> +    str = g->env_runtimedir;
> +  else
> +    str = "/tmp";
> +
> +  return safe_strdup (g, str);
> +}
> +
>  static int
>  lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char
**dest)
>  {
> @@ -145,6 +165,12 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g)
>    return lazy_make_tmpdir (g, guestfs_get_tmpdir, &g->tmpdir);
>  }
>  
> +int
> +guestfs_int_lazy_make_sockdir (guestfs_h *g)
> +{
> +  return lazy_make_tmpdir (g, guestfs_get_sockdir, &g->sockdir);
> +}
> +
>  /* Recursively remove a temporary directory.  If removal fails, just
>   * return (it's a temporary directory so it'll eventually be
cleaned
>   * up by a temp cleaner).  This is done using "rm -rf" because
that's
> @@ -167,3 +193,10 @@ guestfs_int_remove_tmpdir (guestfs_h *g)
>    if (g->tmpdir)
>      guestfs_int_recursive_remove_dir (g, g->tmpdir);
>  }
> +
> +void
> +guestfs_int_remove_sockdir (guestfs_h *g)
> +{
> +  if (g->sockdir)
> +    guestfs_int_recursive_remove_dir (g, g->sockdir);
> +}
> diff --git a/test-tool/test-tool.c b/test-tool/test-tool.c
> index 9d23d6d..495316b 100644
> --- a/test-tool/test-tool.c
> +++ b/test-tool/test-tool.c
> @@ -210,6 +210,9 @@ 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).
> @@ -255,6 +258,9 @@ main (int argc, char *argv[])
>    printf ("guestfs_get_recovery_proc: %d\n",
guestfs_get_recovery_proc (g));
>    printf ("guestfs_get_selinux: %d\n", guestfs_get_selinux (g));
>    printf ("guestfs_get_smp: %d\n", guestfs_get_smp (g));
> +  p = guestfs_get_sockdir (g);
> +  printf ("guestfs_get_sockdir: %s\n", p ? :
"(null)");
> +  free (p);
>    p = guestfs_get_tmpdir (g);
>    printf ("guestfs_get_tmpdir: %s\n", p ? : "(null)");
>    free (p);
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

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/