Libguestfs - Dec 2016 - [PATCH v2 0/2] Improve inspection of /usr filesystems

Hi,

this patch series improves the way /usr filesystems are handled: tag
them appropriately, so later on we can find them and merge results they
contain directly back for the root filesystem.

Changes in v2:
- removed patches #1 and #2, already pushed
- drop patch #3, no more needed
- replace patch #4 with a better suggestion from Rich
- change if into assert in patch #5

Thanks,

Pino Toscano (2):
  inspect: fix existance check of /dev/mapper devices
  inspect: gather info from /usr filesystems as well (RHBZ#1401474)

 src/guestfs-internal.h |  1 +
 src/inspect-fs-unix.c  | 39 +++++++++++++++++++++++++-
 src/inspect-fs.c       |  6 ++--
 src/inspect.c          | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+), 3 deletions(-)

-- 
2.7.4

When checking for the existance of /dev/mapper devices found in the
fstab of a filesystem, using guestfs_exists means they are checked as
files in the guest, while they really appear as devices on the
appliance. Instead, try the lvm name resolution anyway, and ignore them
when they are reported as missing.

Thanks to: Richard W.M. Jones.

Fixes commit 96b6504b09461aeb6850bb2e7b870a0a4c2f5edf.
---
 src/inspect-fs-unix.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index c833304..1e1cf5d 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -24,6 +24,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <libintl.h>
+#include <errno.h>
 
 #ifdef HAVE_ENDIAN_H
 #include <endian.h>
@@ -1820,7 +1821,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec,
Hash_table *md_map,
   char *type, *slice, *disk, *part;
   int r;
 
