Libguestfs - Feb 2013 - [PATCH 1/2] Fix bogus partition number passed to guestfs___check_for_filesystem_on

Partition number was being passed to guestfs___check_for_filesystem_on
based on an index in list_partition. However, this ignores the
possibility of multiple block devices.

This change makes guestfs___check_for_filesystem_on examine the
passed-in device directly to determine if it is a whole device, or
what its partition number is.
---
 src/guestfs-internal.h |  2 +-
 src/inspect-fs.c       | 45 +++++++++++++++++++++++++--------------------
 src/inspect.c          |  8 ++++----
 3 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 86d024a..cf298f2 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -524,7 +524,7 @@ extern struct inspect_fs *guestfs___search_for_root
(guestfs_h *g, const char *r
 /* inspect-fs.c */
 extern int guestfs___is_file_nocase (guestfs_h *g, const char *);
 extern int guestfs___is_dir_nocase (guestfs_h *g, const char *);
-extern int guestfs___check_for_filesystem_on (guestfs_h *g, const char *device,
int is_block, int is_partnum);
+extern int guestfs___check_for_filesystem_on (guestfs_h *g, const char
*device);
 extern int guestfs___parse_unsigned_int (guestfs_h *g, const char *str);
 extern int guestfs___parse_unsigned_int_ignore_trailing (guestfs_h *g, const
char *str);
 extern int guestfs___parse_major_minor (guestfs_h *g, struct inspect_fs *fs);
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index ce075db..0069dc6 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -79,47 +79,47 @@ free_regexps (void)
   pcre_free (re_major_minor);
 }
 
-static int check_filesystem (guestfs_h *g, const char *device, int is_block,
int is_partnum);
+static int check_filesystem (guestfs_h *g, const char *device,
+                             int whole_device);
 static int extend_fses (guestfs_h *g);
 
 /* Find out if 'device' contains a filesystem.  If it does, add
  * another entry in g->fses.
  */
 int
