Libguestfs - Jun 2011 - [PATCH] Change download_to_tmp so it can work with multi-root operating systems.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 2fbb9e01d508fb498ce07a8cb3fe6531bf00902f Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Mon, 27 Jun 2011 16:10:25 +0100
Subject: [PATCH] Change download_to_tmp so it can work with multi-root
 operating systems.

The previous guestfs___download_to_tmp function did not handle
multiboot correctly.  In particular it used the same cache name
for downloaded files from different roots, which could have caused
things like applications in each root to be confused.

This changes the function so that the cache filename is prefixed
with the root / fs number, eg. $tmpdir/0-Name instead of $tmpdir/Name.

This change also requires the function to return the new name, so
all places in the code which called this function had to be
updated.

This updates and fixes commit 3c1f762abed92f7a358f3bc93e3396d0606b18ad.
---
 src/guestfs-internal.h   |    2 +-
 src/inspect.c            |   73 ++++++++++++++++++----------------
 src/inspect_apps.c       |   98 +++++++++++++++++++++-------------------------
 src/inspect_fs_windows.c |   29 ++++++-------
 4 files changed, 98 insertions(+), 104 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 9193b97..0650eb2 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -353,7 +353,7 @@ extern void guestfs___call_callbacks_message (guestfs_h *g,
uint64_t event, cons
 extern void guestfs___call_callbacks_array (guestfs_h *g, uint64_t event, const
uint64_t *array, size_t array_len);
 #if defined(HAVE_PCRE) && defined(HAVE_HIVEX)
 extern int guestfs___check_for_filesystem_on (guestfs_h *g, const char *device,
int is_block, int is_partnum);
-extern int guestfs___download_to_tmp (guestfs_h *g, const char *filename, const
char *basename, int64_t max_size);
+extern char *guestfs___download_to_tmp (guestfs_h *g, struct inspect_fs *fs,
const char *filename, const char *basename, int64_t max_size);
 extern char *guestfs___case_sensitive_path_silently (guestfs_h *g, const char
*);
 extern struct inspect_fs *guestfs___search_for_root (guestfs_h *g, const char
*root);
 extern int guestfs___parse_unsigned_int (guestfs_h *g, const char *str);
diff --git a/src/inspect.c b/src/inspect.c
index 7061208..f04668d 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -500,69 +500,74 @@ guestfs__inspect_get_hostname (guestfs_h *g, const char
*root)
 }
 
 /* Download a guest file to a local temporary file.  The file is
- * downloaded into g->tmpdir, unless it already exists in g->tmpdir.
- * The final name will be g->tmpdir + "/" + basename.  Refuse to
- * download the guest file if it is larger than max_size.  The caller
- * does not need to delete the temporary file after use: it will be
- * deleted when the handle is cleaned up.
+ * cached in the temporary directory, and is not downloaded again.
+ *
+ * The name of the temporary (downloaded) file is returned.  The
+ * caller must free the pointer, but does *not* need to delete the
+ * temporary file.  It will be deleted when the handle is closed.
+ *
+ * Refuse to download the guest file if it is larger than max_size.
+ * On this and other errors, NULL is returned.
+ *
+ * There is actually one cache per 'struct inspect_fs *' in order
+ * to handle the case of multiple roots.
  */
