Bug 1965941

One line reproducer:

$ guestfish -N disk part-init /dev/sda mbr : part-add /dev/sda p 2 100000 : part-add /dev/sda p 100001 200000 : pvcreate /dev/sda1 : pvcreate /dev/sda2 : pvs : lvm-set-filter /dev/sda1 : pvs

Actual output (lvm2-9:2.03.12-3.el9.x86_64):

/dev/sda1
/dev/sda2
/dev/sda1
/dev/sda2

Expected output:

/dev/sda1
/dev/sda2
/dev/sda1

With debugging enabled, we see this additional warning which is not
present in the earlier LVM:

  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.

Caused by this change in LVM2: bug 1957040

While this bug is filed against RHEL 9, I believe it also affects (or
will affect) RHEL 8.5 and RHEL AV 8.5, since LVM has made this change
there too.

I have a proposed fix but it is not complete:

https://listman.redhat.com/archives/libguestfs/2021-June/msg00001.html

(In reply to Richard W.M. Jones from comment #4)
> While this bug is filed against RHEL 9, I believe it also affects (or
> will affect) RHEL 8.5 and RHEL AV 8.5, since LVM has made this change
> there too.

The devices file feature will be present in 8.5 but it will not be enabled by default in RHEL8, only in RHEL9.  In RHEL8 or RHEL9 you can set lvm.conf use_devicesfile=0 (or via --config), or you can add --devicesfile "" to an individual command to tell that command to not use a devices file (if one exists).

> I have a proposed fix but it is not complete:
> 
> https://listman.redhat.com/archives/libguestfs/2021-June/msg00001.html

I haven't studied this closely, but I expect you want to expose one or more specific devices to an lvm command which you did previously using a filter that contained only those devices?  You can continue to do that by running the command with that filter and disabling the devices file for that command.  Or, you could use the new --devices option in place of using a filter (--devices on the command line is introduced along with the devices file feature.)

If you run 'command --devices /dev/foo' the command will only be exposed to /dev/foo.  You can repeat the --devices option to expose the command to more than one device, e.g. --devices /dev/foo --devices /dev/bar (or --devices /dev/foo,/dev/bar).

Thanks.  We use the guestfs_lvm_set_filter API like this:

  guestfs_lvm_set_filter ["/dev/sda", "/dev/sdb", "/dev/sdc1"]
  ...
     some LVM commands that we only want to affect /dev/sda, /dev/sdb
     or any partitions therein, and /dev/sdc1, but NOT /dev/sdc2, NOT /dev/sdd etc
  ...
  guestfs_lvm_clear_filter ()

Previously these were translated into the following filters:

devices {
    filter = [
        "a|^/dev/sda$|",
        "a|^/dev/sda[0-9]|",
        "a|^/dev/sdb$|",
        "a|^/dev/sdb[0-9]|",
        "a|^/dev/sdc1$|",
        "r|.*|"
    ]
    global_filter = [
        "a|^/dev/sda$|",
        "a|^/dev/sda[0-9]|",
        "a|^/dev/sdb$|",
        "a|^/dev/sdb[0-9]|",
        "a|^/dev/sdc1$|",
        "r|.*|"
    ]
}

by the code here:
https://github.com/libguestfs/libguestfs/blob/master/daemon/lvm-filter.c

My attempt to translate this into lvmdevices commands was to use:

rm -f /etc/lvm/devices/system.devices
lvmdevices --adddev /dev/sda
lvmdevices --adddev /dev/sdb
lvmdevices --adddev /dev/sdc1

But if /dev/sda or /dev/sdb has a partition on it then the commands fail.

It's kind of awkward to add --devices to any LVM command we use, although
I guess it would be possible, but AIUI it doesn't solve the above problem.

(In reply to Richard W.M. Jones from comment #6)
> Thanks.  We use the guestfs_lvm_set_filter API like this:
> 
>   guestfs_lvm_set_filter ["/dev/sda", "/dev/sdb", "/dev/sdc1"]
>   ...
>      some LVM commands that we only want to affect /dev/sda, /dev/sdb
>      or any partitions therein, and /dev/sdc1, but NOT /dev/sdc2, NOT
> /dev/sdd etc

OK, that's interesting.

>   ...
>   guestfs_lvm_clear_filter ()
> 
> Previously these were translated into the following filters:
> 
> devices {
>     filter = [
>         "a|^/dev/sda$|",
>         "a|^/dev/sda[0-9]|",
>         "a|^/dev/sdb$|",
>         "a|^/dev/sdb[0-9]|",
>         "a|^/dev/sdc1$|",
>         "r|.*|"
>     ]
>     global_filter = [
>         "a|^/dev/sda$|",
>         "a|^/dev/sda[0-9]|",
>         "a|^/dev/sdb$|",
>         "a|^/dev/sdb[0-9]|",
>         "a|^/dev/sdc1$|",
>         "r|.*|"
>     ]
> }
> 
> by the code here:
> https://github.com/libguestfs/libguestfs/blob/master/daemon/lvm-filter.c
> 
> My attempt to translate this into lvmdevices commands was to use:
> 
> rm -f /etc/lvm/devices/system.devices
> lvmdevices --adddev /dev/sda
> lvmdevices --adddev /dev/sdb
> lvmdevices --adddev /dev/sdc1
> 
> But if /dev/sda or /dev/sdb has a partition on it then the commands fail.

So it's creating a filter to cover a limited range of device names, while not knowing exactly which device names will be needed.

That's hard to translate since the devices file lists explicit, existing devices, and lvmdevices is also trying to keep the user from doing something that's usually a mistake (like adding both a whole device and a partition since lvm won't use both sda and sda1.)

To deal with this, one approach is to have logic in your code to only use lvmdevices --adddev on devs that exist, and logic to avoid adding both whole dev and partitions.  Another approach is to call lvmdevices --adddev for every possible name and expect errors for devs that don't exist or when a whole dev and partitions exist.  I'll think a little more about this case to see if there's anything else that makes sense.

> It's kind of awkward to add --devices to any LVM command we use, although
> I guess it would be possible, but AIUI it doesn't solve the above problem.

IIUC, this is a special transient environment where modifying system.devices won't interfere with the running system itself?  In that case it seems fine to take over system.devices for your own purposes.  (You can also specify a custom devices file for your own purposes by adding --devicesfile mytempdevs to every lvm command which will then use /etc/lvm/devices/mytempdevs.devices.)

I've done some investigation here. The problem reproduces with
libguestfs-1.48.3-1.el9.x86_64 and lvm2-2.03.14-4.el9.x86_64 (using the
reproducer from comment#1).

My claim is that this is an lvm bug. According to
<https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c8>, the libguestfs
appliance already starts with a pristine LVM_SYSTEM_DIR, relative to
which no "devices/system.devices" file exists. The "use_devicesfile"
knob may default to 1, yes, but according to
<https://bugzilla.redhat.com/show_bug.cgi?id=2072493#c4>, the absence of
the actual "system.devices" file is supposed to override that.

And that does not occur in the lvm2 _accept_p() function
[lib/filters/filter-regex.c], which is what breaks the libguestfs
lvm-set-filter API:

> 	if (cmd->enable_devices_file && !cmd->filter_regex_with_devices_file) {
> 		/* can't warn in create_filter because enable_devices_file is set later */
> 		if (rf->config_filter && !rf->warned_filter) {
> 			log_warn("Please remove the lvm.conf filter, it is ignored with the devices file.");
> 			rf->warned_filter = 1;
> 		}
> 		if (rf->config_global_filter && !rf->warned_global_filter) {
> 			log_warn("Please remove the lvm.conf global_filter, it is ignored with the devices file.");
> 			rf->warned_global_filter = 1;
> 		}
> 		return 1;
> 	}

The warnings come from lvm2 commit 71933d3496df ("devices file:
recommend removing filter", 2021-05-05), but the actual logic comes from
83fe6e720f42 ("device usage based on devices file", 2021-02-23).

(

Note that "filter_regex_with_devices_file" is an internal override (not
exposed to external callers as far as I can tell); it is only set in
"tools/vgimportdevices.c". The vgimportdevices(8) manual says,

> When a devices file is used, the regex filter is ignored, except in
> the case of vgimportdevices which will apply the regex filter when
> looking for the VGs to import to the devices file. Use vgimportdevices
> -a to  import all VGs on a system to the devices file.

And the lvmdevices(8) manual says,

> With no devices file, lvm will use any device on the system, and
> applies the filter to limit the full set of system devices. With a
> devices file, the regex filter is not used, and the filter settings in
> lvm.conf or the command line are ignored. The vgimportdevices command
> is one exception which does apply the regex filter when looking for a
> VG to import.

)

So the lvm2 issue here (IMO) is that this code snippet only checks for
the knob, and not for the existence of
"$LVM_SYSTEM_DIR/devices/system.devices".

Anyway... I guess it's faster if we work it around. We can update the
appliance "init" file as follows:

> --- init.orig   2022-05-26 15:56:18.000000000 +0200
> +++ init        2022-05-27 11:49:49.957602265 +0200
> @@ -142,6 +142,17 @@
>  # 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


Note that the "use_devicesfile=0" setting from this *sticks around*,
even after the "lvm-set-filter" and "lvm-clear-filter" APIs completely
overwrite "lvm.conf" in the set_filter() function [daemon/lvm-filter.c].

Here's a test:

# guestfish -N disk \
    part-init /dev/sda mbr : \
    part-add /dev/sda p 2 100000 : \
    part-add /dev/sda p 100001 200000 : \
    pvcreate /dev/sda1 : \
    pvcreate /dev/sda2 : \
    pvs : \
    echo ==== : \
    lvm-set-filter /dev/sda1 : \
    pvs : \
    echo ==== : \
    lvm-clear-filter : \
    pvs : \
    echo ==== : \
    lvm-set-filter /dev/sda1 : \
    pvs : \
    echo ==== : \
    lvm-clear-filter : \
    pvs

Output:

> /dev/sda1
> /dev/sda2
> ====
> /dev/sda1
> ====
> /dev/sda1
> /dev/sda2
> ====
> /dev/sda1
> ====
> /dev/sda1
> /dev/sda2

Re-running the same with the "-v -x" options added, the log contains:

> + mkdir -p /tmp/lvm
> + touch /tmp/lvm/lvm.conf
> + command -v lvmdevices
> /sbin/lvmdevices
> + printf 'devices {\n'
> + printf '\tuse_devicesfile = 0\n'
> + printf '}\n'
> + LVM_SYSTEM_DIR=/tmp/lvm
> + export LVM_SYSTEM_DIR

from "init", then

> + lvm config
> devices {
> \tuse_devicesfile=0
> }
> + lvm pvs
> + lvm vgs
> + lvm lvs

also from init, then:

> lvm config:
> devices {
> \tfilter=["a|^/dev/sda1$|","r|.*|"]
> \tglobal_filter=["a|^/dev/sda1$|","r|.*|"]
> }

> lvm config:
> devices {
> \tfilter="a/.*/"
> \tglobal_filter="a/.*/"
> }

> lvm config:
> devices {
> \tfilter=["a|^/dev/sda1$|","r|.*|"]
> \tglobal_filter=["a|^/dev/sda1$|","r|.*|"]
> }

> lvm config:
> devices {
> \tfilter="a/.*/"
> \tglobal_filter="a/.*/"
> }

As the "lvm-set-filter" and "lvm-clear-filter" commands on the guestfish
command line repeatedly overwrite "lvm.conf" *entirely*. This shows that
the initial "use_devicesfile = 0" setting from the "init" script
*persists* -- meaning that we need not modify "daemon/lvm-filter.c" at
all.

(In reply to Laszlo Ersek from comment #8)
> The "use_devicesfile" knob may default to 1, yes, but ...
> the absence of the actual "system.devices" file is supposed to override that.

Right.  If you've found a case where that's not happening, could you send the lvm command with -vvvv?

> > 	if (cmd->enable_devices_file && !cmd->filter_regex_with_devices_file) {

cmd->enable_devices_file should be 0 when the devices file (system.devices) doesn't exist...

> > 		/* can't warn in create_filter because enable_devices_file is set later */
> > 		if (rf->config_filter && !rf->warned_filter) {
> > 			log_warn("Please remove the lvm.conf filter, it is ignored with the devices file.");
> > 			rf->warned_filter = 1;
> > 		}
> > 		if (rf->config_global_filter && !rf->warned_global_filter) {
> > 			log_warn("Please remove the lvm.conf global_filter, it is ignored with the devices file.");
> > 			rf->warned_global_filter = 1;
> > 		}
> > 		return 1;
> > 	}

so that block of code should not be running.

> So the lvm2 issue here (IMO) is that this code snippet only checks for
> the knob, and not for the existence of
> "$LVM_SYSTEM_DIR/devices/system.devices".

See setup_devices():

        file_exists = devices_file_exists(cmd);

        /*
         * Removing the devices file is another way of disabling the use of
         * a devices file, unless the command creates the devices file.
         */
        if (!file_exists && !cmd->create_edit_devices_file) {
                log_debug("Devices file not found, ignoring.");
                cmd->enable_devices_file = 0;
                goto scan;
        }

But there may be a command which is not using setup_devices() and is missing that file_exists check, so please send the specific command you're having problems with.

> Why does Fedora not have a package of release 2.03.12?

You're not the only one confused by this, I've been hearing that it's supposed to be updated soon.

Created attachment 1885102 [details]
lvm2 log

Please find the log attached (for lvm2-2.03.14-4.el9.x86_64).

It was produced by the following libguestfs debug patch (on top of
commit ad24b9f4d695 ("build: Pick first field in ID_LIKE", 2022-05-26)
on the rhel-9.1 branch), using the reproducer in comment#8:

> diff --git a/daemon/lvm-filter.c b/daemon/lvm-filter.c
> index c6dd35156d37..b6ba96d19f4b 100644
> --- a/daemon/lvm-filter.c
> +++ b/daemon/lvm-filter.c
> @@ -147,7 +147,8 @@ rescan (void)
>    unlink (lvm_cache);
>
>    CLEANUP_FREE char *err = NULL;
> -  int r = command (NULL, &err, "lvm", "vgscan", "--cache", NULL);
> +  int r = command (NULL, &err, "lvm", "vgscan", "-v", "-v", "-v", "-v",
> +                   "--cache", NULL);
>    if (r == -1) {
>      reply_with_error ("vgscan: %s", err);
>      return -1;

It looks like something *creates* the devices file previously, even though we never ask for it:

> 12:32:51.471105 vgscan[257] device/device_id.c:550  device_ids_read /tmp/lvm/devices/system.devices
> 12:32:51.473047 vgscan[257] device/device_id.c:573  read devices file version 1.1.2

This should not succeed.

Subject: [libguestfs PATCH] appliance, daemon: disable lvm2 devicesfile
Message-Id: <20220530141027.16167-1-lersek>
https://listman.redhat.com/archives/libguestfs/2022-May/029030.html

(In reply to Laszlo Ersek from comment #13)
> Subject: [libguestfs PATCH] appliance, daemon: disable lvm2 devicesfile
> Message-Id: <20220530141027.16167-1-lersek>
> https://listman.redhat.com/archives/libguestfs/2022-May/029030.html

Upstream commit 8fc4d167153a.

Tested with libguestfs-1.48.3-2.el9.x86_64:

Steps:

1. On rhel9.1 host
$ guestfish -N disk part-init /dev/sda mbr : part-add /dev/sda p 2 100000 : part-add /dev/sda p 100001 200000 : pvcreate /dev/sda1 : pvcreate /dev/sda2 : pvs : lvm-set-filter /dev/sda1 : pvs
/dev/sda1
/dev/sda2
/dev/sda1

The result is expected.

The test case of this bug in our automation passed in the recent compose test, so verified this bug.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (Low: libguestfs security, bug fix, and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2022:7958

I just found this issue made my system unbootable when patching my from RHEL 9.0 (5.14.0-70.30.1.el9_0) to RHEL 9.2 (5.14.0-284.30.1.el9_2)

resolution was to:

Boot a recovery USB. 

chroot /mnt/sysroot

edit lvm.conf 

set
  use_devicesfile = 0 


On reboot the system found the device.

(In reply to Paul Armstrong from comment #21)
> 
> I just found this issue made my system unbootable when patching my from RHEL
> 9.0 (5.14.0-70.30.1.el9_0) to RHEL 9.2 (5.14.0-284.30.1.el9_2)
> 

Aye. This seems to still be a problem with fully patched 9.3. Specifically, a custom vagrant box built with virtualbox+kickstart+packer breaks as described with lvs that should be ie mounted at /var and /tmp --

  zerombr
  clearpart --all --initlabel
  part /boot --ondisk=sda --fstype="xfs"  --size=1024
  part pv.00 --ondisk=sda --size=8192 --grow
  volgroup vg00 pv.00
  logvol swap --vgname=vg00 --fstype="swap" --size=2048 --name=swap
  logvol /tmp --vgname=vg00 --fstype="xfs" --size=2048 --name=lv_tmp --label=tmp
  logvol /    --vgname=vg00 --fstype="xfs" --size=1024 --name=lv_root --label=root --grow
  logvol /var --vgname=vg00 --fstype="xfs" --size=4096 --name=lv_var --label=var

The hack seems to be munging the lvm.conf file in the %post kickstart section:

  sed -i 's/# use_devicesfile = 1/use_devicesfile = 0/' /etc/lvm/lvm.conf

Are you referring to libguestfs (which is what this bug is about), or the
bug in LVM itself (bug 1957040, I think)?

(In reply to Richard W.M. Jones from comment #23)
> Are you referring to libguestfs (which is what this bug is about), or the
> bug in LVM itself (bug 1957040, I think)?

You might be correct that I have the wrong bug. 1957040 seems to talk about what should happen when lvm.conf is not present.

Your comment made me dig into the problem more. For the sake of completeness: it looks like the reason this was good in RHEL 8 but broke for us in RHEL 9 is because the default in lvm.conf use_devicesfile changed from 0 to 1. As best I can tell, our problem is that /etc/lvm/devices/system.devices is not being properly regenerated during the image creation and never was. It was a coincidence that RHEL 8 was fine, but when RHEL 9 activated use_devicesfile by default, it broke LVM.

I'm not entirely sure when/how system.devices is supposed to regenerate, but that's out of scope for this bug. Apologies for misreading and appreciate your comment.