-guestfs___check_for_filesystem_on (guestfs_h *g, const char *device,
-                                   int is_block, int is_partnum)
+guestfs___check_for_filesystem_on (guestfs_h *g, const char *device)
 {
-  CLEANUP_FREE char *vfs_type = NULL;
-  int is_swap, r;
-  struct inspect_fs *fs;
+  int r;
 
   /* Get vfs-type in order to check if it's a Linux(?) swap device.
    * If there's an error we should ignore it, so to do that we have to
    * temporarily replace the error handler with a null one.
    */
   guestfs_push_error_handler (g, NULL, NULL);
-  vfs_type = guestfs_vfs_type (g, device);
+  CLEANUP_FREE char *vfs_type = guestfs_vfs_type (g, device);
   guestfs_pop_error_handler (g);
 
-  is_swap = vfs_type && STREQ (vfs_type, "swap");
-
-  debug (g, "check_for_filesystem_on: %s %d %d (%s)",
-         device, is_block, is_partnum,
-         vfs_type ? vfs_type : "failed to get vfs type");
+  debug (g, "check_for_filesystem_on: %s (%s)",
+         device, vfs_type ? vfs_type : "failed to get vfs type");
 
-  if (is_swap) {
+  if (vfs_type && STREQ (vfs_type, "swap")) {
     if (extend_fses (g) == -1)
       return -1;
-    fs = &g->fses[g->nr_fses-1];
+    struct inspect_fs *fs = &g->fses[g->nr_fses-1];
     fs->device = safe_strdup (g, device);
     return 0;
   }
 
   /* If it's a whole device, see if it is an install ISO. */
-  if (is_block) {
+  int whole_device = guestfs_is_whole_device (g, device);
+  if (whole_device == -1) {
+    return -1;
+  }
+
+  if (whole_device) {
     if (extend_fses (g) == -1)
       return -1;
-    fs = &g->fses[g->nr_fses-1];
+    struct inspect_fs *fs = &g->fses[g->nr_fses-1];
 
     r = guestfs___check_installer_iso (g, fs, device);
     if (r == -1) {              /* Fatal error. */
@@ -149,7 +149,7 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const char
*device,
     return 0;
 
   /* Do the rest of the checks. */
-  r = check_filesystem (g, device, is_block, is_partnum);
+  r = check_filesystem (g, device, whole_device);
 
   /* Unmount the filesystem. */
   if (guestfs_umount_all (g) == -1)
@@ -165,12 +165,17 @@ guestfs___check_for_filesystem_on (guestfs_h *g, const
char *device,
  * (eg. /dev/sda1 => is_partnum == 1).
  */
 static int
-check_filesystem (guestfs_h *g, const char *device,
-                  int is_block, int is_partnum)
+check_filesystem (guestfs_h *g, const char *device, int whole_device)
 {
   if (extend_fses (g) == -1)
     return -1;
 
+  int partnum = -1;
+  if (!whole_device) {
+    partnum = guestfs_part_to_partnum (g, device);
+    /* If this returns an error it just means it's not a partition */
+  }
+
   struct inspect_fs *fs = &g->fses[g->nr_fses-1];
 
   fs->device = safe_strdup (g, device);
@@ -292,7 +297,7 @@ check_filesystem (guestfs_h *g, const char *device,
    * Skip these checks if it's not a whole device (eg. CD) or the
    * first partition (eg. bootable USB key).
    */
-  else if ((is_block || is_partnum == 1) &&
+  else if ((whole_device || partnum == 1) &&
            (guestfs_is_file (g, "/isolinux/isolinux.cfg") > 0 ||
             guestfs_is_dir (g, "/EFI/BOOT") > 0 ||
             guestfs_is_file (g, "/images/install.img") > 0 ||
diff --git a/src/inspect.c b/src/inspect.c
index c51c3f5..d0eb012 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -64,7 +64,7 @@ guestfs__inspect_os (guestfs_h *g)
 
   size_t i;
   for (i = 0; devices[i] != NULL; ++i) {
-    if (guestfs___check_for_filesystem_on (g, devices[i], 1, 0) == -1) {
+    if (guestfs___check_for_filesystem_on (g, devices[i]) == -1) {
       guestfs___free_string_list (devices);
       guestfs___free_inspect_info (g);
       return NULL;
@@ -83,7 +83,7 @@ guestfs__inspect_os (guestfs_h *g)
     if (parent_device_already_probed (g, partitions[i]))
       continue;
 
-    if (guestfs___check_for_filesystem_on (g, partitions[i], 0, i+1) == -1) {
+    if (guestfs___check_for_filesystem_on (g, partitions[i]) == -1) {
       guestfs___free_string_list (partitions);
       guestfs___free_inspect_info (g);
       return NULL;
@@ -99,7 +99,7 @@ guestfs__inspect_os (guestfs_h *g)
   }
 
   for (i = 0; mds[i] != NULL; ++i) {
-    if (guestfs___check_for_filesystem_on (g, mds[i], 0, i+1) == -1) {
+    if (guestfs___check_for_filesystem_on (g, mds[i]) == -1) {
       guestfs___free_string_list (mds);
       guestfs___free_inspect_info (g);
       return NULL;
@@ -117,7 +117,7 @@ guestfs__inspect_os (guestfs_h *g)
     }
 
     for (i = 0; lvs[i] != NULL; ++i) {
-      if (guestfs___check_for_filesystem_on (g, lvs[i], 0, 0) == -1) {
+      if (guestfs___check_for_filesystem_on (g, lvs[i]) == -1) {
         guestfs___free_string_list (lvs);
         guestfs___free_inspect_info (g);
         return NULL;
-- 
1.8.1

---
 src/inspect.c | 96 ++++-------------------------------------------------------
 1 file changed, 6 insertions(+), 90 deletions(-)

diff --git a/src/inspect.c b/src/inspect.c
index d0eb012..ae746cb 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -41,8 +41,6 @@
 #include "guestfs-internal-actions.h"
 #include "guestfs_protocol.h"
 
-static int parent_device_already_probed (guestfs_h *g, const char *partition);
-
 /* The main inspection code. */
 char **
 guestfs__inspect_os (guestfs_h *g)
@@ -53,78 +51,19 @@ guestfs__inspect_os (guestfs_h *g)
   if (guestfs_umount_all (g) == -1)
     return NULL;
 
-  /* Iterate over all possible devices.  Try to mount each
-   * (read-only).  Examine ones which contain filesystems and add that
-   * information to the handle.
+  /* Iterate over all detected filesystems.  Inspect each one in turn
+   * and add that information to the handle.
    */
-  /* Look to see if any devices directly contain filesystems (RHBZ#590167). */
-  char **devices = guestfs_list_devices (g);
-  if (devices == NULL)
-    return NULL;
 
-  size_t i;
-  for (i = 0; devices[i] != NULL; ++i) {
-    if (guestfs___check_for_filesystem_on (g, devices[i]) == -1) {
-      guestfs___free_string_list (devices);
-      guestfs___free_inspect_info (g);
-      return NULL;
-    }
-  }
-  guestfs___free_string_list (devices);
-
-  /* Look at all partitions. */
-  char **partitions = guestfs_list_partitions (g);
-  if (partitions == NULL) {
-    guestfs___free_inspect_info (g);
-    return NULL;
-  }
-
-  for (i = 0; partitions[i] != NULL; ++i) {
-    if (parent_device_already_probed (g, partitions[i]))
-      continue;
-
-    if (guestfs___check_for_filesystem_on (g, partitions[i]) == -1) {
-      guestfs___free_string_list (partitions);
-      guestfs___free_inspect_info (g);
-      return NULL;
-    }
-  }
-  guestfs___free_string_list (partitions);
-
-  /* Look at MD devices. */
-  char **mds = guestfs_list_md_devices (g);
-  if (mds == NULL) {
-    guestfs___free_inspect_info (g);
-    return NULL;
-  }
+  CLEANUP_FREE_STRING_LIST char **fses = guestfs_list_filesystems (g);
+  if (fses == NULL) return NULL; 
 
-  for (i = 0; mds[i] != NULL; ++i) {
-    if (guestfs___check_for_filesystem_on (g, mds[i]) == -1) {
-      guestfs___free_string_list (mds);
+  for (char **fs = fses; *fs; fs += 2) {
+    if (guestfs___check_for_filesystem_on (g, *fs)) {
       guestfs___free_inspect_info (g);
       return NULL;
     }
   }
-  guestfs___free_string_list (mds);
-
-  /* Look at all LVs. */
-  if (guestfs___feature_available (g, "lvm2")) {
-    char **lvs;
-    lvs = guestfs_lvs (g);
-    if (lvs == NULL) {
-      guestfs___free_inspect_info (g);
-      return NULL;
-    }
-
-    for (i = 0; lvs[i] != NULL; ++i) {
-      if (guestfs___check_for_filesystem_on (g, lvs[i]) == -1) {
-        guestfs___free_string_list (lvs);
-        guestfs___free_inspect_info (g);
-        return NULL;
-      }
-    }
-    guestfs___free_string_list (lvs);
-  }
 
   /* At this point we have, in the handle, a list of all filesystems
    * found and data about each one.  Now we assemble the list of
@@ -137,29 +76,6 @@ guestfs__inspect_os (guestfs_h *g)
   return ret;
 }
 
-/* If we found a filesystem on the parent device then ignore the
- * partitions within.
- */
-static int
-parent_device_already_probed (guestfs_h *g, const char *partition)
-{
-  CLEANUP_FREE char *device = NULL;
-  size_t i;
-
-  guestfs_push_error_handler (g, NULL, NULL);
-  device = guestfs_part_to_dev (g, partition);
-  guestfs_pop_error_handler (g);
-  if (!device)
-    return 0;
-
-  for (i = 0; i < g->nr_fses; ++i) {
-    if (STREQ (device, g->fses[i].device))
-      return 1;
-  }
-
-  return 0;
-}
-
 static int
 compare_strings (const void *vp1, const void *vp2)
 {
-- 
1.8.1

This patch moves a lot of variable declarations inline.  I've
been steadily moving them to the top of function definitions ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
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