-  if (STRPREFIX (spec, "/dev/mapper/") && guestfs_exists (g,
spec) > 0) {
+  if (STRPREFIX (spec, "/dev/mapper/")) {
     /* LVM2 does some strange munging on /dev/mapper paths for VGs and
      * LVs which contain '-' character:
      *
@@ -1831,7 +1832,17 @@ resolve_fstab_device (guestfs_h *g, const char *spec,
Hash_table *md_map,
      * This makes it impossible to reverse those paths directly, so
      * we have implemented lvm_canonical_lv_name in the daemon.
      */
+    guestfs_push_error_handler (g, NULL, NULL);
     device = guestfs_lvm_canonical_lv_name (g, spec);
+    guestfs_pop_error_handler (g);
+    if (device == NULL) {
+      if (guestfs_last_errno (g) == ENOENT) {
+        /* Ignore devices that don't exist. (RHBZ#811872) */
+      } else {
+        guestfs_int_error_errno (g, guestfs_last_errno (g), "%s",
guestfs_last_error (g));
+        return NULL;
+      }
+    }
   }
   else if (match3 (g, spec, re_xdev, &type, &disk, &part)) {
     r = resolve_fstab_device_xdev (g, type, disk, part, &device);
-- 
2.7.4

Flag the filesystems for Linux /usr properly as USR role, and detect
some data out of it, like the distro information from an os-release
(if present), and the architecture (since the binaries used for our
architecture check will be available there only).

Later on, collect the results in a way similar to what is done for
CoreOS: for each non-CoreOS root, try to find its /usr filesystem, and
if found then merge what is missing from root; in the last case, also
override the distro inspection data (version, product name) if available
in /usr.
---
 src/guestfs-internal.h |  1 +
 src/inspect-fs-unix.c  | 26 ++++++++++++++++++
 src/inspect-fs.c       |  6 ++--
 src/inspect.c          | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 2 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index d10191d..fbbfb90 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -864,6 +864,7 @@ extern void guestfs_int_merge_fs_inspections (guestfs_h *g,
struct inspect_fs *d
 
 /* inspect-fs-unix.c */
 extern int guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs);
+extern int guestfs_int_check_linux_usr (guestfs_h *g, struct inspect_fs *fs);
 extern int guestfs_int_check_freebsd_root (guestfs_h *g, struct inspect_fs
*fs);
 extern int guestfs_int_check_netbsd_root (guestfs_h *g, struct inspect_fs *fs);
 extern int guestfs_int_check_openbsd_root (guestfs_h *g, struct inspect_fs
*fs);
diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 1e1cf5d..fcc39e5 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -787,6 +787,32 @@ guestfs_int_check_linux_root (guestfs_h *g, struct
inspect_fs *fs)
   return 0;
 }
 
+/* The currently mounted device looks like a Linux /usr. */
+int
+guestfs_int_check_linux_usr (guestfs_h *g, struct inspect_fs *fs)
+{
+  int r;
+
+  fs->type = OS_TYPE_LINUX;
+  fs->role = OS_ROLE_USR;
+
+  if (guestfs_is_file_opts (g, "/lib/os-release",
+                            GUESTFS_IS_FILE_OPTS_FOLLOWSYMLINKS, 1, -1) > 0)
{
+    r = parse_os_release (g, fs, "/lib/os-release");
+    if (r == -1)        /* error */
+      return -1;
+    if (r == 1)         /* ok - detected the release from this file */
+      goto skip_release_checks;
+  }
+
+ skip_release_checks:;
+
+  /* Determine the architecture. */
+  check_architecture (g, fs);
+
+  return 0;
+}
+
 /* The currently mounted device is known to be a FreeBSD root. */
 int
 guestfs_int_check_freebsd_root (guestfs_h *g, struct inspect_fs *fs)
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 1951678..9f7630b 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -245,8 +245,10 @@ check_filesystem (guestfs_h *g, const char *mountable,
            is_dir_bin &&
            is_dir_share &&
            guestfs_is_dir (g, "/local") > 0 &&
-           guestfs_is_file (g, "/etc/fstab") == 0)
-    ;
+           guestfs_is_file (g, "/etc/fstab") == 0) {
+    if (guestfs_int_check_linux_usr (g, fs) == -1)
+      return -1;
+  }
   /* CoreOS /usr? */
   else if (is_dir_bin &&
            is_dir_share &&
diff --git a/src/inspect.c b/src/inspect.c
index 9055226..f6d0840 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -31,6 +31,7 @@
 #include <string.h>
 #include <sys/stat.h>
 #include <libintl.h>
+#include <assert.h>
 
 #ifdef HAVE_ENDIAN_H
 #include <endian.h>
@@ -46,6 +47,8 @@ COMPILE_REGEXP (re_primary_partition,
"^/dev/(?:h|s|v)d.[1234]$", 0)
 
 static void check_for_duplicated_bsd_root (guestfs_h *g);
 static void collect_coreos_inspection_info (guestfs_h *g);
+static void collect_linux_inspection_info (guestfs_h *g);
+static void collect_linux_inspection_info_for (guestfs_h *g, struct inspect_fs
*root);
 
 /**
  * The main inspection API.
@@ -88,6 +91,12 @@ guestfs_impl_inspect_os (guestfs_h *g)
    */
   check_for_duplicated_bsd_root (g);
 
+  /* For Linux guests with a separate /usr filesyste, merge some of the
+   * inspected information in that partition to the inspect_fs struct
+   * of the root filesystem.
+   */
+  collect_linux_inspection_info (g);
+
   /* 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
    * filesystems which are root devices and return that to the user.
@@ -149,6 +158,71 @@ collect_coreos_inspection_info (guestfs_h *g)
 }
 
 /**
+ * Traverse through the filesystems and find the /usr filesystem for
+ * the specified C<root>: if found, merge its basic inspection details
+ * to the root when they were set (i.e. because the /usr had os-release
+ * or other ways to identify the OS).
+ */
+static void
+collect_linux_inspection_info_for (guestfs_h *g, struct inspect_fs *root)
+{
+  size_t i;
+  struct inspect_fs *usr = NULL;
+
+  for (i = 0; i < g->nr_fses; ++i) {
+    struct inspect_fs *fs = &g->fses[i];
+    size_t j;
+
+    if (!(fs->distro == root->distro || fs->distro ==
OS_DISTRO_UNKNOWN) ||
+        fs->role != OS_ROLE_USR)
+      continue;
+
+    for (j = 0; j < root->nr_fstab; ++j) {
+      if (STREQ (fs->mountable, root->fstab[j].mountable)) {
+        usr = fs;
+        goto got_usr;
+      }
+    }
+  }
+
+  assert (usr == NULL);
+  return;
+
+ got_usr:
+  /* If the version information in /usr is not null, then most probably
+   * there was an os-release file there, so reset what is in root
+   * and pick the results from /usr.
+   */
+  if (!version_is_null (&usr->version)) {
+    root->distro = OS_DISTRO_UNKNOWN;
+    free (root->product_name);
+    root->product_name = NULL;
+  }
+
+  guestfs_int_merge_fs_inspections (g, root, usr);
+}
+
+/**
+ * Traverse through the filesystem list and find out if it contains
+ * the C</> and C</usr> filesystems of a Linux image (but not
CoreOS,
+ * for which there is a separate C<collect_coreos_inspection_info>).
+ * If this is the case, sum up all the collected information on each
+ * root fs from the respective /usr filesystems.
+ */
+static void
+collect_linux_inspection_info (guestfs_h *g)
+{
+  size_t i;
+
+  for (i = 0; i < g->nr_fses; ++i) {
+    struct inspect_fs *fs = &g->fses[i];
+
+    if (fs->distro != OS_DISTRO_COREOS && fs->role ==
OS_ROLE_ROOT)
+      collect_linux_inspection_info_for (g, fs);
+  }
+}
+
+/**
  * On *BSD systems, sometimes F</dev/sda[1234]> is a shadow of the
  * real root filesystem that is probably F</dev/sda5> (see:
  * L<http://www.freebsd.org/doc/handbook/disk-organization.html>)
-- 
2.7.4

On Wed, Dec 07, 2016 at 01:27:54PM +0100, Pino Toscano
wrote:
> Hi,
> 
> this patch series improves the way /usr filesystems are handled: tag
> them appropriately, so later on we can find them and merge results they
> contain directly back for the root filesystem.
ACK series.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/