Libguestfs - Jan 2013 - [PATCH] btrfs: Fix btrfs_subvolume_list on F18

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     | 67 ++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index a05771e..1fe8e12 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..a940f0c 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -21,6 +21,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <inttypes.h>
+#include <pcre.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -326,7 +327,7 @@ do_btrfs_subvolume_list (const char *fs)
   char *fs_buf;
   const char *argv[MAX_ARGS];
   size_t i = 0;
-  char *out, *err, **lines, *pos;
+  char *out, *err, **lines;
   size_t nr_subvolumes;
   int r;
 
@@ -358,13 +359,18 @@ do_btrfs_subvolume_list (const char *fs)
 
   /* 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);
 
@@ -384,33 +390,64 @@ do_btrfs_subvolume_list (const char *fs)
     return NULL;
   }
 
+  const char *errptr;
+  int erroffset;
+  pcre *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);
+    free (ret->guestfs_int_btrfssubvolume_list_val);
+    free (ret);
+    free_stringslen (lines, nr_subvolumes);
+    return NULL;
+  }
+
   for (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 3
+    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);
+      pcre_free(re);
       return NULL;
     }
 
-    pos = strstr (line, " path ");
-    if (pos == NULL)
-      goto unexpected_output;
-    pos += 6;
+    #if __WORDSIZE == 64
+      #define STRTOU64 strtoul
+    #else
+      #define STRTOU64 strtoull
+    #endif
+
+    errno = 0;
+    ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_id +     
STRTOU64(line + ovector[0], NULL, 10);
+    if (errno == ERANGE) goto unexpected_output;
+    ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_top_level_id
+      STRTOU64(line + ovector[2], NULL, 10);
+    if (errno == ERANGE) goto unexpected_output;
 
-    memmove (line, pos, strlen (pos) + 1);
+    #undef STRTOU64
+
+    memmove (line, line + ovector[4], ovector[5] + 1);
     ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path = line;
   }
 
+  pcre_free(re);
+
   return ret;
 }
 
-- 
1.8.1

On Thu, Jan 24, 2013 at 12:16:39PM +0000, Matthew Booth
wrote:
> 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     | 67
++++++++++++++++++++++++++++++++++++++++++------------
>  2 files changed, 56 insertions(+), 17 deletions(-)
> 
> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
> index a05771e..1fe8e12 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)
Indentation seems to be broken in these two hunks.
>  # Manual pages and HTML files for the website.
>  man_MANS = guestfsd.8
> diff --git a/daemon/btrfs.c b/daemon/btrfs.c
> index 8ecde01..a940f0c 100644
> --- a/daemon/btrfs.c
> +++ b/daemon/btrfs.c
> @@ -21,6 +21,7 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <inttypes.h>
> +#include <pcre.h>
>  #include <string.h>
>  #include <unistd.h>
>  
> @@ -326,7 +327,7 @@ do_btrfs_subvolume_list (const char *fs)
>    char *fs_buf;
>    const char *argv[MAX_ARGS];
>    size_t i = 0;
> -  char *out, *err, **lines, *pos;
> +  char *out, *err, **lines;
>    size_t nr_subvolumes;
>    int r;
>  
> @@ -358,13 +359,18 @@ do_btrfs_subvolume_list (const char *fs)
>  
>    /* 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.
>     */
Can you make these lines a little shorter.  On emacs, M-q will format
the paragraph correctly.
>    nr_subvolumes = count_strings (lines);
>  
> @@ -384,33 +390,64 @@ do_btrfs_subvolume_list (const char *fs)
>      return NULL;
>    }
>  
> +  const char *errptr;
> +  int erroffset;
> +  pcre *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);
> +    free (ret->guestfs_int_btrfssubvolume_list_val);
> +    free (ret);
> +    free_stringslen (lines, nr_subvolumes);
> +    return NULL;
> +  }
This whole function is crying out for a common return path.  The
current code 3 separate places where stuff is freed on error, and this
adds another one.  Not that this is your fault or anything, but it
would surely be an improvement in the code if you could make this
change.
>    for (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 3
> +    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);
> +      pcre_free(re);
>        return NULL;
>      }
>  
> -    pos = strstr (line, " path ");
> -    if (pos == NULL)
> -      goto unexpected_output;
> -    pos += 6;
> +    #if __WORDSIZE == 64
> +      #define STRTOU64 strtoul
> +    #else
> +      #define STRTOU64 strtoull
> +    #endif
> +
> +    errno = 0;
> +    ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_id >
+      STRTOU64(line + ovector[0], NULL, 10);
> +    if (errno == ERANGE) goto unexpected_output;
> +   
ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_top_level_id >
+      STRTOU64(line + ovector[2], NULL, 10);
> +    if (errno == ERANGE) goto unexpected_output;
I have a feeling if Meyering was around he'd say that you should be
using 'xstrtol' from gnulib.  Compare the code in
'src/inspect-fs.c'
for example.  You could also use sscanf here ...
> -    memmove (line, pos, strlen (pos) + 1);
> +    #undef STRTOU64
> +
> +    memmove (line, line + ovector[4], ovector[5] + 1);
>      ret->guestfs_int_btrfssubvolume_list_val[i].btrfssubvolume_path =
line;
>    }
>  
> +  pcre_free(re);
> +
>    return ret;
>  }
>  
> -- 
> 1.8.1
You also need to add pcre (or whatever the library package is called
on Fedora / Debian / Arch) to the package list.

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#)

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

