[Libguestfs] [PATCH] btrfs: Fix btrfs_subvolume_list on F18

Matthew Booth mbooth at redhat.com
Thu Jan 24 14:43:18 UTC 2013


The output of btrfs subvolume list has changed in F18 to include generation,
which breaks the parsing in btrfs_subvolume_list. This change replaces sscanf
with a more robust regular expression. The new regular expression should also
handle the addition of future unexpected columns.

Fixes RHBZ#903620
---
 daemon/Makefile.am |   6 ++-
 daemon/btrfs.c     | 156 ++++++++++++++++++++++++++++++++++-------------------
 2 files changed, 105 insertions(+), 57 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index a05771e..d70dbd7 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -197,14 +197,16 @@ guestfsd_LDADD = \
 	$(LIBSOCKET) \
 	$(LIB_CLOCK_GETTIME) \
 	$(LIBINTL) \
-	$(SERVENT_LIB)
+	$(SERVENT_LIB) \
+	$(PCRE_LIBS)
 
 guestfsd_CPPFLAGS = -I$(top_srcdir)/gnulib/lib -I$(top_builddir)/gnulib/lib
 guestfsd_CFLAGS = \
 	$(WARN_CFLAGS) $(WERROR_CFLAGS) \
 	$(AUGEAS_CFLAGS) \
 	$(HIVEX_CFLAGS) \
-	$(YAJL_CFLAGS)
+	$(YAJL_CFLAGS) \
+	$(PCRE_CFLAGS)
 
 # Manual pages and HTML files for the website.
 man_MANS = guestfsd.8
diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 8ecde01..f13d025 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -21,12 +21,14 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <inttypes.h>
+#include <pcre.h>
 #include <string.h>
 #include <unistd.h>
 
 #include "daemon.h"
 #include "actions.h"
 #include "optgroups.h"
+#include "xstrtol.h"
 
 GUESTFSD_EXT_CMD(str_btrfs, btrfs);
 GUESTFSD_EXT_CMD(str_btrfstune, btrfstune);
