thr3ads.net - Libguestfs - [Libguestfs] [PATCH 1/6] launch: unix: check for length of sockets [Jan 2016]

If this information is useful, please help other people find it:
Share via:

Pino Toscano

2016-Jan-29 18:25 UTC

[Libguestfs] [PATCH 1/6] launch: unix: check for length of sockets

Error out early if the path to the socket will not fit into
sockaddr_un::sun_path, as we will not be able to connect to it.
---
 src/launch-unix.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/launch-unix.c b/src/launch-unix.c
index 740c554..973e14b 100644
--- a/src/launch-unix.c
+++ b/src/launch-unix.c
@@ -47,6 +47,12 @@ launch_unix (guestfs_h *g, void *datav, const char *sockpath)
     return -1;
   }
 
+  if (strlen (sockpath) > UNIX_PATH_MAX-1) {
+    error (g, _("socket filename too long (more than %d characters):
%s"),
+           UNIX_PATH_MAX-1, sockpath);
+    return -1;
+  }
+
   if (g->verbose)
     guestfs_int_print_timestamped_message (g, "connecting to %s",
sockpath);
 
-- 
2.5.0

Pino Toscano

2016-Jan-29 18:25 UTC

head link

[Libguestfs] [PATCH 2/6] launch: direct: save the path of the daemon socket

Save the path of the socket passed to qemu for communication with
guestfsd; we will need it to clean it correctly.
---
 src/launch-direct.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/launch-direct.c b/src/launch-direct.c
index 9f12730..1c805e3 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -61,6 +61,8 @@ struct backend_direct_data {
   int qemu_version_major, qemu_version_minor;
 
   int virtio_scsi;        /* See function qemu_supports_virtio_scsi */
+
+  char guestfsd_sock[UNIX_PATH_MAX]; /* Path to daemon socket. */
 };
 
 static int is_openable (guestfs_h *g, const char *path, int flags);
@@ -232,7 +234,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   int r;
   int flags;
   int sv[2];
-  char guestfsd_sock[256];
   struct sockaddr_un addr;
   CLEANUP_FREE char *uefi_code = NULL, *uefi_vars = NULL;
   CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL;
@@ -294,8 +295,8 @@ 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 (guestfsd_sock, sizeof guestfsd_sock, "%s/guestfsd.sock",
g->tmpdir);
-  unlink (guestfsd_sock);
+  snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock,
"%s/guestfsd.sock", g->tmpdir);
+  unlink (data->guestfsd_sock);
 
   daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (daemon_accept_sock == -1) {
@@ -304,7 +305,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
   }
 
   addr.sun_family = AF_UNIX;
