Bring the add_sprintf function for stringsbuf from the library. --- daemon/daemon.h | 2 ++ daemon/guestfsd.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/daemon/daemon.h b/daemon/daemon.h index 0caad45..705aa95 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -89,6 +89,8 @@ struct stringsbuf { */ extern int add_string_nodup (struct stringsbuf *sb, char *str); extern int add_string (struct stringsbuf *sb, const char *str); +extern int add_sprintf (struct stringsbuf *sb, const char *fs, ...) + __attribute__((format (printf,2,3))); extern int end_stringsbuf (struct stringsbuf *sb); extern size_t count_strings (char *const *argv); diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index 0a59c84..321544f 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -560,6 +560,26 @@ add_string (struct stringsbuf *sb, const char *str) } int +add_sprintf (struct stringsbuf *sb, const char *fs, ...) +{ + va_list args; + char *str; + int r; + + va_start (args, fs); + r = vasprintf (&str, fs, args); + va_end (args); + if (r == -1) { + reply_with_perror ("vasprintf"); + free_stringslen (sb->argv, sb->size); + sb->argv = NULL; + return -1; + } + + return add_string_nodup (sb, str); +} + +int end_stringsbuf (struct stringsbuf *sb) { return add_string_nodup (sb, NULL); -- 1.9.3

The way to set the filter for lvm devices was to open lvm.conf, look for uncommented "filter =" lines and replace the configuration there. This had the issue that if there is no uncommented filter line, then the filter cannot be changed at all; considering newer lvm2 releases ship a sample configuration with no uncommented filter lines, then the old way became wrong and not sufficient. Instead, take a copy of the upstream lvm.aug lens, with a simple change to allow parsing also negative values in configuration, and install it in the daemon. When asking to change the lvm filter, use augeas making sure to use this custom lens for the lvm.conf copy used within the appliance. --- appliance/Makefile.am | 6 +- appliance/guestfs_lvm_conf.aug | 74 +++++++++++++++++ daemon/lvm-filter.c | 183 +++++++++++++++++++++-------------------- 3 files changed, 172 insertions(+), 91 deletions(-) create mode 100644 appliance/guestfs_lvm_conf.aug diff --git a/appliance/Makefile.am b/appliance/Makefile.am index 418a6f6..bb0c0e8 100644 --- a/appliance/Makefile.am +++ b/appliance/Makefile.am @@ -21,6 +21,7 @@ EXTRA_DIST = \ 99-guestfs-serial.rules \ excludefiles.in \ guestfsd.suppressions \ + guestfs_lvm_conf.aug \ hostfiles.in \ init \ libguestfs-make-fixed-appliance.in \ @@ -74,12 +75,13 @@ packagelist: packagelist.in Makefile cmp -s $@ $@-t || mv $@-t $@ rm -f $@-t -supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions +supermin.d/daemon.tar.gz: ../daemon/guestfsd guestfsd.suppressions guestfs_lvm_conf.aug rm -f $@ $@-t rm -rf tmp-d - mkdir -p tmp-d$(DAEMON_SUPERMIN_DIR) tmp-d/etc + mkdir -p tmp-d$(DAEMON_SUPERMIN_DIR) tmp-d/etc tmp-d/usr/share/guestfs ln ../daemon/guestfsd tmp-d$(DAEMON_SUPERMIN_DIR)/guestfsd ln $(srcdir)/guestfsd.suppressions tmp-d/etc/guestfsd.suppressions + ln $(srcdir)/guestfs_lvm_conf.aug tmp-d/usr/share/guestfs/guestfs_lvm_conf.aug ( cd tmp-d && tar zcf - * ) > $@-t rm -r tmp-d mv $@-t $@ diff --git a/appliance/guestfs_lvm_conf.aug b/appliance/guestfs_lvm_conf.aug new file mode 100644 index 0000000..ffa5b01 --- /dev/null +++ b/appliance/guestfs_lvm_conf.aug @@ -0,0 +1,74 @@ +(* +Module: LVM + Parses LVM metadata. + +Author: Gabriel de Perthuis <g2p.code+augeas(a)gmail.com&gt; + +About: License + This file is licensed under the LGPL v2+. + +About: Configuration files + This lens applies to files in /etc/lvm/backup and /etc/lvm/archive. + +About: Examples + The <Test_LVM> file contains various examples and tests. +*) + +module Guestfs_LVM_conf = + autoload xfm + + (* See lvm2/libdm/libdm-config.c for tokenisation; + * libdm uses a blacklist but I prefer the safer whitelist approach. *) + (* View: identifier + * The left hand side of a definition *) + let identifier = /[a-zA-Z0-9_-]+/ + + (* strings can contain backslash-escaped dquotes, but I don't know + * how to get the message across to augeas *) + let str = [label "str". Quote.do_dquote (store /([^\"]|\\\\.)*/)] + let int = [label "int". store Rx.relinteger] + (* View: flat_literal + * A literal without structure *) + let flat_literal = int|str + + (* allow multiline and mixed int/str, used for raids and stripes *) + (* View: list + * A list containing flat literals *) + let list = [ + label "list" . counter "list" + . del /\[[ \t\n]*/ "[" + .([seq "list". flat_literal . del /,[ \t\n]*/ ", "]* + . [seq "list". flat_literal . del /[ \t\n]*/ ""])? + . Util.del_str "]"] + + (* View: val + * Any value that appears on the right hand side of an assignment *) + let val = flat_literal | list + + (* View: nondef + * A line that doesn't contain a statement *) + let nondef = + Util.empty + | Util.comment + + (* Build.block couldn't be reused, because of recursion and + * a different philosophy of whitespace handling. *) + (* View: def + * An assignment, or a block containing definitions *) + let rec def = [ + Util.indent . key identifier . ( + del /[ \t]*\{\n/ " {\n" + .[label "dict".(nondef | def)*] + . Util.indent . Util.del_str "}\n" + |Sep.space_equal . val . Util.comment_or_eol)] + + (* View: lns + * The main lens *) + let lns = (nondef | def)* + + let filter = + incl "/etc/lvm/archive/*.vg" + . incl "/etc/lvm/backup/*" + . Util.stdexcl + + let xfm = transform lns filter diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c index 6976bab..3bab9bf 100644 --- a/daemon/lvm-filter.c +++ b/daemon/lvm-filter.c @@ -25,12 +25,30 @@ #include <unistd.h> #include <sys/stat.h> +#include <augeas.h> + #include "c-ctype.h" #include "ignore-value.h" #include "daemon.h" #include "actions.h" +#ifdef HAVE_ATTRIBUTE_CLEANUP +#define CLEANUP_AUG_CLOSE __attribute__((cleanup(cleanup_aug_close))) + +static void +cleanup_aug_close (void *ptr) +{ + augeas *aug = * (augeas **) ptr; + + if (aug != NULL) + aug_close (aug); +} + +#else +#define CLEANUP_AUG_CLOSE +#endif + GUESTFSD_EXT_CMD(str_lvm, lvm); GUESTFSD_EXT_CMD(str_cp, cp); GUESTFSD_EXT_CMD(str_rm, rm); @@ -107,87 +125,76 @@ rm_lvm_system_dir (void) ignore_value (system (cmd)); } -/* Does the current line match the regexp /^\s*filter\s*=/ */ +/* Rewrite the 'filter = [ ... ]' line in lvm.conf. */ static int -is_filter_line (const char *line) +set_filter (char *const *filters) { - while (*line && c_isspace (*line)) - line++; - if (!*line) - return 0; - - if (! STRPREFIX (line, "filter")) - return 0; - line += 6; - - while (*line && c_isspace (*line)) - line++; - if (!*line) - return 0; - - if (*line != '=') - return 0; + CLEANUP_AUG_CLOSE augeas *aug = NULL; + int r; + int count; - return 1; -} + /* Small optimization: do not load the files at init time, + * but do that only after having applied the transformation. + */ + const int flags = AUG_NO_ERR_CLOSE | AUG_NO_LOAD; + aug = aug_init (lvm_system_dir, "/usr/share/guestfs/", flags); + if (!aug) { + reply_with_error ("augeas initialization failed"); + return -1; + } -/* Rewrite the 'filter = [ ... ]' line in lvm.conf. */ -static int -set_filter (const char *filter) -{ - char lvm_conf[64]; - snprintf (lvm_conf, sizeof lvm_conf, "%s/lvm/lvm.conf", lvm_system_dir); + if (aug_error (aug) != AUG_NOERROR) { + AUGEAS_ERROR ("aug_init"); + return -1; + } - char lvm_conf_new[64]; - snprintf (lvm_conf_new, sizeof lvm_conf, "%s/lvm/lvm.conf.new", - lvm_system_dir); + r = aug_transform (aug, "guestfs_lvm_conf", "/lvm/lvm.conf", + 0 /* = included */); + if (r == -1) { + AUGEAS_ERROR ("aug_transform"); + return -1; + } - FILE *ifp = fopen (lvm_conf, "r"); - if (ifp == NULL) { - reply_with_perror ("open: %s", lvm_conf); + if (aug_load (aug) == -1) { + AUGEAS_ERROR ("aug_load"); return -1; } - FILE *ofp = fopen (lvm_conf_new, "w"); - if (ofp == NULL) { - reply_with_perror ("open: %s", lvm_conf_new); - fclose (ifp); + + /* Remove all the old filters ... */ + r = aug_rm (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*"); + if (r == -1) { + AUGEAS_ERROR ("aug_rm"); return -1; } - CLEANUP_FREE char *line = NULL; - size_t allocsize = 0; - while (getline (&line, &allocsize, ifp) != -1) { - int r; - if (is_filter_line (line)) { - r = fprintf (ofp, " filter = [ %s ]\n", filter); - } else { - r = fprintf (ofp, "%s", line); - } - if (r < 0) { - /* NB. fprintf doesn't set errno on error. */ - reply_with_error ("%s: write failed", lvm_conf_new); - fclose (ifp); - fclose (ofp); - unlink (lvm_conf_new); + /* ... and add the new ones. */ + for (count = 0; filters[count] != NULL; ++count) { + char buf[128]; + + snprintf (buf, sizeof buf, + "/files/lvm/lvm.conf/devices/dict/filter/list/%d/str", + count + 1); + + if (aug_set (aug, buf, filters[count]) == -1) { + AUGEAS_ERROR ("aug_set: %d: %s", count, filters[count]); return -1; } } - if (fclose (ifp) == EOF) { - reply_with_perror ("close: %s", lvm_conf); - unlink (lvm_conf_new); - fclose (ofp); + /* Safety check for the written filter nodes. */ + r = aug_match (aug, "/files/lvm/lvm.conf/devices/dict/filter/list/*/str", + NULL); + if (r == -1) { + AUGEAS_ERROR ("aug_match"); return -1; } - if (fclose (ofp) == EOF) { - reply_with_perror ("close: %s", lvm_conf_new); - unlink (lvm_conf_new); + if (r != count) { + reply_with_error ("filters# vs matches mismatch: %d vs %d", count, r); return -1; } - if (rename (lvm_conf_new, lvm_conf) == -1) { - reply_with_perror ("rename: %s", lvm_conf); - unlink (lvm_conf_new); + if (aug_save (aug) == -1) { + AUGEAS_ERROR ("aug_save"); return -1; } @@ -240,25 +247,16 @@ rescan (void) return 0; } -/* Construct the new, specific filter string. We can assume that +/* Construct the new, specific filter strings. We can assume that * the 'devices' array does not contain any regexp metachars, * because it's already been checked by the stub code. */ -static char * -make_filter_string (char *const *devices) +static char ** +make_filter_strings (char *const *devices) { size_t i; - size_t len = 64; - for (i = 0; devices[i] != NULL; ++i) - len += 2 * strlen (devices[i]) + 64; - - char *filter = malloc (len); - if (filter == NULL) { - reply_with_perror ("malloc"); - return NULL; - } + DECLARE_STRINGSBUF (ret); - char *p = filter; for (i = 0; devices[i] != NULL; ++i) { /* Because of the way matching works in LVM (yes, they wrote their * own regular expression engine!), each match clause should be either: @@ -269,33 +267,38 @@ make_filter_string (char *const *devices) * "a|^/dev/sda$|", "a|^/dev/sda[0-9]|", */ size_t slen = strlen (devices[i]); - char str[2*slen+64]; - if (c_isdigit (devices[i][slen-1])) /* single partition */ - snprintf (str, 2*slen+64, "\"a|^%s$|\", ", devices[i]); - else /* whole block device */ - snprintf (str, 2*slen+64, "\"a|^%s$|\", \"a|^%s[0-9]|\", ", - devices[i], devices[i]); + if (add_sprintf (&ret, "a|^%s$|", devices[i]) == -1) + goto error; - strcpy (p, str); - p += strlen (str); + if (!c_isdigit (devices[i][slen-1])) { + /* whole block device */ + if (add_sprintf (&ret, "a|^%s[0-9]|", devices[i]) == -1) + goto error; + } } - strcpy (p, "\"r|.*|\""); - - return filter; /* Caller must free. */ + if (add_string (&ret, "r|.*|") == -1) + goto error; + + end_stringsbuf (&ret); + return ret.argv; +error: + if (ret.argv) + free_stringslen (ret.argv, ret.size); + return NULL; } int do_lvm_set_filter (char *const *devices) { - CLEANUP_FREE char *filter = make_filter_string (devices); - if (filter == NULL) + CLEANUP_FREE_STRING_LIST char **filters = make_filter_strings (devices); + if (filters == NULL) return -1; if (deactivate () == -1) return -1; - int r = set_filter (filter); + int r = set_filter (filters); if (r == -1) return -1; @@ -308,10 +311,12 @@ do_lvm_set_filter (char *const *devices) int do_lvm_clear_filter (void) { + const char *const filters[2] = { "a/.*/", NULL }; + if (deactivate () == -1) return -1; - if (set_filter ("\"a/.*/\"") == -1) + if (set_filter ((char *const *) filters) == -1) return -1; if (rescan () == -1) -- 1.9.3