@@ -321,97 +323,141 @@ do_btrfs_subvolume_create (const char *dest)
 guestfs_int_btrfssubvolume_list *
 do_btrfs_subvolume_list (const char *fs)
 {
-  const size_t MAX_ARGS = 64;
-  guestfs_int_btrfssubvolume_list *ret;
-  char *fs_buf;
-  const char *argv[MAX_ARGS];
-  size_t i = 0;
-  char *out, *err, **lines, *pos;
-  size_t nr_subvolumes;
-  int r;
-
-  fs_buf = sysroot_path (fs);
-  if (fs_buf == NULL) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
+  char **lines;
 
-  ADD_ARG (argv, i, str_btrfs);
-  ADD_ARG (argv, i, "subvolume");
-  ADD_ARG (argv, i, "list");
-  ADD_ARG (argv, i, fs_buf);
-  ADD_ARG (argv, i, NULL);
+  /* Execute 'btrfs subvolume list <fs>', and split the output into lines */
+  {
+    char *fs_buf = sysroot_path (fs);
+    if (fs_buf == NULL) {
+      reply_with_perror ("malloc");
+      return NULL;
+    }
 
-  r = commandv (&out, &err, argv);
-  free (fs_buf);
-  if (r == -1) {
-    reply_with_error ("%s: %s", fs, err);
+    size_t i = 0;
+    const size_t MAX_ARGS = 64;
+    const char *argv[MAX_ARGS];
+    ADD_ARG (argv, i, str_btrfs);
+    ADD_ARG (argv, i, "subvolume");
+    ADD_ARG (argv, i, "list");
+    ADD_ARG (argv, i, fs_buf);
+    ADD_ARG (argv, i, NULL);
+
+    char *out, *err;
+    int r = commandv (&out, &err, argv);
+    free (fs_buf);
+    if (r == -1) {
+      reply_with_error ("%s: %s", fs, err);
+      free (err);
+      return NULL;
+    }
     free (err);
-    return NULL;
-  }
-  free (err);
 
-  lines = split_lines (out);
-  free (out);
-  if (!lines)
-    return NULL;
+    lines = split_lines (out);
+    free (out); out = NULL;
+    if (!lines)
+      return NULL;
+  }
 
   /* Output is:
    *
-   * ID 256 top level 5 path test1
-   * ID 257 top level 5 path dir/test2
-   * ID 258 top level 5 path test3
+   * ID 256 gen 30 top level 5 path test1
+   * ID 257 gen 30 top level 5 path dir/test2
+   * ID 258 gen 30 top level 5 path test3
    *
-   * "ID <n>" is the subvolume ID.  "top level <n>" is the top level
-   * subvolume ID.  "path <str>" is the subvolume path, relative to
-   * the top of the filesystem.
+   * "ID <n>" is the subvolume ID.
+   * "gen <n>" is the generation when the root was created or last
+   * updated.
+   * "top level <n>" is the top level subvolume ID.
+   * "path <str>" is the subvolume path, relative to the top of the
+   * filesystem.
+   *
+   * Note that the order that each of the above is fixed, but
+   * different versions of btrfs may display different sets of data.
+   * Specifically, older versions of btrfs do not display gen.
    */
-  nr_subvolumes = count_strings (lines);
+
+  guestfs_int_btrfssubvolume_list *ret = NULL;
+  pcre *re = NULL;
+
+  size_t nr_subvolumes = count_strings (lines);
 
   ret = malloc (sizeof *ret);
   if (!ret) {
     reply_with_perror ("malloc");
-    free_stringslen (lines, nr_subvolumes);
-    return NULL;
+    goto error;
   }
+
   ret->guestfs_int_btrfssubvolume_list_len = nr_subvolumes;
   ret->guestfs_int_btrfssubvolume_list_val =
     calloc (nr_subvolumes, sizeof (struct guestfs_int_btrfssubvolume));
   if (ret->guestfs_int_btrfssubvolume_list_val == NULL) {
     reply_with_perror ("malloc");
-    free (ret);
-    free_stringslen (lines, nr_subvolumes);
-    return NULL;
+    goto error;
   }
 
-  for (i = 0; i < nr_subvolumes; ++i) {
+  const char *errptr;
+  int erroffset;
+  re = pcre_compile ("ID\\s+(\\d+).*\\s"
+                     "top level\\s+(\\d+).*\\s"
+                     "path\\s(.*)",
+                     0, &errptr, &erroffset, NULL);
+  if (re == NULL) {
+    reply_with_error ("pcre_compile (%i): %s", erroffset, errptr);
+    goto error;
+  }
+
+  for (size_t i = 0; i < nr_subvolumes; ++i) {
     /* To avoid allocations, reuse the 'line' buffer to store the
      * path.  Thus we don't need to free 'line', since it will be
      * freed by the calling (XDR) code.
      */
     char *line = lines[i];
+    #define N_MATCHES 4
+    int ovector[N_MATCHES * 3];
 
-    if (sscanf (line, "ID %" SCNu64 " top level %" SCNu64 " path ",
-                &ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_id,
-                &ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_top_level_id) != 2) {
+    if (pcre_exec (re, NULL, line, strlen (line), 0, 0,
+                   ovector, N_MATCHES * 3) < 0)
+    #undef N_MATCHES
+    {
     unexpected_output:
       reply_with_error ("unexpected output from 'btrfs subvolume list' command: %s", line);
-      free_stringslen (lines, nr_subvolumes);
-      free (ret->guestfs_int_btrfssubvolume_list_val);
-      free (ret);
-      return NULL;
+      goto error;
     }
 
-    pos = strstr (line, " path ");
-    if (pos == NULL)
+    struct guestfs_int_btrfssubvolume *this  =
+      &ret->guestfs_int_btrfssubvolume_list_val[i];
+
+    #if __WORDSIZE == 64
+      #define XSTRTOU64 xstrtoul
+    #else
+      #define XSTRTOU64 xstrtoull
+    #endif
+
+    if (XSTRTOU64 (line + ovector[2], NULL, 10,
+                   &this->btrfssubvolume_id, NULL) != LONGINT_OK)
+      goto unexpected_output;
+    if (XSTRTOU64 (line + ovector[4], NULL, 10,
+                   &this->btrfssubvolume_top_level_id, NULL) != LONGINT_OK)
       goto unexpected_output;
-    pos += 6;
 
-    memmove (line, pos, strlen (pos) + 1);
-    ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path = line;
+    #undef XSTRTOU64
+
+    memmove (line, line + ovector[6], ovector[7] + 1);
+    this->btrfssubvolume_path = line;
   }
 
+  free (lines);
+  pcre_free (re);
+
   return ret;
+
+error:
+  free_stringslen (lines, nr_subvolumes);
+  if (ret) free (ret->guestfs_int_btrfssubvolume_list_val);
+  free (ret);
+  if (re) pcre_free (re);
+
+  return NULL;
 }
 
 int
-- 
1.8.1




More information about the Libguestfs mailing list