[Libguestfs] [PATCH 1/1] inspect: Fix a bug in the *BSD root detection
Richard W.M. Jones
rjones at redhat.com
Fri Nov 28 14:59:24 UTC 2014
On Fri, Nov 28, 2014 at 03:42:58PM +0100, Pino Toscano wrote:
> On Friday 28 November 2014 14:31:01 Richard W.M. Jones wrote:
> > How about the attached patch? It's basically the same as your patch
> > but I moved the code between files and tidied up some whitespace
> > issues.
>
> Present in both the patches:
>
> > +/* On *BSD systems, sometimes /dev/sda[1234] is a shadow of the real root
> > + * filesystem that is probably /dev/sda5
> > + * (see: http://www.freebsd.org/doc/handbook/disk-organization.html)
> > + */
> > +static void
> > +check_for_duplicated_bsd_root (guestfs_h *g)
> > +{
> > + size_t i;
> > + bool is_primary, is_bsd;
> > + struct inspect_fs *fs, *bsd_primary = NULL;
> > +
> > + for (i = 0; i < g->nr_fses; ++i) {
> > + fs = &g->fses[i];
> > +
> > + is_primary = match (g, fs->mountable, re_primary_partition);
> > + is_bsd =
> > + fs->type == OS_TYPE_FREEBSD ||
> > + fs->type == OS_TYPE_NETBSD ||
> > + fs->type == OS_TYPE_OPENBSD;
> > +
> > + if (fs->is_root && is_primary && is_bsd) {
> > + bsd_primary = fs;
> > + continue;
> > + }
>
> This will run the regexp matching for every filesystem found; what
> about inlining the match call as last part of the if, like:
>
> is_bsd =
> fs->type == OS_TYPE_FREEBSD ||
> fs->type == OS_TYPE_NETBSD ||
> fs->type == OS_TYPE_OPENBSD;
>
> if (fs->is_root && is_bsd && is_primary
> && match (g, fs->mountable, re_primary_partition)) {
> bsd_primary = fs;
> continue;
> }
>
> This way it is done only for *BSD filesystems, and is_primary is
> used only in that if anyway.
>
> Also, is_bsd and fs could be declared just inside the for, as they are
> not needed outside of it.
Good points.
See updated patch below.
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/
-------------- next part --------------
From 282132c58bc81216a391fc62c28fc6f01b8c8fb6 Mon Sep 17 00:00:00 2001
From: Nikos Skalkotos <skalkoto at grnet.gr>
Date: Thu, 27 Nov 2014 18:30:33 +0200
Subject: [PATCH] inspect: Fix a bug in the *BSD root detection
The assumption that Linux will map the MBR partition to /dev/sda1
and the BSD 'a' partition to /dev/sda5 is not always correct.
Signed-off-by: Nikos Skalkotos <skalkoto at grnet.gr>
---
src/inspect-fs.c | 19 --------------
src/inspect.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 19 deletions(-)
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 539d814..b023ffc 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -47,7 +47,6 @@
* multiple threads call into the libguestfs API functions below
* simultaneously.
*/
-static pcre *re_first_partition;
static pcre *re_major_minor;
static void compile_regexps (void) __attribute__((constructor));
@@ -68,14 +67,12 @@ compile_regexps (void)
} \
} while (0)
- COMPILE (re_first_partition, "^/dev/(?:h|s|v)d.1$", 0);
COMPILE (re_major_minor, "(\\d+)\\.(\\d+)", 0);
}
static void
free_regexps (void)
{
- pcre_free (re_first_partition);
pcre_free (re_major_minor);
}
@@ -207,14 +204,6 @@ check_filesystem (guestfs_h *g, const char *mountable,
is_dir_bin &&
guestfs_is_file (g, "/etc/freebsd-update.conf") > 0 &&
guestfs_is_file (g, "/etc/fstab") > 0) {
- /* Ignore /dev/sda1 which is a shadow of the real root filesystem
- * that is probably /dev/sda5 (see:
- * http://www.freebsd.org/doc/handbook/disk-organization.html)
- */
- if (m->im_type == MOUNTABLE_DEVICE &&
- match (g, m->im_device, re_first_partition))
- return 0;
-
fs->is_root = 1;
fs->format = OS_FORMAT_INSTALLED;
if (guestfs___check_freebsd_root (g, fs) == -1)
@@ -225,14 +214,6 @@ check_filesystem (guestfs_h *g, const char *mountable,
guestfs_is_file (g, "/netbsd") > 0 &&
guestfs_is_file (g, "/etc/fstab") > 0 &&
guestfs_is_file (g, "/etc/release") > 0) {
- /* Ignore /dev/sda1 which is a shadow of the real root filesystem
- * that is probably /dev/sda5 (see:
- * http://www.freebsd.org/doc/handbook/disk-organization.html)
- */
- if (m->im_type == MOUNTABLE_DEVICE &&
- match (g, m->im_device, re_first_partition))
- return 0;
-
fs->is_root = 1;
fs->format = OS_FORMAT_INSTALLED;
if (guestfs___check_netbsd_root (g, fs) == -1)
diff --git a/src/inspect.c b/src/inspect.c
index 9248b06..0e98d41 100644
--- a/src/inspect.c
+++ b/src/inspect.c
@@ -32,11 +32,49 @@
#include <endian.h>
#endif
+#include "ignore-value.h"
+
#include "guestfs.h"
#include "guestfs-internal.h"
#include "guestfs-internal-actions.h"
#include "guestfs_protocol.h"
+/* Compile all the regular expressions once when the shared library is
+ * loaded. PCRE is thread safe so we're supposedly OK here if
+ * multiple threads call into the libguestfs API functions below
+ * simultaneously.
+ */
+static pcre *re_primary_partition;
+
+static void compile_regexps (void) __attribute__((constructor));
+static void free_regexps (void) __attribute__((destructor));
+
+static void
+compile_regexps (void)
+{
+ const char *err;
+ int offset;
+
+#define COMPILE(re,pattern,options) \
+ do { \
+ re = pcre_compile ((pattern), (options), &err, &offset, NULL); \
+ if (re == NULL) { \
+ ignore_value (write (2, err, strlen (err))); \
+ abort (); \
+ } \
+ } while (0)
+
+ COMPILE (re_primary_partition, "^/dev/(?:h|s|v)d.[1234]$", 0);
+}
+
+static void
+free_regexps (void)
+{
+ pcre_free (re_primary_partition);
+}
+
+static void check_for_duplicated_bsd_root (guestfs_h *g);
+
/* The main inspection code. */
char **
guestfs__inspect_os (guestfs_h *g)
@@ -64,6 +102,12 @@ guestfs__inspect_os (guestfs_h *g)
}
}
+ /* Check if the same filesystem was listed twice as root in g->fses.
+ * This may happen for the *BSD root partition where an MBR partition
+ * is a shadow of the real root partition probably /dev/sda5
+ */
+ check_for_duplicated_bsd_root (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.
@@ -75,6 +119,40 @@ guestfs__inspect_os (guestfs_h *g)
return ret;
}
+/* On *BSD systems, sometimes /dev/sda[1234] is a shadow of the real root
+ * filesystem that is probably /dev/sda5
+ * (see: http://www.freebsd.org/doc/handbook/disk-organization.html)
+ */
+static void
+check_for_duplicated_bsd_root (guestfs_h *g)
+{
+ size_t i;
+ struct inspect_fs *bsd_primary = NULL;
+
+ for (i = 0; i < g->nr_fses; ++i) {
+ bool is_bsd;
+ struct inspect_fs *fs = &g->fses[i];
+
+ is_bsd =
+ fs->type == OS_TYPE_FREEBSD ||
+ fs->type == OS_TYPE_NETBSD ||
+ fs->type == OS_TYPE_OPENBSD;
+
+ if (fs->is_root && is_bsd &&
+ match (g, fs->mountable, re_primary_partition)) {
+ bsd_primary = fs;
+ continue;
+ }
+
+ if (fs->is_root && bsd_primary && bsd_primary->type == fs->type) {
+ /* remove the is root flag from the bsd_primary */
+ bsd_primary->is_root = 0;
+ bsd_primary->format = OS_FORMAT_UNKNOWN;
+ return;
+ }
+ }
+}
+
static int
compare_strings (const void *vp1, const void *vp2)
{
--
2.1.0
More information about the Libguestfs
mailing list