[Libguestfs] [PATCH] daemon: Reimplement LVM filters using lvmdevices command

LVM recently broke filters (https://bugzilla.redhat.com/1957040) This fixes them using the new lvmdevices command. Incidental to this change, the code is reimplemented in OCaml. Reported-by: Yongkui Guo Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1965941 --- .gitignore | 1 + daemon/Makefile.am | 3 +- daemon/lvm-filter.c | 243 -------------------------------------- daemon/lvm_filter.ml | 45 +++++++ docs/C_SOURCE_FILES | 1 - generator/actions_core.ml | 2 + generator/daemon.ml | 26 ++-- 7 files changed, 68 insertions(+), 253 deletions(-) diff --git a/.gitignore b/.gitignore index ebb67850c..3c28f4340 100644 --- a/.gitignore +++ b/.gitignore @@ -98,6 +98,7 @@ Makefile.in /daemon/listfs.mli /daemon/lvm.mli /daemon/lvm_dm.mli +/daemon/lvm_filter.mli /daemon/lvm-tokenization.c /daemon/md.mli /daemon/mount.mli diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 6f13bd43c..5ceb4094d 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -150,7 +150,6 @@ guestfsd_SOURCES = \ ls.c \ luks.c \ lvm.c \ - lvm-filter.c \ lvm-tokenization.c \ md.c \ mkfs.c \ @@ -297,6 +296,7 @@ SOURCES_MLI = \ listfs.mli \ lvm.mli \ lvm_dm.mli \ + lvm_filter.mli \ lvm_utils.mli \ md.mli \ mount.mli \ @@ -332,6 +332,7 @@ SOURCES_ML = \ lvm.ml \ lvm_utils.ml \ lvm_dm.ml \ + lvm_filter.ml \ findfs.ml \ md.ml \ mount.ml \ diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c deleted file mode 100644 index c6dd35156..000000000 --- a/daemon/lvm-filter.c +++ /dev/null @@ -1,243 +0,0 @@ -/* libguestfs - the guestfsd daemon - * Copyright (C) 2010-2012 Red Hat Inc. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#include <config.h> - -#include <stdio.h> -#include <stdlib.h> -#include <inttypes.h> -#include <string.h> -#include <unistd.h> -#include <errno.h> -#include <error.h> -#include <sys/stat.h> -#include <sys/wait.h> - -#include <augeas.h> - -#include "c-ctype.h" -#include "ignore-value.h" - -#include "daemon.h" -#include "actions.h" - -static void debug_lvm_config (void); - -/* Read LVM_SYSTEM_DIR environment variable, or set it to a default - * value if the environment variable is not set. - */ -static char *lvm_system_dir; -static void get_lvm_system_dir (void) __attribute__((constructor)); -static void free_lvm_system_dir (void) __attribute__((destructor)); - -static void -get_lvm_system_dir (void) -{ - const char *p; - - p = getenv ("LVM_SYSTEM_DIR"); - if (p) { - lvm_system_dir = strdup (p); - if (lvm_system_dir == NULL) abort (); - } - if (!lvm_system_dir) { - lvm_system_dir = strdup ("/etc/lvm"); - if (lvm_system_dir == NULL) abort (); - } - fprintf (stderr, "lvm_system_dir = %s\n", lvm_system_dir); -} - -static void -free_lvm_system_dir (void) -{ - free (lvm_system_dir); -} - -/* Rewrite the 'filter = [ ... ]' line in lvm.conf. */ -static int -set_filter (char *const *filters) -{ - const char *filter_types[] = { "filter", "global_filter", NULL }; - CLEANUP_FREE char *conf = NULL; - FILE *fp; - size_t i, j; - - if (asprintf (&conf, "%s/lvm.conf", lvm_system_dir) == -1) { - reply_with_perror ("asprintf"); - return -1; - } - fp = fopen (conf, "we"); - if (fp == NULL) { - reply_with_perror ("open: %s", conf); - return -1; - } - - fprintf (fp, "devices {\n"); - for (j = 0; filter_types[j] != NULL; ++j) { - fprintf (fp, " %s = [\n", filter_types[j]); - fprintf (fp, " "); - - for (i = 0; filters[i] != NULL; ++i) { - if (i > 0) - fprintf (fp, ",\n "); - fprintf (fp, "\"%s\"", filters[i]); - } - - fprintf (fp, "\n"); - fprintf (fp, " ]\n"); - } - fprintf (fp, "}\n"); - - fclose (fp); - - debug_lvm_config (); - - return 0; -} - -static int -vgchange (const char *vgchange_flag) -{ - CLEANUP_FREE char *err = NULL; - int r = command (NULL, &err, "lvm", "vgchange", vgchange_flag, NULL); - if (r == -1) { - reply_with_error ("vgchange %s: %s", vgchange_flag, err); - return -1; - } - - return 0; -} - -/* Deactivate all VGs. */ -static int -deactivate (void) -{ - return vgchange ("-an"); -} - -/* Reactivate all VGs. */ -static int -reactivate (void) -{ - return vgchange ("-ay"); -} - -/* Clear the cache and rescan. */ -static int -rescan (void) -{ - char lvm_cache[64]; - snprintf (lvm_cache, sizeof lvm_cache, "%s/cache/.cache", lvm_system_dir); - - unlink (lvm_cache); - - CLEANUP_FREE char *err = NULL; - int r = command (NULL, &err, "lvm", "vgscan", "--cache", NULL); - if (r == -1) { - reply_with_error ("vgscan: %s", err); - return -1; - } - - return 0; -} - -/* Show what lvm thinks is the current config. Useful for debugging. */ -static void -debug_lvm_config (void) -{ - if (verbose) { - fprintf (stderr, "lvm config:\n"); - ignore_value (system ("lvm config")); - } -} - -/* 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_strings (char *const *devices) -{ - size_t i; - CLEANUP_FREE_STRINGSBUF DECLARE_STRINGSBUF (ret); - - 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: - * - * for single partitions: - * "a|^/dev/sda1$|", - * for whole block devices: - * "a|^/dev/sda$|", "a|^/dev/sda[0-9]|", - */ - const size_t slen = strlen (devices[i]); - - if (add_sprintf (&ret, "a|^%s$|", devices[i]) == -1) - return NULL; - - if (!c_isdigit (devices[i][slen-1])) { - /* whole block device */ - if (add_sprintf (&ret, "a|^%s[0-9]|", devices[i]) == -1) - return NULL; - } - } - if (add_string (&ret, "r|.*|") == -1) - return NULL; - - if (end_stringsbuf (&ret) == -1) - return NULL; - - return take_stringsbuf (&ret); -} - -int -do_lvm_set_filter (char *const *devices) -{ - CLEANUP_FREE_STRING_LIST char **filters = make_filter_strings (devices); - if (filters == NULL) - return -1; - - if (deactivate () == -1) - return -1; - - int r = set_filter (filters); - if (r == -1) - return -1; - - if (rescan () == -1) - return -1; - - return reactivate (); -} - -int -do_lvm_clear_filter (void) -{ - const char *const filters[2] = { "a/.*/", NULL }; - - if (deactivate () == -1) - return -1; - - if (set_filter ((char *const *) filters) == -1) - return -1; - - if (rescan () == -1) - return -1; - - return reactivate (); -} diff --git a/daemon/lvm_filter.ml b/daemon/lvm_filter.ml new file mode 100644 index 000000000..9893b164f --- /dev/null +++ b/daemon/lvm_filter.ml @@ -0,0 +1,45 @@ +(* guestfs-inspection + * Copyright (C) 2009-2021 Red Hat Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + *) + +open Unix +open Printf + +open Std_utils + +open Utils + +let lvm_system_dir = + try getenv "LVM_SYSTEM_DIR" with Not_found -> "/etc/lvm" + +let devices_file = lvm_system_dir ^ "/devices/system.devices" + +let rec set_filter devices = + empty_filter (); + Array.iter add_device_to_filter devices + +and add_device_to_filter device = + ignore (command "lvmdevices" ["--adddev"; device ]) + +(* Make the filter an empty file so no devices can be seen. *) +and empty_filter () = + with_openfile devices_file [O_WRONLY; O_TRUNC; O_CREAT; + O_NOCTTY; O_CLOEXEC] 0 + (fun _ -> ()) + +(* Clear the filter so all devices can be seen. *) +and clear_filter () = try unlink devices_file with Unix_error _ -> () diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index 6a97d8b0e..bd3953c8c 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -109,7 +109,6 @@ daemon/ldm.c daemon/link.c daemon/ls.c daemon/luks.c -daemon/lvm-filter.c daemon/lvm-tokenization.c daemon/lvm.c daemon/md.c diff --git a/generator/actions_core.ml b/generator/actions_core.ml index bb602ee02..a434e035d 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -5625,6 +5625,7 @@ To find a filesystem from the UUID, use C<guestfs_findfs_uuid>." }; { defaults with name = "lvm_set_filter"; added = (1, 5, 1); style = RErr, [StringList (Device, "devices")], []; + impl = OCaml "Lvm_filter.set_filter"; optional = Some "lvm2"; test_excuse = "cannot be tested with the current framework because the VG is being used by the mounted filesystem, so the 'vgchange -an' command we do first will fail"; shortdesc = "set LVM device filter"; @@ -5655,6 +5656,7 @@ filtering out that VG." }; { defaults with name = "lvm_clear_filter"; added = (1, 5, 1); style = RErr, [], []; + impl = OCaml "Lvm_filter.clear_filter"; test_excuse = "cannot be tested with the current framework because the VG is being used by the mounted filesystem, so the 'vgchange -an' command we do first will fail"; shortdesc = "clear LVM device filter"; longdesc = "\ diff --git a/generator/daemon.ml b/generator/daemon.ml index b1047427b..6c555f85d 100644 --- a/generator/daemon.ml +++ b/generator/daemon.ml @@ -788,23 +788,33 @@ let generate_daemon_caml_stubs () = ) optargs; List.iter ( fun arg -> - pr " args[%d] = " !i; + incr i; + let i = !i - 1 in (match arg with - | Bool n -> pr "Val_bool (%s)" n - | Int n -> pr "Val_int (%s)" n - | Int64 n -> pr "caml_copy_int64 (%s)" n + | Bool n -> pr " args[%d] = Val_bool (%s);\n" i n + | Int n -> pr " args[%d] = Val_int (%s);\n" i n + | Int64 n -> pr " args[%d] = caml_copy_int64 (%s);\n" i n | String ((PlainString|Device|Pathname|Dev_or_Path|Key), n) -> - pr "caml_copy_string (%s)" n + pr " args[%d] = caml_copy_string (%s);\n" i n | String ((Mountable|Mountable_or_Path), n) -> - pr "guestfs_int_daemon_copy_mountable (%s)" n + pr " args[%d] = guestfs_int_daemon_copy_mountable (%s);\n" i n | String _ -> assert false | OptString _ -> assert false + | StringList ((PlainString|Device|Pathname|Dev_or_Path|Key), n) -> + pr " {\n"; + pr " size_t i, n;\n"; + pr "\n"; + pr " n = guestfs_int_count_strings (%s);\n" n; + pr " args[%d] = caml_alloc (n, 0);\n" i; + pr " for (i = 0; i < n; ++i) {\n"; + pr " v = caml_copy_string (%s[i]);\n" n; + pr " Store_field (args[%d], i, v);\n" i; + pr " }\n"; + pr " }\n" | StringList _ -> assert false | BufferIn _ -> assert false | Pointer _ -> assert false ); - pr ";\n"; - incr i ) args; assert (!i = nr_args); -- 2.31.1