lib: Rework temporary and cache directory code.

New APIs: set-tmpdir, get-tmpdir, set-cachedir, get-cachedir.

The current code has evolved over time and has a number of problems:

(a) A single environment variable ($TMPDIR) controls the
location of several directories.

(b) It's hard for the library user to predict which directory
libguestfs will use, unless the user simulates the same internal steps
that libguestfs performs.

This commit fixes these issues.

(a) Now three environment variables control the location of all small
temporary files, and the appliance cache:

  For temporary files: $LIBGUESTFS_TMPDIR or $TMPDIR or /tmp.

  For the appliance cache: $LIBGUESTFS_CACHEDIR or $TMPDIR or /var/tmp.

The user can also set these directories explicitly through API calls
(guestfs_set_tmpdir and guestfs_set_cachedir).

(b) The user can also retrieve the actual directories that libguestfs
will use, by calling guestfs_get_tmpdir and guestfs_get_cachedir.
These functions are also used internally.

This commit also:

 - reworks the internal tmpdir code

 - removes the internal (undocumented) guestfs_tmpdir call (replacing
   it with calls to the documented guestfs_get_tmpdir API instead)

 - changes the ./run script to set LIBGUESTFS_TMPDIR and
   LIBGUESTFS_CACHEDIR

 - adds a test

 - fixes a few places like libguestfs-make-fixed-appliance which
   depended on $TMPDIR

9 files changed, 208 insertions, 117 deletions

diff --git a/src/Makefile.am b/src/Makefile.am
index d3755beb..7b7f6065 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -165,6 +165,7 @@ libguestfs_la_SOURCES = \
 	listfs.c \
 	match.c \
 	proto.c \
+	tmpdirs.c \
 	libguestfs.syms
 
 libguestfs_la_LIBADD = \