On Thu, Jan 24, 2013 at 02:43:18PM +0000, Matthew Booth
wrote:
> 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.
You still need to add pcre (or whatever the library package is
called on Fedora/Debian/Arch) to appliance/packagelist.in.

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#)

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
---
 appliance/packagelist.in |   3 +
 daemon/Makefile.am       |   6 +-
 daemon/btrfs.c           | 156 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 108 insertions(+), 57 deletions(-)

diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index ead6d41..a43039c 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -42,6 +42,7 @@
   ntfsprogs
   ntfs-3g
   openssh-clients
+  pcre
   reiserfs-utils
   libselinux
   systemd /* for /sbin/reboot and udevd */
@@ -65,6 +66,7 @@
   libaugeas0
   libcap2
   libhivex0
+  libpcre3
   libyajl2
   linux-image
   nilfs-tools
@@ -95,6 +97,7 @@
   nilfs-utils
   ntfsprogs
   ntfs-3g
+  pcre
   reiserfsprogs
   systemd
   vim
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 1a1626f..b7731ac 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -198,14 +198,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

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
---
 appliance/packagelist.in |   3 +
 daemon/Makefile.am       |   6 +-
 daemon/btrfs.c           | 149 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 104 insertions(+), 54 deletions(-)

diff --git a/appliance/packagelist.in b/appliance/packagelist.in
index ead6d41..a43039c 100644
--- a/appliance/packagelist.in
+++ b/appliance/packagelist.in
@@ -42,6 +42,7 @@
   ntfsprogs
   ntfs-3g
   openssh-clients
+  pcre
   reiserfs-utils
   libselinux
   systemd /* for /sbin/reboot and udevd */
@@ -65,6 +66,7 @@
   libaugeas0
   libcap2
   libhivex0
+  libpcre3
   libyajl2
   linux-image
   nilfs-tools
@@ -95,6 +97,7 @@
   nilfs-utils
   ntfsprogs
   ntfs-3g
+  pcre
   reiserfsprogs
   systemd
   vim
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index eac9058..a1e5e68 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -194,7 +194,8 @@ guestfsd_LDADD = \
 	$(LIBSOCKET) \
 	$(LIB_CLOCK_GETTIME) \
 	$(LIBINTL) \
-	$(SERVENT_LIB)
+	$(SERVENT_LIB) \
+	$(PCRE_LIBS)
 
 guestfsd_CPPFLAGS = \
 	-I$(top_srcdir)/gnulib/lib \
@@ -204,7 +205,8 @@ 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 c100d18..81ce5f5 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);
@@ -307,94 +309,137 @@ 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;
-  CLEANUP_FREE char *fs_buf = NULL;
-  const char *argv[MAX_ARGS];
-  size_t i = 0;
-  char **lines, *pos;
-  CLEANUP_FREE char *out = NULL, *err = NULL;
-  size_t nr_subvolumes;
-  int r;
+  char **lines;
 
-  fs_buf = sysroot_path (fs);
-  if (fs_buf == NULL) {
-    reply_with_perror ("malloc");
-    return NULL;
-  }
+  /* Execute 'btrfs subvolume list <fs>', and split the output
into lines */
+  {
+    CLEANUP_FREE char *fs_buf = sysroot_path (fs);
+    if (fs_buf == NULL) {
+      reply_with_perror ("malloc");
+      return NULL;
+    }
 
-  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);
+    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);
+
+    CLEANUP_FREE char *out = NULL, *err = NULL;
+    int r = commandv (&out, &err, argv);
+    if (r == -1) {
+      reply_with_error ("%s: %s", fs, err);
+      return NULL;
+    }
 
-  r = commandv (&out, &err, argv);
-  if (r == -1) {
-    reply_with_error ("%s: %s", fs, err);
-    return NULL;
+    lines = split_lines (out);
+    if (!lines)
+      return NULL;
   }
 
-  lines = split_lines (out);
-  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;
+  }
+
+  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 (i = 0; i < nr_subvolumes; ++i) {
+  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