[Libguestfs] [PATCH] daemon: Reimplement LVM filters using lvmdevices command
Richard W.M. Jones
rjones at redhat.com
Tue Jun 1 13:37:34 UTC 2021
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
More information about the Libguestfs
mailing list