diff --git a/src/appliance.c b/src/appliance.c
index 2241e181..8a22064b 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -354,7 +354,7 @@ check_for_cached_appliance (guestfs_h *g,
                             uid_t uid,
                             char **kernel, char **initrd, char **appliance)
 {
-  const char *tmpdir = guestfs___persistent_tmpdir ();
+  char *tmpdir = guestfs_get_cachedir (g);
 
   /* len must be longer than the length of any pathname we can
    * generate in this function.
@@ -365,6 +365,8 @@ check_for_cached_appliance (guestfs_h *g,
   char filename[len];
   snprintf (filename, len, "%s/checksum", cachedir);
 
+  free (tmpdir);
+
   (void) mkdir (cachedir, 0755);
 
   /* See if the cache directory exists and passes some simple checks
@@ -472,15 +474,18 @@ build_supermin_appliance (guestfs_h *g,
                           uid_t uid,
                           char **kernel, char **initrd, char **appliance)
 {
+  char *tmpdir;
+  size_t len;
+
   if (g->verbose)
     guestfs___print_timestamped_message (g, "begin building supermin appliance");
 
-  const char *tmpdir = guestfs___persistent_tmpdir ();
+  tmpdir = guestfs_get_cachedir (g);
 
   /* len must be longer than the length of any pathname we can
    * generate in this function.
    */
-  size_t len = strlen (tmpdir) + 128;
+  len = strlen (tmpdir) + 128;
 
   /* Build the appliance into a temporary directory. */
   char tmpcd[len];
@@ -488,6 +493,7 @@ build_supermin_appliance (guestfs_h *g,
 
   if (mkdtemp (tmpcd) == NULL) {
     perrorf (g, "mkdtemp");
+    free (tmpdir);
     return -1;
   }
 
@@ -496,7 +502,8 @@ build_supermin_appliance (guestfs_h *g,
 
   int r = run_supermin_helper (g, supermin_path, tmpcd);
   if (r == -1) {
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
+    free (tmpdir);
     return -1;
   }
 
@@ -509,13 +516,15 @@ build_supermin_appliance (guestfs_h *g,
   char filename2[len];
   snprintf (filename, len, "%s/checksum", cachedir);
 
+  free (tmpdir);
+
   /* Open and acquire write lock on checksum file.  The file might
    * not exist, in which case we want to create it.
    */
   int fd = open (filename, O_WRONLY|O_CREAT|O_NOCTTY|O_CLOEXEC, 0755);
   if (fd == -1) {
     perrorf (g, "open: %s", filename);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
   struct flock fl;
@@ -529,7 +538,7 @@ build_supermin_appliance (guestfs_h *g,
       goto again;
     perrorf (g, "fcntl: F_SETLKW: %s", filename);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
@@ -541,7 +550,7 @@ build_supermin_appliance (guestfs_h *g,
   if (ftruncate (fd, clen) == -1) {
     perrorf (g, "ftruncate: %s", filename);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
@@ -549,13 +558,13 @@ build_supermin_appliance (guestfs_h *g,
   if (rr == -1) {
     perrorf (g, "write: %s", filename);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
   if ((size_t) rr != clen) {
     error (g, "partial write: %s", filename);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
@@ -565,7 +574,7 @@ build_supermin_appliance (guestfs_h *g,
   if (rename (filename, filename2) == -1) {
     perrorf (g, "rename: %s %s", filename, filename2);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
@@ -575,7 +584,7 @@ build_supermin_appliance (guestfs_h *g,
   if (rename (filename, filename2) == -1) {
     perrorf (g, "rename: %s %s", filename, filename2);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
@@ -585,11 +594,11 @@ build_supermin_appliance (guestfs_h *g,
   if (rename (filename, filename2) == -1) {
     perrorf (g, "rename: %s %s", filename, filename2);
     close (fd);
-    guestfs___remove_tmpdir (g, tmpcd);
+    guestfs___recursive_remove_dir (g, tmpcd);
     return -1;
   }
 
-  guestfs___remove_tmpdir (g, tmpcd);
+  guestfs___recursive_remove_dir (g, tmpcd);
 
   /* Now finish off by linking to the cached appliance and returning it. */
   if (hard_link_to_cached_appliance (g, cachedir,
diff --git a/src/filearch.c b/src/filearch.c
index 93a697f5..1ad6af68 100644
--- a/src/filearch.c
+++ b/src/filearch.c
@@ -127,7 +127,7 @@ is_regular_file (const char *filename)
 static char *
 cpio_arch (guestfs_h *g, const char *file, const char *path)
 {
-  TMP_TEMPLATE_ON_STACK (dir);
+  TMP_TEMPLATE_ON_STACK (g, dir);
 #define dir_len (strlen (dir))
 #define initrd_len (dir_len + 16)
   char initrd[initrd_len];
@@ -221,7 +221,7 @@ cpio_arch (guestfs_h *g, const char *file, const char *path)
   if (cmd)
     guestfs___cmd_close (cmd);
 
-  guestfs___remove_tmpdir (g, dir);
+  guestfs___recursive_remove_dir (g, dir);
 
   return ret;
 #undef dir_len
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index aed4182c..17903d30 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -71,10 +71,11 @@
 #define TRACE4(name, arg1, arg2, arg3, arg4)
 #endif
 
-#define TMP_TEMPLATE_ON_STACK(var)                        \
-  const char *ttos_tmpdir = guestfs_tmpdir ();            \
+#define TMP_TEMPLATE_ON_STACK(g,var)                      \
+  char *ttos_tmpdir = guestfs_get_tmpdir (g);             \
   char var[strlen (ttos_tmpdir) + 32];                    \
-  sprintf (var, "%s/libguestfsXXXXXX", ttos_tmpdir)       \
+  sprintf (var, "%s/libguestfsXXXXXX", ttos_tmpdir);      \
+  free (ttos_tmpdir)
 
 #ifdef __APPLE__
 #define UNIX_PATH_MAX 104
@@ -234,11 +235,19 @@ struct guestfs_h
   const struct attach_ops *attach_ops;
 
   /**** Runtime information. ****/
-  char *tmpdir;			/* Temporary directory containing socket. */
-
   char *last_error;             /* Last error on handle. */
   int last_errnum;              /* errno, or 0 if there was no errno */
 
+  /* Temporary and cache directories. */
+  /* The actual temporary directory - this is not created with the
+   * handle, you have to call guestfs___lazy_make_tmpdir.
+   */
+  char *tmpdir;
+  /* Environment variables that affect tmpdir/cachedir locations. */
+  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 */
+
   /* Callbacks. */
   guestfs_abort_cb           abort_cb;
   guestfs_error_handler_cb   error_cb;
@@ -526,13 +535,15 @@ extern void guestfs___call_callbacks_void (guestfs_h *g, uint64_t event);
 extern void guestfs___call_callbacks_message (guestfs_h *g, uint64_t event, const char *buf, size_t buf_len);
 extern void guestfs___call_callbacks_array (guestfs_h *g, uint64_t event, const uint64_t *array, size_t array_len);
 
+/* tmpdirs.c */
+extern int guestfs___lazy_make_tmpdir (guestfs_h *g);
+extern void guestfs___remove_tmpdir (guestfs_h *g);
+extern void guestfs___recursive_remove_dir (guestfs_h *g, const char *dir);
+
 /* appliance.c */
 extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **initrd, char **appliance);
 
 /* launch.c */
-extern const char *guestfs___persistent_tmpdir (void);
-extern int guestfs___lazy_make_tmpdir (guestfs_h *g);
-extern void guestfs___remove_tmpdir (guestfs_h *g, const char *dir);
 extern int64_t guestfs___timeval_diff (const struct timeval *x, const struct timeval *y);
 extern void guestfs___print_timestamped_message (guestfs_h *g, const char *fs, ...);
 extern void guestfs___launch_send_progress (guestfs_h *g, int perdozen);
diff --git a/src/guestfs.c b/src/guestfs.c
index 5428b28a..4a1ee930 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -195,6 +195,18 @@ parse_environment (guestfs_h *g,
   if (str != NULL && STREQ (str, "1"))
     guestfs_set_verbose (g, 1);
 
+  str = do_getenv (data, "LIBGUESTFS_TMPDIR");
+  if (str)
+    guestfs_set_tmpdir (g, str);
+
+  str = do_getenv (data, "LIBGUESTFS_CACHEDIR");
+  if (str)
+    guestfs_set_cachedir (g, str);
+
+  free (g->env_tmpdir);
+  str = do_getenv (data, "TMPDIR");
+  g->env_tmpdir = str ? safe_strdup (g, str) : NULL;
+
   str = do_getenv (data, "LIBGUESTFS_PATH");
   if (str)
     guestfs_set_path (g, str);
@@ -300,14 +312,13 @@ guestfs_close (guestfs_h *g)
   /* Run user close callbacks. */
   guestfs___call_callbacks_void (g, GUESTFS_EVENT_CLOSE);
 
-  /* Remove whole temporary directory. */
-  if (g->tmpdir)
-    guestfs___remove_tmpdir (g, g->tmpdir);
-
   /* Test output file used by bindtests. */
   if (g->test_fp != NULL)
     fclose (g->test_fp);
 
+  /* Remove temporary directory. */
+  guestfs___remove_tmpdir (g);
+
   /* Mark the handle as dead and then free up all memory. */
   g->state = NO_HANDLE;
 
diff --git a/src/guestfs.pod b/src/guestfs.pod
index f243c2c4..bac81d86 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -2934,7 +2934,7 @@ C<febootstrap-supermin-helper> is invoked to create the kernel, a
 small initrd and the appliance.
 
 The appliance is cached in C</var/tmp/.guestfs-E<lt>UIDE<gt>> (or in
-another directory if C<TMPDIR> is set).
+another directory if C<LIBGUESTFS_CACHEDIR> or C<TMPDIR> are set).
 
 For a complete description of how the appliance is created and cached,
 read the L<febootstrap(8)> and L<febootstrap-supermin-helper(8)> man
@@ -3890,6 +3890,17 @@ Pass additional options to the guest kernel.
 Choose the default way to create the appliance.  See
 L</guestfs_set_attach_method>.
 
+=item LIBGUESTFS_CACHEDIR
+
+The location where libguestfs will cache its appliance, when
+using a supermin appliance.  The appliance is cached and shared
+between all handles which have the same effective user ID.
+
+If C<LIBGUESTFS_CACHEDIR> is not set, then C<TMPDIR> is used.  If
+C<TMPDIR> is not set, then C</var/tmp> is used.
+
+See also L</LIBGUESTFS_TMPDIR>, L</guestfs_set_cachedir>.
+
 =item LIBGUESTFS_DEBUG
 
 Set C<LIBGUESTFS_DEBUG=1> to enable verbose messages.  This
@@ -3915,6 +3926,16 @@ used.
 
 See also L</QEMU WRAPPERS> above.
 
+=item LIBGUESTFS_TMPDIR
+
+The location where libguestfs will store temporary files used
+by each handle.
+
+If C<LIBGUESTFS_TMPDIR> is not set, then C<TMPDIR> is used.  If
+C<TMPDIR> is not set, then C</tmp> is used.
+
+See also L</LIBGUESTFS_CACHEDIR>, L</guestfs_set_tmpdir>.
+
 =item LIBGUESTFS_TRACE
 
 Set C<LIBGUESTFS_TRACE=1> to enable command traces.  This
@@ -3922,14 +3943,7 @@ has the same effect as calling C<guestfs_set_trace (g, 1)>.
 
 =item TMPDIR
 
-Location of temporary directory, defaults to C</tmp> except for the
-cached supermin appliance which defaults to C</var/tmp>.
-
-If libguestfs was compiled to use the supermin appliance then the
-real appliance is cached in this directory, shared between all
-handles belonging to the same EUID.  You can use C<$TMPDIR> to
-configure another directory to use in case C</var/tmp> is not large
-enough.
+See L</LIBGUESTFS_CACHEDIR>, L</LIBGUESTFS_TMPDIR>.
 
 =back
 
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index c243fcbb..e4df2a6b 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -1043,16 +1043,15 @@ static int
 construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo)
 {
   struct qemu_param *qp;
-  const char *tmpdir;
+  char *tmpdir;
 
   XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:commandline"));
 
-  /* We need to ensure the snapshots are created in $TMPDIR (RHBZ#856619).
-   * If TMPDIR is not set, we must choose one, because otherwise libvirt
-   * will use a random TMPDIR (RHBZ#865464).  Luckily the
-   * guestfs___persistent_tmpdir function does both of these tasks.
+  /* We need to ensure the snapshots are created in the persistent
+   * temporary directory (RHBZ#856619).  We must set one, because
+   * otherwise libvirt will use a random TMPDIR (RHBZ#865464).
    */
-  tmpdir = guestfs___persistent_tmpdir ();
+  tmpdir = guestfs_get_cachedir (g);
 
   XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "qemu:env"));
   XMLERROR (-1,
@@ -1063,6 +1062,8 @@ construct_libvirt_xml_qemu_cmdline (guestfs_h *g, xmlTextWriterPtr xo)
                                          BAD_CAST tmpdir));
   XMLERROR (-1, xmlTextWriterEndElement (xo));
 
+  free (tmpdir);
+
   /* Workaround because libvirt user networking cannot specify "net="
    * parameter.
    */
diff --git a/src/launch.c b/src/launch.c
index 46817073..d2c1a42a 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -701,81 +701,6 @@ guestfs___launch_failed_error (guestfs_h *g)
                 "and/or run 'libguestfs-test-tool'."));
 }
 
-/* Return the location of the tmpdir (eg. "/tmp") and allow users
- * to override it at runtime using $TMPDIR.
- * http://www.pathname.com/fhs/pub/fhs-2.3.html#TMPTEMPORARYFILES
- */
-const char *
-guestfs_tmpdir (void)
-{
-  const char *tmpdir;
-
-#ifdef P_tmpdir
-  tmpdir = P_tmpdir;
-#else
-  tmpdir = "/tmp";
-#endif
-
-  const char *t = getenv ("TMPDIR");
-  if (t) tmpdir = t;
-
-  return tmpdir;
-}
-
-/* Return the location of the persistent tmpdir (eg. "/var/tmp") and
- * allow users to override it at runtime using $TMPDIR.
- * http://www.pathname.com/fhs/pub/fhs-2.3.html#VARTMPTEMPORARYFILESPRESERVEDBETWEE
- */
-const char *
-guestfs___persistent_tmpdir (void)
-{
-  const char *tmpdir;
-
-  tmpdir = "/var/tmp";
-
-  const char *t = getenv ("TMPDIR");
-  if (t) tmpdir = t;
-
-  return tmpdir;
-}
-
-/* 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.
- */
-int
-guestfs___lazy_make_tmpdir (guestfs_h *g)
-{
-  if (!g->tmpdir) {
-    TMP_TEMPLATE_ON_STACK (dir_template);
-    g->tmpdir = safe_strdup (g, dir_template);
-    if (mkdtemp (g->tmpdir) == NULL) {
-      perrorf (g, _("%s: cannot create temporary directory"), dir_template);
-      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
- * simpler and safer.
- */
-void
-guestfs___remove_tmpdir (guestfs_h *g, const char *dir)
-{
-  struct command *cmd;
-
-  cmd = guestfs___new_command (g);
-  guestfs___cmd_add_arg (cmd, "rm");
-  guestfs___cmd_add_arg (cmd, "-rf");
-  guestfs___cmd_add_arg (cmd, dir);
-  /* Ignore failures. */
-  guestfs___cmd_run (cmd);
-  guestfs___cmd_close (cmd);
-}
-
 int
 guestfs__get_pid (guestfs_h *g)
 {
diff --git a/src/tmpdirs.c b/src/tmpdirs.c
new file mode 100644
index 00000000..b7167a9d
--- /dev/null
+++ b/src/tmpdirs.c
@@ -0,0 +1,119 @@
+/* libguestfs
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "guestfs.h"
+#include "guestfs-internal.h"
+#include "guestfs-internal-actions.h"
+
+int
+guestfs__set_tmpdir (guestfs_h *g, const char *tmpdir)
+{
+  free (g->int_tmpdir);
+  g->int_tmpdir = tmpdir ? safe_strdup (g, tmpdir) : NULL;
+  return 0;
+}
+
+/* Note this actually calculates the tmpdir, so it never returns NULL. */
+char *
+guestfs__get_tmpdir (guestfs_h *g)
+{
+  const char *str;
+
+  if (g->int_tmpdir)
+    str = g->int_tmpdir;
+  else if (g->env_tmpdir)
+    str = g->env_tmpdir;
+  else
+    str = "/tmp";
+
+  return safe_strdup (g, str);
+}
+
+int
+guestfs__set_cachedir (guestfs_h *g, const char *cachedir)
+{
+  free (g->int_cachedir);
+  g->int_cachedir = cachedir ? safe_strdup (g, cachedir) : NULL;
+  return 0;
+}
+
+/* Note this actually calculates the cachedir, so it never returns NULL. */
+char *
+guestfs__get_cachedir (guestfs_h *g)
+{
+  const char *str;
+
+  if (g->int_cachedir)
+    str = g->int_cachedir;
+  else if (g->env_tmpdir)
+    str = g->env_tmpdir;
+  else
+    str = "/var/tmp";
+
+  return safe_strdup (g, str);
+}
+
+/* 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.
+ */
+int
+guestfs___lazy_make_tmpdir (guestfs_h *g)
+{
+  if (!g->tmpdir) {
+    TMP_TEMPLATE_ON_STACK (g, dir_template);
+    g->tmpdir = safe_strdup (g, dir_template);
+    if (mkdtemp (g->tmpdir) == NULL) {
+      perrorf (g, _("%s: cannot create temporary directory"), dir_template);
+      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
+ * simpler and safer.
+ */
+void
+guestfs___recursive_remove_dir (guestfs_h *g, const char *dir)
+{
+  struct command *cmd;
+
+  cmd = guestfs___new_command (g);
+  guestfs___cmd_add_arg (cmd, "rm");
+  guestfs___cmd_add_arg (cmd, "-rf");
+  guestfs___cmd_add_arg (cmd, dir);
+  /* Ignore failures. */
+  guestfs___cmd_run (cmd);
+  guestfs___cmd_close (cmd);
+}
+
+void
+guestfs___remove_tmpdir (guestfs_h *g)
+{
+  if (g->tmpdir)
+    guestfs___recursive_remove_dir (g, g->tmpdir);
+}