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