-  strncpy (addr.sun_path, guestfsd_sock, UNIX_PATH_MAX);
+  strncpy (addr.sun_path, data->guestfsd_sock, UNIX_PATH_MAX);
   addr.sun_path[UNIX_PATH_MAX-1] = '\0';
 
   if (bind (daemon_accept_sock, (struct sockaddr *) &addr,
@@ -599,7 +600,7 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
 
   /* Set up virtio-serial for the communications channel. */
   ADD_CMDLINE ("-chardev");
-  ADD_CMDLINE_PRINTF ("socket,path=%s,id=channel0", guestfsd_sock);
+  ADD_CMDLINE_PRINTF ("socket,path=%s,id=channel0",
data->guestfsd_sock);
   ADD_CMDLINE ("-device");
   ADD_CMDLINE
("virtserialport,chardev=channel0,name=org.libguestfs.channel.0");
 
-- 
2.5.0

Pino Toscano

2016-Jan-29 18:25 UTC

head link

[Libguestfs] [PATCH 3/6] launch: direct: cleanup daemon socket on shutdown

Unlink the daemon socket in the shutdown callback, instead of right
before creating a new one.  This makes sure we are unlinking the right
socket.
---
 src/launch-direct.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/launch-direct.c b/src/launch-direct.c
index 1c805e3..b8e453d 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -296,7 +296,6 @@ launch_direct (guestfs_h *g, void *datav, const char *arg)
    * for qemu to connect to.
    */
   snprintf (data->guestfsd_sock, sizeof data->guestfsd_sock,
"%s/guestfsd.sock", g->tmpdir);
-  unlink (data->guestfsd_sock);
 
   daemon_accept_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (daemon_accept_sock == -1) {
@@ -1510,6 +1509,11 @@ shutdown_direct (guestfs_h *g, void *datav, int
check_for_errors)
 
   data->pid = data->recoverypid = 0;
 
+  if (data->guestfsd_sock[0] != '\0') {
+    unlink (data->guestfsd_sock);
+    data->guestfsd_sock[0] = '\0';
+  }
+
   free (data->qemu_help);
   data->qemu_help = NULL;
   free (data->qemu_version);
-- 
2.5.0

Pino Toscano

2016-Jan-29 18:25 UTC

head link

[Libguestfs] [PATCH 4/6] launch: libvirt: move socket path variables

Move the paths of the sockets used, from the libvirt_xml_params struct
to backend_libvirt_data; this way, they will be usable also outside the
launch callback.

Simply code motion.
---
 src/launch-libvirt.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 6239f45..185b44b 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -123,6 +123,8 @@ struct backend_libvirt_data {
   size_t nr_secrets;
   char *uefi_code;		/* UEFI (firmware) code and variables. */
   char *uefi_vars;
+  char guestfsd_path[UNIX_PATH_MAX]; /* paths to sockets */
+  char console_path[UNIX_PATH_MAX];
 };
 
 /* Parameters passed to construct_libvirt_xml and subfunctions.  We
@@ -135,8 +137,6 @@ struct libvirt_xml_params {
   char *appliance_overlay;      /* path to qcow2 overlay backed by appliance */
   char appliance_dev[64];       /* appliance device name */
   size_t appliance_index;       /* index of appliance */
-  char guestfsd_path[UNIX_PATH_MAX]; /* paths to sockets */
-  char console_path[UNIX_PATH_MAX];
   bool enable_svirt;            /* false if we decided to disable sVirt */
   bool current_proc_is_root;    /* true = euid is root */
 };
@@ -395,9 +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 (params.guestfsd_path, sizeof params.guestfsd_path,
+  snprintf (data->guestfsd_path, sizeof data->guestfsd_path,
             "%s/guestfsd.sock", g->tmpdir);
-  unlink (params.guestfsd_path);
+  unlink (data->guestfsd_path);
 
   set_socket_create_context (g);
 
@@ -408,7 +408,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
   }
 
   addr.sun_family = AF_UNIX;
-  memcpy (addr.sun_path, params.guestfsd_path, UNIX_PATH_MAX);
+  memcpy (addr.sun_path, data->guestfsd_path, UNIX_PATH_MAX);
 
   if (bind (daemon_accept_sock, (struct sockaddr *) &addr,
             sizeof addr) == -1) {
@@ -422,9 +422,9 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
   }
 
   /* For the serial console. */
-  snprintf (params.console_path, sizeof params.console_path,
+  snprintf (data->console_path, sizeof data->console_path,
             "%s/console.sock", g->tmpdir);
-  unlink (params.console_path);
+  unlink (data->console_path);
 
   console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (console_sock == -1) {
@@ -433,7 +433,7 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
   }
 
   addr.sun_family = AF_UNIX;
-  memcpy (addr.sun_path, params.console_path, UNIX_PATH_MAX);
+  memcpy (addr.sun_path, data->console_path, UNIX_PATH_MAX);
 
   if (bind (console_sock, (struct sockaddr *) &addr, sizeof addr) == -1) {
     perrorf (g, "bind");
@@ -470,24 +470,24 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
      */
     struct group *grp;
 
-    if (chmod (params.guestfsd_path, 0660) == -1) {
-      perrorf (g, "chmod: %s", params.guestfsd_path);
+    if (chmod (data->guestfsd_path, 0660) == -1) {
+      perrorf (g, "chmod: %s", data->guestfsd_path);
       goto cleanup;
     }
 
-    if (chmod (params.console_path, 0660) == -1) {
-      perrorf (g, "chmod: %s", params.console_path);
+    if (chmod (data->console_path, 0660) == -1) {
+      perrorf (g, "chmod: %s", data->console_path);
       goto cleanup;
     }
 
     grp = getgrnam ("qemu");
     if (grp != NULL) {
-      if (chown (params.guestfsd_path, 0, grp->gr_gid) == -1) {
-        perrorf (g, "chown: %s", params.guestfsd_path);
+      if (chown (data->guestfsd_path, 0, grp->gr_gid) == -1) {
+        perrorf (g, "chown: %s", data->guestfsd_path);
         goto cleanup;
       }
-      if (chown (params.console_path, 0, grp->gr_gid) == -1) {
-        perrorf (g, "chown: %s", params.console_path);
+      if (chown (data->console_path, 0, grp->gr_gid) == -1) {
+        perrorf (g, "chown: %s", data->console_path);
         goto cleanup;
       }
     } else
@@ -1282,7 +1282,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
       attribute ("type", "unix");
       start_element ("source") {
         attribute ("mode", "connect");
-        attribute ("path", params->console_path);
+        attribute ("path", params->data->console_path);
       } end_element ();
       start_element ("target") {
         attribute ("port", "0");
@@ -1294,7 +1294,7 @@ construct_libvirt_xml_devices (guestfs_h *g,
       attribute ("type", "unix");
       start_element ("source") {
         attribute ("mode", "connect");
-        attribute ("path", params->guestfsd_path);
+        attribute ("path", params->data->guestfsd_path);
       } end_element ();
       start_element ("target") {
         attribute ("type", "virtio");
-- 
2.5.0

Pino Toscano

2016-Jan-29 18:25 UTC

head link

[Libguestfs] [PATCH 5/6] launch: libvirt: cleanup sockets on shutdown

Unlink the sockets in the shutdown callback, instead of right before
creating a new ones.  This makes sure we are unlinking the right
sockets.
---
 src/launch-libvirt.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 185b44b..8a5d93e 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -397,7 +397,6 @@ launch_libvirt (guestfs_h *g, void *datav, const char
*libvirt_uri)
    */
   snprintf (data->guestfsd_path, sizeof data->guestfsd_path,
             "%s/guestfsd.sock", g->tmpdir);
-  unlink (data->guestfsd_path);
 
   set_socket_create_context (g);
 
@@ -424,7 +423,6 @@ 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);
-  unlink (data->console_path);
 
   console_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
   if (console_sock == -1) {
@@ -2027,6 +2025,16 @@ shutdown_libvirt (guestfs_h *g, void *datav, int
check_for_errors)
   if (conn != NULL)
     virConnectClose (conn);
 
+  if (data->guestfsd_path[0] != '\0') {
+    unlink (data->guestfsd_path);
+    data->guestfsd_path[0] = '\0';
+  }
+
+  if (data->console_path[0] != '\0') {
+    unlink (data->console_path);
+    data->console_path[0] = '\0';
+  }
+
   data->conn = NULL;
   data->dom = NULL;
 
-- 
2.5.0

Pino Toscano

2016-Jan-29 18:25 UTC

head link

[Libguestfs] [PATCH 6/6] launch: avoid too long paths for sockets

The API for UNIX sockets limits the path to a static size (usually 104
or 108 characters, NULL included), which is internally represented as
UNIX_PATH_MAX.  If the temporary directory set is long enough (e.g.
when running tools as uninstalled, using "run") then these socket
paths
get trucated, if not even cause failures when binding the sockets.

Introduce a new internal API to create paths for sockets, and a new
helper path where store them in the above case. This new path is created
only when needed, on a temporary directory under /tmp (hardcoded), and
cleaned when the handle is destroyed.
---
 src/guestfs-internal.h |  8 ++++++++
 src/handle.c           |  4 +++-
 src/launch-direct.c    |  4 +++-
 src/launch-libvirt.c   | 10 ++++++----
 src/launch.c           | 30 ++++++++++++++++++++++++++++++
 src/tmpdirs.c          | 22 ++++++++++++++++++++++
 6 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 5ecd322..5575aff 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -438,6 +438,11 @@ struct guestfs_h
   char *env_tmpdir;             /* $TMPDIR (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
*/
+  /* Optional socket directory - this is created on demand only when
+   * creating a socket whose absolute path in tmpdir would not fit
+   * into sockaddr_un::sun_path.
+   */
+  char *sockdir;
 
   /* Error handler, plus stack of old error handlers. */
   guestfs_error_handler_cb   error_cb;
@@ -758,7 +763,9 @@ 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_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 */
@@ -782,6 +789,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/handle.c b/src/handle.c
index 947818c..076e3fd 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -347,8 +347,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;
@@ -380,6 +381,7 @@ guestfs_close (guestfs_h *g)
   free (g->env_tmpdir);
   free (g->int_tmpdir);
   free (g->int_cachedir);
+  free (g->sockdir);
   free (g->last_error);
   free (g->identifier);
   free (g->program);
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..1f1f93d 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -418,6 +418,36 @@ guestfs_int_get_cpu_model (int kvm)
 #endif
 }
 
+/* Create the path for a socket with the selected filename in the tmpdir,
+ * falling back to creating a separate sockdir and using that as base.
+ */
+int
+guestfs_int_create_socketname (guestfs_h *g, const char *filename,
+                               char (*sockpath)[UNIX_PATH_MAX])
+{
+  char *path = g->tmpdir;
+
+  /* Check whether the new socket can fit into tmpdir. */
+  if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
+    if (guestfs_int_lazy_make_sockdir (g) == -1)
+      return -1;
+
+    path = g->sockdir;
+
+    /* Safety check for the new path in sockdir. */
+    if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
+      error (g, _("cannot get a path for the socket '%s'"),
+             filename);
+      return -1;
+    }
+  }
+
+  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
diff --git a/src/tmpdirs.c b/src/tmpdirs.c
index 9154d8b..e406e53 100644
--- a/src/tmpdirs.c
+++ b/src/tmpdirs.c
@@ -139,6 +139,21 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g)
   return 0;
 }
 
+int
+guestfs_int_lazy_make_sockdir (guestfs_h *g)
+{
+  if (!g->sockdir) {
+    g->sockdir = safe_strdup (g, "/tmp/guestfs-sockXXXXXX");
+    if (mkdtemp (g->sockdir) == NULL) {
+      perrorf (g, _("%s: cannot create temporary directory"),
g->sockdir);
+      free (g->sockdir);
+      g->sockdir = NULL;
+      return -1;
+    }
+  }
+  return 0;
+}
+
 /* 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
@@ -161,3 +176,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);
+}
-- 
2.5.0

Richard W.M. Jones

2016-Jan-29 19:09 UTC

head link

Re: [Libguestfs] [PATCH 5/6] launch: libvirt: cleanup sockets on shutdown

On Fri, Jan 29, 2016 at 07:25:29PM +0100, Pino Toscano
wrote:> Unlink the sockets in the shutdown callback, instead of right before
> creating a new ones.  This makes sure we are unlinking the right
> sockets.
ACK up to patch 5.

I have some questions about patch 6, coming up.

Rich.

-- 
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

Richard W.M. Jones

2016-Jan-29 19:14 UTC

head link

Re: [Libguestfs] [PATCH 6/6] launch: avoid too long paths for sockets

On Fri, Jan 29, 2016 at 07:25:30PM +0100, Pino Toscano
wrote:> The API for UNIX sockets limits the path to a static size (usually 104
> or 108 characters, NULL included), which is internally represented as
> UNIX_PATH_MAX.  If the temporary directory set is long enough (e.g.
> when running tools as uninstalled, using "run") then these socket
paths
> get trucated, if not even cause failures when binding the sockets.
> 
> Introduce a new internal API to create paths for sockets, and a new
> helper path where store them in the above case. This new path is created
> only when needed, on a temporary directory under /tmp (hardcoded), and
> cleaned when the handle is destroyed.
I think reading between the lines, you want to hard code /tmp so that
UNIX_PATH_MAX can never be exceeded, even if the user sets $TMPDIR to
some stupid long path?

This new behaviour certainly needs to be documented, eg. in
guestfs(3), because it changes an assumption that setting TMPDIR will
move every temporary file that libguestfs creates.

Are there machines where /tmp is unusable?

Should we provide an API to read the sockname, analogous to
guestfs_get_tmpdir and guestfs_get_cachedir?

Should we use /run or /dev/shm instead?  I would say, not /run/user
because systemd doesn't reliably create it, unfortunately, but maybe
somewhere else in /run would be acceptable.

How does libvirt handle socket paths?  Does it put them in /run, and
can we emulate its behaviour?

Are there implications for testing?  [When running tests, the ./run
script sets TMPDIR to point to /path/to/libguestfs/tmp.  Mainly I did
that so that if you ran multiple parallel copies of the tests (eg.
you are testing libguestfs master and a stable branch), you don't want
the appliance to constantly get rebuilt.]

Rich.
>  src/guestfs-internal.h |  8 ++++++++
>  src/handle.c           |  4 +++-
>  src/launch-direct.c    |  4 +++-
>  src/launch-libvirt.c   | 10 ++++++----
>  src/launch.c           | 30 ++++++++++++++++++++++++++++++
>  src/tmpdirs.c          | 22 ++++++++++++++++++++++
>  6 files changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
> index 5ecd322..5575aff 100644
> --- a/src/guestfs-internal.h
> +++ b/src/guestfs-internal.h
> @@ -438,6 +438,11 @@ struct guestfs_h
>    char *env_tmpdir;             /* $TMPDIR (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 */
> +  /* Optional socket directory - this is created on demand only when
> +   * creating a socket whose absolute path in tmpdir would not fit
> +   * into sockaddr_un::sun_path.
> +   */
> +  char *sockdir;
>  
>    /* Error handler, plus stack of old error handlers. */
>    guestfs_error_handler_cb   error_cb;
> @@ -758,7 +763,9 @@ 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_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 */
> @@ -782,6 +789,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/handle.c b/src/handle.c
> index 947818c..076e3fd 100644
> --- a/src/handle.c
> +++ b/src/handle.c
> @@ -347,8 +347,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;
> @@ -380,6 +381,7 @@ guestfs_close (guestfs_h *g)
>    free (g->env_tmpdir);
>    free (g->int_tmpdir);
>    free (g->int_cachedir);
> +  free (g->sockdir);
>    free (g->last_error);
>    free (g->identifier);
>    free (g->program);
> 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..1f1f93d 100644
> --- a/src/launch.c
> +++ b/src/launch.c
> @@ -418,6 +418,36 @@ guestfs_int_get_cpu_model (int kvm)
>  #endif
>  }
>  
> +/* Create the path for a socket with the selected filename in the tmpdir,
> + * falling back to creating a separate sockdir and using that as base.
> + */
> +int
> +guestfs_int_create_socketname (guestfs_h *g, const char *filename,
> +                               char (*sockpath)[UNIX_PATH_MAX])
> +{
> +  char *path = g->tmpdir;
> +
> +  /* Check whether the new socket can fit into tmpdir. */
> +  if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
> +    if (guestfs_int_lazy_make_sockdir (g) == -1)
> +      return -1;
> +
> +    path = g->sockdir;
> +
> +    /* Safety check for the new path in sockdir. */
> +    if (strlen (path) + 1 + strlen (filename) > UNIX_PATH_MAX-1) {
> +      error (g, _("cannot get a path for the socket
'%s'"),
> +             filename);
> +      return -1;
> +    }
> +  }
> +
> +  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
> diff --git a/src/tmpdirs.c b/src/tmpdirs.c
> index 9154d8b..e406e53 100644
> --- a/src/tmpdirs.c
> +++ b/src/tmpdirs.c
> @@ -139,6 +139,21 @@ guestfs_int_lazy_make_tmpdir (guestfs_h *g)
>    return 0;
>  }
>  
> +int
> +guestfs_int_lazy_make_sockdir (guestfs_h *g)
> +{
> +  if (!g->sockdir) {
> +    g->sockdir = safe_strdup (g, "/tmp/guestfs-sockXXXXXX");
> +    if (mkdtemp (g->sockdir) == NULL) {
> +      perrorf (g, _("%s: cannot create temporary directory"),
g->sockdir);
> +      free (g->sockdir);
> +      g->sockdir = NULL;
> +      return -1;
> +    }
> +  }
> +  return 0;
> +}
> +
>  /* 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
> @@ -161,3 +176,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);
> +}
> -- 
> 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
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Possibly Parallel Threads

Search for more maybe matching threads

Libguestfs - Jan 2016 - [PATCH 1/6] launch: unix: check for length of sockets

[Libguestfs] [PATCH 1/6] launch: unix: check for length of sockets

[Libguestfs] [PATCH 2/6] launch: direct: save the path of the daemon socket

[Libguestfs] [PATCH 3/6] launch: direct: cleanup daemon socket on shutdown

[Libguestfs] [PATCH 4/6] launch: libvirt: move socket path variables

[Libguestfs] [PATCH 5/6] launch: libvirt: cleanup sockets on shutdown

[Libguestfs] [PATCH 6/6] launch: avoid too long paths for sockets

Re: [Libguestfs] [PATCH 5/6] launch: libvirt: cleanup sockets on shutdown

Re: [Libguestfs] [PATCH 6/6] launch: avoid too long paths for sockets

Possibly Parallel Threads