Libguestfs - May 2022 - [libguestfs PATCH] appliance, daemon: disable lvm2 devicesfile

In guestfs-tools commit 4fe8a03cd2d3 ('sysprep: remove lvm2's default
"system.devices" file', 2022-04-11), we disabled the use of
LVM2's new
"devicesfile" feature, which could interfere with the cloning of
virtual
machines.

We suspected in

  https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c6

that the same lvm2 feature could affect the libguestfs appliance itself,
but decided in

  https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c8
  https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c10

that this would not be the case, because "appliance/init" already
constructed a pristine LVM_SYSTEM_DIR.

Unfortunately, that's not enough: due to the "use_devicesfile=1"
default
(on RHEL9 anyway), some "lvm" invocation, possibly inside the
lvm-set-filter API, *creates*
"$LVM_SYSTEM_DIR/devices/system.devices".
And then we get (minimally) warnings such as
> Please remove the lvm.conf global_filter, it is ignored with the devices
> file.
> Please remove the lvm.conf filter, it is ignored with the devices file.
when using the lvm-set-filter API.

Explicitly disable the "devices file" in "appliance/init",
and also
whenever we rewrite "lvm.conf" -- that is, in set_filter()
[daemon/lvm-filter.c]. In the former, check for the feature by locating
the devicesfile-related utilities "lvmdevices" and
"vgimportdevices". In
the C code, invoke the utilities with the "--help" option instead. (In
"appliance/init",  I thought it was best not to call any lvm2
utilities
even with "--help", with our lvm2.conf still under construction
there.) If
either utility is available, set "use_devicesfile = 0".

Cc: David Teigland <teigland at redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1965941
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    Tested:
    
    - on top of "master" (7afbf5ee4415), using a Fedora 35 host, where
the
      lvm2 packages copied into the appliance do not have the
"devicesfile"
      feature (-> regression test);
    
    - on top of "rhel-9.1" (ad24b9f4d695), in the form of a Brew
scratch
      build, using a nightly RHEL-9.1 install, where the lvm2 packages
      copied into the appliance have the "devicesfile" feature (->
bugfix
      test).

 daemon/lvm-filter.c | 19 +++++++++++++++++++
 appliance/init      | 11 +++++++++++
 2 files changed, 30 insertions(+)

diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
index c6dd35156d37..f75e2198831e 100644
--- a/daemon/lvm-filter.c
+++ b/daemon/lvm-filter.c
@@ -68,6 +68,18 @@ free_lvm_system_dir (void)
   free (lvm_system_dir);
 }
 
+static bool devicesfile_feature (void)
+{
+  static bool checked, available;
+
+  if (!checked) {
+    checked = true;
+    available = command (NULL, NULL, "lvmdevices",
"--help", NULL) == 0 ||
+                command (NULL, NULL, "vgimportdevices",
"--help", NULL) == 0;
+  }
+  return available;
+}
+
 /* Rewrite the 'filter = [ ... ]' line in lvm.conf. */
 static int
 set_filter (char *const *filters)
@@ -88,6 +100,13 @@ set_filter (char *const *filters)
   }
 
   fprintf (fp, "devices {\n");
