Richard W.M. Jones
2021-Jun-01 13:37 UTC
[Libguestfs] [PATCH INCOMPLETE] daemon: Reimplement LVM filters using lvmdevices command
LVM2 managed to break device filters. This patch attempts a fix. https://bugzilla.redhat.com/show_bug.cgi?id=1965941 However it is not working, because the new "lvmdevices" command which is supposed to be used to set the new filters does not work for partitioned whole devices. As far as I can tell there is no way to emulate the old behaviour of filtering such a device. (Adding filters for each partition in the device is one possible workaround, but the filters will be out of date as soon as a new partition is created on the device.) While I investigate this, I'm posting the patch FYI. Rich.
Richard W.M. Jones
2021-Jun-01 13:37 UTC
[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
Nir Soffer
2021-Jun-01 21:23 UTC
[Libguestfs] [PATCH INCOMPLETE] daemon: Reimplement LVM filters using lvmdevices command
On Tue, Jun 1, 2021 at 4:38 PM Richard W.M. Jones <rjones at redhat.com> wrote:> > LVM2 managed to break device filters. This patch attempts a fix. > > https://bugzilla.redhat.com/show_bug.cgi?id=1965941 > > However it is not working, because the new "lvmdevices" command which > is supposed to be used to set the new filters does not work for > partitioned whole devices. As far as I can tell there is no way to > emulate the old behaviour of filtering such a device. (Adding filters > for each partition in the device is one possible workaround, but the > filters will be out of date as soon as a new partition is created on > the device.)The best way to use lvm filter is to use a filter that includes only the devices you want lvm to use, and reject everything else: "a|^/dev/sda1$|", "r|.*|" With this there are never any surprises when someone adds a new device or partition. You need to add the new device to the filter to use it. This is how ovirt configures lvm filter on hosts so they use only the devices needed by the host and cannot access the devices used by guests, managed by ovirt. ovirt lvm commands override the host filter using: --config "device { filter = ["a|^/dev/sdb$|", "r|.*|"] }" So it can manage vgs used by ovirt to provide guest disks. With --config you don't need to modify the guest when you make changes. Using the new devices file should be much simpler since it keeps the same semantics without the need to manage a filter. Nir