Libguestfs - Feb 2016 - Re: [PATCH 1/3] launch: add internal helper for socket paths creation

Introduce an internal helper to create paths for sockets; will be useful
for changing later the logic for placing sockets.
---
 src/guestfs-internal.h |  1 +
 src/launch-direct.c    |  4 +++-
 src/launch-libvirt.c   | 10 ++++++----
 src/launch.c           | 15 +++++++++++++++
 4 files changed, 25 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..ec061e3 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -418,6 +418,21 @@ 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])
+{
+  char *path = g->tmpdir;
+
+  snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", path, filename);
+  (*sockpath)[UNIX_PATH_MAX-1] = '\0';
+
+  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

Extract the bit of code for lazy creation of a temporary directory, so
it can be used for more directories as well.

This is just code motion, with no behaviour changes.
---
 src/tmpdirs.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/tmpdirs.c b/src/tmpdirs.c
index 9154d8b..2ae9c74 100644
--- a/src/tmpdirs.c
+++ b/src/tmpdirs.c
@@ -119,6 +119,22 @@ guestfs_impl_get_cachedir (guestfs_h *g)
   return safe_strdup (g, str);
 }
 
+static int
+lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char **dest)
+{
+  if (!*dest) {
+    CLEANUP_FREE char *tmpdir = getdir (g);
+    char *tmppath = safe_asprintf (g, "%s/libguestfsXXXXXX", tmpdir);
+    if (mkdtemp (tmppath) == NULL) {
+      perrorf (g, _("%s: cannot create temporary directory"),
tmppath);
+      free (tmpdir);
+      return -1;
+    }
+    *dest = tmppath;
+  }
+  return 0;
+}
+
 /* The g->tmpdir (per-handle temporary directory) is not created when
  * the handle is created.  Instead we create it lazily before the
  * first time it is used, or during launch.
@@ -126,17 +142,7 @@ guestfs_impl_get_cachedir (guestfs_h *g)
 int
 guestfs_int_lazy_make_tmpdir (guestfs_h *g)
 {
-  if (!g->tmpdir) {
-    CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
-    g->tmpdir = safe_asprintf (g, "%s/libguestfsXXXXXX", tmpdir);
-    if (mkdtemp (g->tmpdir) == NULL) {
-      perrorf (g, _("%s: cannot create temporary directory"),
g->tmpdir);
-      free (g->tmpdir);
-      g->tmpdir = NULL;
-      return -1;
-    }
-  }
-  return 0;
+  return lazy_make_tmpdir (g, guestfs_get_tmpdir, &g->tmpdir);
 }
 
 /* Recursively remove a temporary directory.  If removal fails, just
-- 
2.5.0

Introduce a new read-only API to get a path where to store temporary
sockets: this is different than tmpdir, as we need short paths for
sockets (due to sockaddr_un::sun_path), and is either XDG_RUNTIME_DIR
if set, or /tmp.

Adapt guestfs_int_create_socketname to create sockets in that location,
checking whether the resulting path still fits in the limited buffer.

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/handle.c           |  9 ++++++++-
 src/launch.c           | 10 ++++++++--
 src/tmpdirs.c          | 33 +++++++++++++++++++++++++++++++++
 test-tool/test-tool.c  |  6 ++++++
 7 files changed, 88 insertions(+), 4 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..5b6898c 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 than 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/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 ec061e3..e6972d1 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -425,9 +425,15 @@ int
 guestfs_int_create_socketname (guestfs_h *g, const char *filename,
                                char (*sockpath)[UNIX_PATH_MAX])
 {
-  char *path = g->tmpdir;
+  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-1, "%s/%s", path, filename);
+  snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", g->sockdir,
filename);
   (*sockpath)[UNIX_PATH_MAX-1] = '\0';
 
   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 Tue, Feb 02, 2016 at 03:27:39PM +0100, Pino Toscano
wrote:
> Introduce an internal helper to create paths for sockets; will be useful
> for changing later the logic for placing sockets.
> ---
>  src/guestfs-internal.h |  1 +
>  src/launch-direct.c    |  4 +++-
>  src/launch-libvirt.c   | 10 ++++++----
>  src/launch.c           | 15 +++++++++++++++
>  4 files changed, 25 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]);
I'm sure I read a rant from Linus about how this doesn't actually
enforce the array length.  However I have tested it, and gcc warns if
I pass the wrong array length, so this looks OK.
>  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..ec061e3 100644
> --- a/src/launch.c
> +++ b/src/launch.c
> @@ -418,6 +418,21 @@ 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])
> +{
> +  char *path = g->tmpdir;
> +
> +  snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", path,
filename);
> +  (*sockpath)[UNIX_PATH_MAX-1] = '\0';
What's wrong with:

  snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", path, 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
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 Tue, Feb 02, 2016 at 03:27:40PM +0100, Pino Toscano
wrote:
> Extract the bit of code for lazy creation of a temporary directory, so
> it can be used for more directories as well.
> 
> This is just code motion, with no behaviour changes.
> ---
>  src/tmpdirs.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/src/tmpdirs.c b/src/tmpdirs.c
> index 9154d8b..2ae9c74 100644
> --- a/src/tmpdirs.c
> +++ b/src/tmpdirs.c
> @@ -119,6 +119,22 @@ guestfs_impl_get_cachedir (guestfs_h *g)
>    return safe_strdup (g, str);
>  }
>  
> +static int
> +lazy_make_tmpdir (guestfs_h *g, char *(*getdir) (guestfs_h *g), char
**dest)
> +{
> +  if (!*dest) {
> +    CLEANUP_FREE char *tmpdir = getdir (g);
> +    char *tmppath = safe_asprintf (g, "%s/libguestfsXXXXXX",
tmpdir);
> +    if (mkdtemp (tmppath) == NULL) {
> +      perrorf (g, _("%s: cannot create temporary directory"),
tmppath);
> +      free (tmpdir);
> +      return -1;
> +    }
> +    *dest = tmppath;
> +  }
> +  return 0;
> +}
> +
>  /* The g->tmpdir (per-handle temporary directory) is not created when
>   * the handle is created.  Instead we create it lazily before the
>   * first time it is used, or during launch.
> @@ -126,17 +142,7 @@ guestfs_impl_get_cachedir (guestfs_h *g)
>  int
>  guestfs_int_lazy_make_tmpdir (guestfs_h *g)
>  {
> -  if (!g->tmpdir) {
> -    CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
> -    g->tmpdir = safe_asprintf (g, "%s/libguestfsXXXXXX",
tmpdir);
> -    if (mkdtemp (g->tmpdir) == NULL) {
> -      perrorf (g, _("%s: cannot create temporary directory"),
g->tmpdir);
> -      free (g->tmpdir);
> -      g->tmpdir = NULL;
> -      return -1;
> -    }
> -  }
> -  return 0;
> +  return lazy_make_tmpdir (g, guestfs_get_tmpdir, &g->tmpdir);
>  }
>  
>  /* Recursively remove a temporary directory.  If removal fails, just
Looks like a simple refactor, so 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 Tuesday 02 February 2016 19:47:12 Richard W.M. Jones
wrote:
> On Tue, Feb 02, 2016 at 03:27:39PM +0100, Pino Toscano wrote:
> > diff --git a/src/launch.c b/src/launch.c
> > index f59818f..ec061e3 100644
> > --- a/src/launch.c
> > +++ b/src/launch.c
> > @@ -418,6 +418,21 @@ 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])
> > +{
> > +  char *path = g->tmpdir;
> > +
> > +  snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", path,
filename);
> > +  (*sockpath)[UNIX_PATH_MAX-1] = '\0';
> 
> What's wrong with:
> 
>   snprintf (*sockpath, UNIX_PATH_MAX, "%s/%s", path, filename);
If the "$path/$filename" string is longer than UNIX_PATH_MAX, then
*sockpath won't be 0-terminated.  Since the line after that always
puts 0 at the end, we can just save one character.

The truncation of long paths always happens with this patch, and that
is what patch #3 addresses.

-- 
Pino Toscano

On Tue, Feb 02, 2016 at 03:27:41PM +0100, Pino Toscano
wrote:
> Introduce a new read-only API to get a path where to store temporary
> sockets: this is different than tmpdir, as we need short paths for
> sockets (due to sockaddr_un::sun_path), and is either XDG_RUNTIME_DIR
> if set, or /tmp.
> 
> Adapt guestfs_int_create_socketname to create sockets in that location,
> checking whether the resulting path still fits in the limited buffer.
> 
> 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/handle.c           |  9 ++++++++-
>  src/launch.c           | 10 ++++++++--
>  src/tmpdirs.c          | 33 +++++++++++++++++++++++++++++++++
>  test-tool/test-tool.c  |  6 ++++++
>  7 files changed, 88 insertions(+), 4 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
I think this section needs to be duplicated in guestfs(3) too?  Since
both man pages list environment variables.
>  =head1 FILES
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 24c6eb7..5b6898c 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 than C<guestfs_tmpdir>, as we need shorter paths
for
different from
> +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/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 ec061e3..e6972d1 100644
> --- a/src/launch.c
> +++ b/src/launch.c
> @@ -425,9 +425,15 @@ int
>  guestfs_int_create_socketname (guestfs_h *g, const char *filename,
>                                 char (*sockpath)[UNIX_PATH_MAX])
>  {
> -  char *path = g->tmpdir;
> +  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-1, "%s/%s", path,
filename);
> +  snprintf (*sockpath, UNIX_PATH_MAX-1, "%s/%s", g->sockdir,
filename);
>    (*sockpath)[UNIX_PATH_MAX-1] = '\0';
>  
>    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
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v