Libguestfs - Nov 2013 - [PATCH stable-1.24] Fix fstab block device resolution for FreeBSD

The device name prefix for IDE hard drives used to be `ad' but now
is `ada' (http://www.freebsd.org/doc/handbook/disks-naming.html).
For virtio hard drives it is `vtbd'.

Under an mbr partition table a slice will be used, so the name of
the first partitions will be either ada0s1a or vtbd0s1a. Under a
GPT partition table, where no slice is needed, the name of the first
partition will be either ada0p1 or vtbd0p1.

Signed-off-by: Nikos Skalkotos <skalkoto at grnet.gr>
---
 src/guestfs-internal.h |  2 ++
 src/inspect-fs-unix.c  | 30 +++++++++++++++++++++++++-----
 src/match.c            | 25 +++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 9936c84..5356920 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -630,12 +630,14 @@ extern int guestfs___match (guestfs_h *g, const char *str,
const pcre *re);
 extern char *guestfs___match1 (guestfs_h *g, const char *str, const pcre *re);
 extern int guestfs___match2 (guestfs_h *g, const char *str, const pcre *re,
char **ret1, char **ret2);
 extern int guestfs___match3 (guestfs_h *g, const char *str, const pcre *re,
char **ret1, char **ret2, char **ret3);
+extern int guestfs___match4 (guestfs_h *g, const char *str, const pcre *re,
char **ret1, char **ret2, char **ret3, char **ret4);
 extern int guestfs___match6 (guestfs_h *g, const char *str, const pcre *re,
char **ret1, char **ret2, char **ret3, char **ret4, char **ret5, char **ret6);
 
 #define match guestfs___match
 #define match1 guestfs___match1
 #define match2 guestfs___match2
 #define match3 guestfs___match3
+#define match4 guestfs___match4
 #define match6 guestfs___match6
 
 /* stringsbuf.c */
diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c
index 60b081d..6addb43 100644
--- a/src/inspect-fs-unix.c
+++ b/src/inspect-fs-unix.c
@@ -64,7 +64,8 @@ static pcre *re_major_minor;
 static pcre *re_xdev;
 static pcre *re_cciss;
 static pcre *re_mdN;
-static pcre *re_freebsd;
+static pcre *re_freebsd_mbr;
+static pcre *re_freebsd_gpt;
 static pcre *re_diskbyid;
 static pcre *re_netbsd;
 static pcre *re_opensuse;
@@ -115,7 +116,8 @@ compile_regexps (void)
   COMPILE (re_xdev, "^/dev/(h|s|v|xv)d([a-z]+)(\\d*)$", 0);
   COMPILE (re_cciss, "^/dev/(cciss/c\\d+d\\d+)(?:p(\\d+))?$", 0);
   COMPILE (re_mdN, "^(/dev/md\\d+)$", 0);
-  COMPILE (re_freebsd, "^/dev/ad(\\d+)s(\\d+)([a-z])$", 0);
+  COMPILE (re_freebsd_mbr,
"^/dev/(ada{0,1}|vtbd)(\\d+)s(\\d+)([a-z])$", 0);
+  COMPILE (re_freebsd_gpt, "^/dev/(ada{0,1}|vtbd)(\\d+)p(\\d+)$", 0);
   COMPILE (re_diskbyid, "^/dev/disk/by-id/.*-part(\\d+)$", 0);
   COMPILE (re_netbsd, "^NetBSD (\\d+)\\.(\\d+)", 0);
   COMPILE (re_opensuse, "^(openSUSE|SuSE Linux|SUSE LINUX) ", 0);
@@ -143,7 +145,8 @@ free_regexps (void)
   pcre_free (re_xdev);
   pcre_free (re_cciss);
   pcre_free (re_mdN);
-  pcre_free (re_freebsd);
+  pcre_free (re_freebsd_mbr);
+  pcre_free (re_freebsd_gpt);
   pcre_free (re_diskbyid);
   pcre_free (re_netbsd);
   pcre_free (re_opensuse);
@@ -1464,13 +1467,30 @@ resolve_fstab_device (guestfs_h *g, const char *spec,
Hash_table *md_map)
 
     free (disk);
   }
-  else if (match3 (g, spec, re_freebsd, &disk, &slice, &part)) {
+  else if (match3 (g, spec, re_freebsd_gpt, &type, &disk, &part)) {
+    /* If the FreeBSD disk contains GPT partitions, the translation to Linux
+     * device names is straight forward. Partitions on a virtio disk are
+     * prefixed with vtbd. IDE hard drives used to be prefixed with ad and now
+     * are with ada.
+     */
+    char type_c = (strcmp (type, "vtbd") == 0) ? 'v' :
's';
+    int disk_i = guestfs___parse_unsigned_int (g, disk);
+    int part_i = guestfs___parse_unsigned_int (g, part);
+    free (type);
+    free (disk);
+    free (part);
+
+    if (disk_i != -1 && disk_i <= 26 && part_i > 0
&& part_i <= 128)
+      device = safe_asprintf (g, "/dev/%cd%c%d", type_c, disk_i +
'a', part_i);
+  }
+  else if (match4 (g, spec, re_freebsd_mbr, &type, &disk, &slice,
&part)) {
     /* FreeBSD disks are organized quite differently.  See:
      * http://www.freebsd.org/doc/handbook/disk-organization.html
      * FreeBSD "partitions" are exposed as quasi-extended partitions
      * numbered from 5 in Linux.  I have no idea what happens when you
      * have multiple "slices" (the FreeBSD term for MBR partitions).
      */
+    char type_c = (strcmp (type, "vtbd") == 0) ? 'v' :
's';
     int disk_i = guestfs___parse_unsigned_int (g, disk);
     int slice_i = guestfs___parse_unsigned_int (g, slice);
     int part_i = part[0] - 'a' /* counting from 0 */;
@@ -1481,7 +1501,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec,
Hash_table *md_map)
     if (disk_i != -1 && disk_i <= 26 &&
         slice_i > 0 && slice_i <= 1 /* > 4 .. see comment
above */ &&
         part_i >= 0 && part_i < 26) {
-      device = safe_asprintf (g, "/dev/sd%c%d", disk_i + 'a',
part_i + 5);
+      device = safe_asprintf (g, "/dev/%cd%c%d", type_c, disk_i +
'a', part_i + 5);
     }
   }
   else if ((part = match1 (g, spec, re_diskbyid)) != NULL) {
diff --git a/src/match.c b/src/match.c
index 86c0a31..2a3e5a9 100644
--- a/src/match.c
+++ b/src/match.c
@@ -104,6 +104,31 @@ guestfs___match3 (guestfs_h *g, const char *str, const pcre
*re,
   return 1;
 }
 
+/* Match a regular expression which contains exactly four captures. */
+int
+guestfs___match4 (guestfs_h *g, const char *str, const pcre *re,
+                  char **ret1, char **ret2, char **ret3, char **ret4)
+{
+  size_t len = strlen (str);
+  int vec[30], r;
+
+  r = pcre_exec (re, NULL, str, len, 0, 0, vec, 30);
+  if (r == PCRE_ERROR_NOMATCH)
+    return 0;
+
+  *ret1 = NULL;
+  *ret2 = NULL;
+  *ret3 = NULL;
+  *ret4 = NULL;
+
+  if (r > 1) *ret1 = safe_strndup (g, &str[vec[2]], vec[3]-vec[2]);
+  if (r > 2) *ret2 = safe_strndup (g, &str[vec[4]], vec[5]-vec[4]);
+  if (r > 3) *ret3 = safe_strndup (g, &str[vec[6]], vec[7]-vec[6]);
+  if (r > 4) *ret4 = safe_strndup (g, &str[vec[8]], vec[9]-vec[8]);
+
+  return 1;
+}
+
 /* Match a regular expression which contains exactly six captures. */
 int
 guestfs___match6 (guestfs_h *g, const char *str, const pcre *re,
-- 
1.8.4.2

On Sun, Nov 03, 2013 at 11:16:23PM +0200, Nikos Skalkotos
wrote:
> +    char type_c = (strcmp (type, "vtbd") == 0) ? 'v' :
's';
>      int disk_i = guestfs___parse_unsigned_int (g, disk);
>      int slice_i = guestfs___parse_unsigned_int (g, slice);
>      int part_i = part[0] - 'a' /* counting from 0 */;
> @@ -1481,7 +1501,7 @@ resolve_fstab_device (guestfs_h *g, const char *spec,
Hash_table *md_map)
>      if (disk_i != -1 && disk_i <= 26 &&
>          slice_i > 0 && slice_i <= 1 /* > 4 .. see comment
above */ &&
>          part_i >= 0 && part_i < 26) {
> -      device = safe_asprintf (g, "/dev/sd%c%d", disk_i +
'a', part_i + 5);
> +      device = safe_asprintf (g, "/dev/%cd%c%d", type_c, disk_i
+ 'a', part_i + 5);
I think this part is wrong: In libguestfs, the device should always be
called /dev/sdX.  I think it's better just to leave out these two
hunks.

The rest all seems sensible.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)

Hello Richard,

I'll send you the patch tomorrow again without the type translation.

Regards,
Nikos


On Mon 04 Nov 2013 23:41:26 EET, Richard W.M. Jones
wrote:
> On Sun, Nov 03, 2013 at 11:16:23PM +0200, Nikos Skalkotos wrote:
>> +    char type_c = (strcmp (type, "vtbd") == 0) ? 'v'
: 's';
>>      int disk_i = guestfs___parse_unsigned_int (g, disk);
>>      int slice_i = guestfs___parse_unsigned_int (g, slice);
>>      int part_i = part[0] - 'a' /* counting from 0 */;
>> @@ -1481,7 +1501,7 @@ resolve_fstab_device (guestfs_h *g, const char
*spec, Hash_table *md_map)
>>      if (disk_i != -1 && disk_i <= 26 &&
>>          slice_i > 0 && slice_i <= 1 /* > 4 .. see
comment above */ &&
>>          part_i >= 0 && part_i < 26) {
>> -      device = safe_asprintf (g, "/dev/sd%c%d", disk_i +
'a', part_i + 5);
>> +      device = safe_asprintf (g, "/dev/%cd%c%d", type_c,
disk_i + 'a', part_i + 5);
>
> I think this part is wrong: In libguestfs, the device should always be
> called /dev/sdX.  I think it's better just to leave out these two
> hunks.
>
> The rest all seems sensible.
>
> Rich.
>