+
+  /* If lvm2 supports a "devices file", we need to disable its use
+   * (RHBZ#1965941).
+   */
+  if (devicesfile_feature ())
+    fprintf (fp, "    use_devicesfile = 0\n");
+
   for (j = 0; filter_types[j] != NULL; ++j) {
     fprintf (fp, "    %s = [\n", filter_types[j]);
     fprintf (fp, "        ");
diff --git a/appliance/init b/appliance/init
index 7076821d2250..19aa151b73aa 100755
--- a/appliance/init
+++ b/appliance/init
@@ -134,6 +134,17 @@ mdadm -As --auto=yes --no-degraded
 # Empty LVM configuration file means "all defaults".
 mkdir -p /tmp/lvm
 touch /tmp/lvm/lvm.conf
+
+# If lvm2 supports a "devices file", we need to disable its use
+# (RHBZ#1965941).
+if command -v lvmdevices || command -v vgimportdevices; then
+  {
+    printf 'devices {\n'
+    printf '\tuse_devicesfile = 0\n'
+    printf '}\n'
+  } >> /tmp/lvm/lvm.conf
+fi
+
 LVM_SYSTEM_DIR=/tmp/lvm
 export LVM_SYSTEM_DIR
 lvmetad

base-commit: 7afbf5ee4415f6fa2553898d3af238e794062096
-- 
2.19.1.3.g30247aa5d201

On Mon, May 30, 2022 at 04:10:27PM +0200, Laszlo Ersek
wrote:
> In guestfs-tools commit 4fe8a03cd2d3 ('sysprep: remove lvm2's
default
> "system.devices" file', 2022-04-11), we disabled the use of
LVM2's new
> "devicesfile" feature, which could interfere with the cloning of
virtual
> machines.
> 
> We suspected in
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c6
> 
> that the same lvm2 feature could affect the libguestfs appliance itself,
> but decided in
> 
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c8
>   https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c10
> 
> that this would not be the case, because "appliance/init" already
> constructed a pristine LVM_SYSTEM_DIR.
> 
> Unfortunately, that's not enough: due to the
"use_devicesfile=1" default
> (on RHEL9 anyway), some "lvm" invocation, possibly inside the
> lvm-set-filter API, *creates*
"$LVM_SYSTEM_DIR/devices/system.devices".
> And then we get (minimally) warnings such as
> 
> > Please remove the lvm.conf global_filter, it is ignored with the
devices
> > file.
> > Please remove the lvm.conf filter, it is ignored with the devices
file.
> 
> when using the lvm-set-filter API.
> 
> Explicitly disable the "devices file" in
"appliance/init", and also
> whenever we rewrite "lvm.conf" -- that is, in set_filter()
> [daemon/lvm-filter.c]. In the former, check for the feature by locating
> the devicesfile-related utilities "lvmdevices" and
"vgimportdevices". In
> the C code, invoke the utilities with the "--help" option
instead. (In
> "appliance/init",  I thought it was best not to call any lvm2
utilities
> even with "--help", with our lvm2.conf still under construction
there.) If
> either utility is available, set "use_devicesfile = 0".
> 
> Cc: David Teigland <teigland at redhat.com>
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1965941
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
> 
> Notes:
>     Tested:
>     
>     - on top of "master" (7afbf5ee4415), using a Fedora 35 host,
where the
>       lvm2 packages copied into the appliance do not have the
"devicesfile"
>       feature (-> regression test);
>     
>     - on top of "rhel-9.1" (ad24b9f4d695), in the form of a Brew
scratch
>       build, using a nightly RHEL-9.1 install, where the lvm2 packages
>       copied into the appliance have the "devicesfile" feature
(-> bugfix
>       test).
> 
>  daemon/lvm-filter.c | 19 +++++++++++++++++++
>  appliance/init      | 11 +++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
> index c6dd35156d37..f75e2198831e 100644
> --- a/daemon/lvm-filter.c
> +++ b/daemon/lvm-filter.c
> @@ -68,6 +68,18 @@ free_lvm_system_dir (void)
>    free (lvm_system_dir);
>  }
>  
> +static bool devicesfile_feature (void)
> +{
> +  static bool checked, available;
> +
> +  if (!checked) {
> +    checked = true;
> +    available = command (NULL, NULL, "lvmdevices",
"--help", NULL) == 0 ||
> +                command (NULL, NULL, "vgimportdevices",
"--help", NULL) == 0;
> +  }
> +  return available;
> +}
> +
>  /* Rewrite the 'filter = [ ... ]' line in lvm.conf. */
>  static int
>  set_filter (char *const *filters)
> @@ -88,6 +100,13 @@ set_filter (char *const *filters)
>    }
>  
>    fprintf (fp, "devices {\n");
> +
> +  /* If lvm2 supports a "devices file", we need to disable its
use
> +   * (RHBZ#1965941).
> +   */
> +  if (devicesfile_feature ())
> +    fprintf (fp, "    use_devicesfile = 0\n");
> +
>    for (j = 0; filter_types[j] != NULL; ++j) {
>      fprintf (fp, "    %s = [\n", filter_types[j]);
>      fprintf (fp, "        ");
> diff --git a/appliance/init b/appliance/init
> index 7076821d2250..19aa151b73aa 100755
> --- a/appliance/init
> +++ b/appliance/init
> @@ -134,6 +134,17 @@ mdadm -As --auto=yes --no-degraded
>  # Empty LVM configuration file means "all defaults".
>  mkdir -p /tmp/lvm
>  touch /tmp/lvm/lvm.conf
> +
> +# If lvm2 supports a "devices file", we need to disable its use
> +# (RHBZ#1965941).
> +if command -v lvmdevices || command -v vgimportdevices; then
> +  {
> +    printf 'devices {\n'
> +    printf '\tuse_devicesfile = 0\n'
> +    printf '}\n'
> +  } >> /tmp/lvm/lvm.conf
> +fi
> +
>  LVM_SYSTEM_DIR=/tmp/lvm
>  export LVM_SYSTEM_DIR
>  lvmetad
> 
> base-commit: 7afbf5ee4415f6fa2553898d3af238e794062096
> -- 
> 2.19.1.3.g30247aa5d201
Looks reasonable to me, so:

Acked-by: Richard W.M. Jones <rjones at redhat.com>

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org