-int
-guestfs___download_to_tmp (guestfs_h *g, const char *filename,
+char *
+guestfs___download_to_tmp (guestfs_h *g, struct inspect_fs *fs,
+                           const char *filename,
                            const char *basename, int64_t max_size)
 {
-  int tmpdirfd, fd, r = -1;
-  char buf[32];
+  char *r;
+  int fd;
+  char devfd[32];
   int64_t size;
 
-  tmpdirfd = open (g->tmpdir, O_RDONLY);
-  if (tmpdirfd == -1) {
-    perrorf (g, _("%s: temporary directory not found"),
g->tmpdir);
-    return -1;
+  /* Make the basename unique by prefixing it with the fs number. */
+  if (asprintf (&r, "%s/%ld-%s", g->tmpdir, fs - g->fses,
basename) == -1) {
+    perrorf (g, "asprintf");
+    return NULL;
   }
 
   /* If the file has already been downloaded, return. */
-  if (faccessat (tmpdirfd, basename, R_OK, 0) == 0) {
-    r = 0;
-    goto out;
-  }
+  if (access (r, R_OK) == 0)
+    return r;
 
   /* Check size of remote file. */
   size = guestfs_filesize (g, filename);
   if (size == -1)
     /* guestfs_filesize failed and has already set error in handle */
-    goto out;
+    goto error;
   if (size > max_size) {
     error (g, _("size of %s is unreasonably large (%" PRIi64 "
bytes)"),
            filename, size);
-    goto out;
+    goto error;
   }
 
-  fd = openat (tmpdirfd, basename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0600);
+  fd = open (r, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0600);
   if (fd == -1) {
-    perrorf (g, "openat: %s/%s", g->tmpdir, basename);
-    goto out;
+    perrorf (g, "open: %s", r);
+    goto error;
   }
 
-  snprintf (buf, sizeof buf, "/dev/fd/%d", fd);
+  snprintf (devfd, sizeof devfd, "/dev/fd/%d", fd);
 
-  if (guestfs_download (g, filename, buf) == -1) {
-    unlinkat (tmpdirfd, basename, 0);
+  if (guestfs_download (g, filename, devfd) == -1) {
+    unlink (r);
     close (fd);
-    goto out;
+    goto error;
   }
 
   if (close (fd) == -1) {
-    perrorf (g, "close: %s/%s", g->tmpdir, basename);
-    unlinkat (tmpdirfd, basename, 0);
-    goto out;
+    perrorf (g, "close: %s", r);
+    unlink (r);
+    goto error;
   }
 
-  r = 0;
- out:
-  if (tmpdirfd >= 0)
-    close (tmpdirfd);
-
   return r;
+
+ error:
+  free (r);
+  return NULL;
 }
 
 struct inspect_fs *
diff --git a/src/inspect_apps.c b/src/inspect_apps.c
index a41c46c..83c5e5e 100644
--- a/src/inspect_apps.c
+++ b/src/inspect_apps.c
@@ -264,57 +264,53 @@ read_package (guestfs_h *g,
 static struct guestfs_application_list *
 list_applications_rpm (guestfs_h *g, struct inspect_fs *fs)
 {
-  const char *name = "rpm_Name";
-  char tmpdir_name[strlen (g->tmpdir) + strlen (name) + 2];
-  snprintf (tmpdir_name, sizeof tmpdir_name, "%s/%s",
-            g->tmpdir, name);
-
-  if (guestfs___download_to_tmp (g, "/var/lib/rpm/Name", name,
-                                 MAX_PKG_DB_SIZE) == -1)
-    return NULL;
-
-  const char *pkgs = "rpm_Packages";
-  char tmpdir_pkgs[strlen (g->tmpdir) + strlen (pkgs) + 2];
-  snprintf (tmpdir_pkgs, sizeof tmpdir_pkgs, "%s/%s",
-            g->tmpdir, pkgs);
-
-  if (guestfs___download_to_tmp (g, "/var/lib/rpm/Packages", pkgs,
-                                 MAX_PKG_DB_SIZE) == -1)
-    return NULL;
-
-  /* Allocate interim structure to store names and links. */
-  struct rpm_names_list list;
-  list.names = NULL;
-  list.len = 0;
+  char *Name = NULL, *Packages = NULL;
+  struct rpm_names_list list = { .names = NULL, .len = 0 };
+  struct guestfs_application_list *apps = NULL;
+
+  Name = guestfs___download_to_tmp (g, fs,
+                                    "/var/lib/rpm/Name",
"rpm_Name",
+                                    MAX_PKG_DB_SIZE);
+  if (Name == NULL)
+    goto error;
+
+  Packages = guestfs___download_to_tmp (g, fs,
+                                        "/var/lib/rpm/Packages",
"rpm_Packages",
+                                        MAX_PKG_DB_SIZE);
+  if (Packages == NULL)
+    goto error;
 
   /* Read Name database. */
-  if (guestfs___read_db_dump (g, tmpdir_name, &list, read_rpm_name) == -1)
{
-    free_rpm_names_list (&list);
-    return NULL;
-  }
+  if (guestfs___read_db_dump (g, Name, &list, read_rpm_name) == -1)
+    goto error;
 
   /* Sort the names by link field for fast searching. */
   qsort (list.names, list.len, sizeof (struct rpm_name), compare_links);
 
   /* Allocate 'apps' list. */
-  struct guestfs_application_list *apps;
   apps = safe_malloc (g, sizeof *apps);
   apps->len = 0;
   apps->val = NULL;
 
   /* Read Packages database. */
-  struct read_package_data data;
-  data.list = &list;
-  data.apps = apps;
-  if (guestfs___read_db_dump (g, tmpdir_pkgs, &data, read_package) == -1) {
-    free_rpm_names_list (&list);
-    guestfs_free_application_list (apps);
-    return NULL;
-  }
+  struct read_package_data data = { .list = &list, .apps = apps };
+  if (guestfs___read_db_dump (g, Packages, &data, read_package) == -1)
+    goto error;
 
+  free (Name);
+  free (Packages);
   free_rpm_names_list (&list);
 
   return apps;
+
+ error:
+  free (Name);
+  free (Packages);
+  free_rpm_names_list (&list);
+  if (apps != NULL)
+    guestfs_free_application_list (apps);
+
+  return NULL;
 }
 
 #endif /* defined DB_DUMP */
@@ -322,13 +318,10 @@ list_applications_rpm (guestfs_h *g, struct inspect_fs
*fs)
 static struct guestfs_application_list *
 list_applications_deb (guestfs_h *g, struct inspect_fs *fs)
 {
-  const char *basename = "deb_status";
-  char tmpdir_basename[strlen (g->tmpdir) + strlen (basename) + 2];
-  snprintf (tmpdir_basename, sizeof tmpdir_basename, "%s/%s",
-            g->tmpdir, basename);
-
-  if (guestfs___download_to_tmp (g, "/var/lib/dpkg/status", basename,
-                                 MAX_PKG_DB_SIZE) == -1)
+  char *status = NULL;
+  status = guestfs___download_to_tmp (g, fs, "/var/lib/dpkg/status",
"status",
+                                      MAX_PKG_DB_SIZE);
+  if (status == NULL)
     return NULL;
 
   struct guestfs_application_list *apps = NULL, *ret = NULL;
@@ -338,9 +331,9 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs)
   char *name = NULL, *version = NULL, *release = NULL;
   int installed_flag = 0;
 
-  fp = fopen (tmpdir_basename, "r");
+  fp = fopen (status, "r");
   if (fp == NULL) {
-    perrorf (g, "fopen: %s", tmpdir_basename);
+    perrorf (g, "fopen: %s", status);
     goto out;
   }
 
@@ -396,7 +389,7 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs)
   }
 
   if (fclose (fp) == -1) {
-    perrorf (g, "fclose: %s", tmpdir_basename);
+    perrorf (g, "fclose: %s", status);
     goto out;
   }
   fp = NULL;
@@ -411,6 +404,7 @@ list_applications_deb (guestfs_h *g, struct inspect_fs *fs)
   free (name);
   free (version);
   free (release);
+  free (status);
   return ret;
 }
 
@@ -419,11 +413,6 @@ static void list_applications_windows_from_path (guestfs_h
*g, hive_h *h, struct
 static struct guestfs_application_list *
 list_applications_windows (guestfs_h *g, struct inspect_fs *fs)
 {
-  const char *basename = "software";
-  char tmpdir_basename[strlen (g->tmpdir) + strlen (basename) + 2];
-  snprintf (tmpdir_basename, sizeof tmpdir_basename, "%s/%s",
-            g->tmpdir, basename);
-
   size_t len = strlen (fs->windows_systemroot) + 64;
   char software[len];
   snprintf (software, len, "%s/system32/config/software",
@@ -436,17 +425,19 @@ list_applications_windows (guestfs_h *g, struct inspect_fs
*fs)
     return NULL;
   }
 
+  char *software_hive = NULL;
   struct guestfs_application_list *ret = NULL;
   hive_h *h = NULL;
 
-  if (guestfs___download_to_tmp (g, software_path, basename,
-                                 MAX_REGISTRY_SIZE) == -1)
+  software_hive = guestfs___download_to_tmp (g, fs, software_path,
"software",
+                                             MAX_REGISTRY_SIZE);
+  if (software_hive == NULL)
     goto out;
 
   free (software_path);
   software_path = NULL;
 
-  h = hivex_open (tmpdir_basename, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
+  h = hivex_open (software_hive, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
   if (h == NULL) {
     perrorf (g, "hivex_open");
     goto out;
@@ -474,6 +465,7 @@ list_applications_windows (guestfs_h *g, struct inspect_fs
*fs)
  out:
   if (h) hivex_close (h);
   free (software_path);
+  free (software_hive);
 
   return ret;
 }
diff --git a/src/inspect_fs_windows.c b/src/inspect_fs_windows.c
index 9a2e1fb..da7540f 100644
--- a/src/inspect_fs_windows.c
+++ b/src/inspect_fs_windows.c
@@ -159,11 +159,6 @@ check_windows_arch (guestfs_h *g, struct inspect_fs *fs)
 static int
 check_windows_software_registry (guestfs_h *g, struct inspect_fs *fs)
 {
-  const char *basename = "software";
-  char tmpdir_basename[strlen (g->tmpdir) + strlen (basename) + 2];
-  snprintf (tmpdir_basename, sizeof tmpdir_basename, "%s/%s",
-            g->tmpdir, basename);
-
   size_t len = strlen (fs->windows_systemroot) + 64;
   char software[len];
   snprintf (software, len, "%s/system32/config/software",
@@ -176,15 +171,17 @@ check_windows_software_registry (guestfs_h *g, struct
inspect_fs *fs)
      */
     return 0;
 
+  char *software_hive = NULL;
   int ret = -1;
   hive_h *h = NULL;
   hive_value_h *values = NULL;
 
-  if (guestfs___download_to_tmp (g, software_path, basename,
-                                 MAX_REGISTRY_SIZE) == -1)
+  software_hive = guestfs___download_to_tmp (g, fs, software_path,
"software",
+                                             MAX_REGISTRY_SIZE);
+  if (software_hive == NULL)
     goto out;
 
-  h = hivex_open (tmpdir_basename, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
+  h = hivex_open (software_hive, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
   if (h == NULL) {
     perrorf (g, "hivex_open");
     goto out;
@@ -268,6 +265,7 @@ check_windows_software_registry (guestfs_h *g, struct
inspect_fs *fs)
   if (h) hivex_close (h);
   free (values);
   free (software_path);
+  free (software_hive);
 
   return ret;
 }
@@ -275,11 +273,6 @@ check_windows_software_registry (guestfs_h *g, struct
inspect_fs *fs)
 static int
 check_windows_system_registry (guestfs_h *g, struct inspect_fs *fs)
 {
-  const char *basename = "system";
-  char tmpdir_basename[strlen (g->tmpdir) + strlen (basename) + 2];
-  snprintf (tmpdir_basename, sizeof tmpdir_basename, "%s/%s",
-            g->tmpdir, basename);
-
   size_t len = strlen (fs->windows_systemroot) + 64;
   char system[len];
   snprintf (system, len, "%s/system32/config/system",
@@ -292,6 +285,7 @@ check_windows_system_registry (guestfs_h *g, struct
inspect_fs *fs)
      */
     return 0;
 
+  char *system_hive = NULL;
   int ret = -1;
   hive_h *h = NULL;
   hive_node_h root, node;
@@ -299,11 +293,13 @@ check_windows_system_registry (guestfs_h *g, struct
inspect_fs *fs)
   int32_t dword;
   size_t i, count;
 
-  if (guestfs___download_to_tmp (g, system_path, basename,
-                                 MAX_REGISTRY_SIZE) == -1)
+  system_hive +    guestfs___download_to_tmp (g, fs, system_path,
"system",
+                               MAX_REGISTRY_SIZE);
+  if (system_hive == NULL)
     goto out;
 
-  h = hivex_open (tmpdir_basename, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
+  h = hivex_open (system_hive, g->verbose ? HIVEX_OPEN_VERBOSE : 0);
   if (h == NULL) {
     perrorf (g, "hivex_open");
     goto out;
@@ -449,6 +445,7 @@ check_windows_system_registry (guestfs_h *g, struct
inspect_fs *fs)
   if (h) hivex_close (h);
   free (values);
   free (system_path);
+  free (system_hive);
 
   return ret;
 }
-- 
1.7.5.2