discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Heming Zhao]

Hello List,

I remember we discussed about the mpath filter before. It looks lvm2 developers didn't trust udev and wrote hard-coded scanning actions (see commit 3b0f9cec7e999, and below function dev_is_mpath_component()) to replace mpath+udev. But in SUSE env, we had tested/ran a long time and worked fine with setting up lvm2 under obtain_device_list_from_udev=1 & external_device_info_source = "udev".

 From SUSE env, below function at least should put line 702~705 to the beginning of this function. In the other word, consulting udev first, then back off to hard-coded checks.
I don't know if the "udev+mpio+lvm2" combination in RedHat environments often encounters problems with abnormal startup. From SUSE env, it seems we do revert 3b0f9cec7e999 may got better result.

677 int dev_is_mpath_component(struct cmd_context *cmd, struct device *dev, dev_t *holder_devno)
678 {
679     struct dev_types *dt = cmd->dev_types;
680     int primary_result;
681     dev_t primary_dev;
682
683     /*
684      * multipath only uses SCSI or NVME devices
685      */
686     if (!major_is_scsi_device(dt, MAJOR(dev->dev)) && !dev_is_nvme(dt, dev))
687         return 0;
688
689     /*
690      * primary_result 2: dev is a partition, primary_dev is the whole device
691      * primary_result 1: dev is a whole device
692      */
693     if (!(primary_result = dev_get_primary_dev(dt, dev, &primary_dev)))
694         return_0;
695
696     if (_dev_is_mpath_component_sysfs(cmd, dev, primary_result, primary_dev, holder_devno)  == 1)
697         goto found;
698
699     if (_dev_in_wwid_file(cmd, dev, primary_result, primary_dev))
700         goto found;
701
702     if (external_device_info_source() == DEV_EXT_UDEV) {
703         if (_dev_is_mpath_component_udev(dev) == 1)
704             goto found;
705     }
706
707     /*
708      * TODO: save the result of this function in dev->flags and use those
709      * flags on repeated calls to avoid repeating the work multiple times
710      * for the same device when there are partitions on the device.
711      */
712
713     return 0;
714 found:
715     return 1;
716 }

Thanks,
Heming

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Sat, 2023-11-11 at 20:51 +0800, Heming Zhao wrote:

> I remember we discussed about the mpath filter before. It looks lvm2
> developers didn't trust udev and wrote hard-coded scanning actions
> (see commit 3b0f9cec7e999, and below function
> dev_is_mpath_component()) to replace mpath+udev. But in SUSE env, we
> had tested/ran a long time and worked fine with setting up lvm2 under
> obtain_device_list_from_udev=1 & external_device_info_source =
> "udev".
> 
>  From SUSE env, below function at least should put line 702~705 to
> the beginning of this function. In the other word, consulting udev
> first, then back off to hard-coded checks.
> I don't know if the "udev+mpio+lvm2" combination in RedHat
> environments often encounters problems with abnormal startup. From
> SUSE env, it seems we do revert 3b0f9cec7e999 may got better result.

Adding Ben as RH's multipath maintainer, and Hannes.

TL;DR: I believe that 3b0f9ce ("filter-mpath: get wwids from sysfs
vpd_pg83") is wrong. With "external_device_info_source = udev", LVM
must fully rely on udev properties.

Long story:

multipath-tools has complex logic for determining whether a given
device should be considered a multipath component. This logic depends
non-trivially on configuration settings in multipath.conf. Other tools
are ill-advised to try to re-implement multipath's logic.

We have a mechanism that works. multipath and multipathd work together
to set the udev property DM_MULTIPATH_DEVICE_PATH on potential
multipath component devices to indicate multipath's own decision about
the device.

I can't stress enough that this is *the only mechanism* that works
correctly. udev serves as central hub to retrieve device properties
from, and this is how it ought to be. In know that LVM maintainers have
a low opinion about udev. But all issues that I've been made aware of
in the last couple of years have been addressed. There have been
problems with all tools involved — multipath, lvm, udev and udev rules,
systemd's device activation logic, dracut — but I firmly believe that
they have been overcome, and that LVM can rely on
DM_MULTIPATH_DEVICE_PATH safely on every real-world system. The only
exceptions I am aware of are environments where udev isn't available,
such as image build environments. If you know about any counter-
examples, please let me know. As multipath maintainers, we are
determined to fix them [1].

From the code Heming showed, _dev_is_mpath_component_sysfs() is ok-ish,
but redundant; DM_MULTIPATH_DEVICE_PATH implements the same logic.
_dev_in_wwid_file() is wrong though. There are various possible cases
in which a a device should not be part of a multipath map even though
its WWID is listed in the WWIDs file. multipath might be disabled via
systemd or kernel command line, the device might be blacklisted, or
marked as "failed_wwid" [2]. This list is incomplete.
DM_MULTIPATH_DEVICE_PATH takes all these possibilities into account,
LVM's new logic does not. 

I am not suggesting that LVM improve it's implementation of multipath
component detection.

Rather, LVM must rely on DM_MULTIPATH_DEVICE_PATH if
"external_device_info_source = udev". Current LVM release are lying
about external_device_info_source when it's set to udev, as they do
_not_ respect what udev tells them. If you really need a mode in which
udev properties are only partially respected, don't call it
"external_device_info_source = udev".

Regards
Martin


PS: Here's a related remark about 17a3585 ("pvscan: use alternate
device names from DEVLINKS to check filter"). I can see why this was
necessary, but I don't understand why this is found to be necessary
_now_; the same issue should have always existed if "pvscan" is running
during a "change" event for any given device. The solution of 17a3585
"worked" for us, but it looks only semi-ok to me. Other udev rules may
modify the DEVLINKS list after pvscan had been running. A correct
solution must make sure that pvscan runs after all udev rules. IOW,
pvscan should be triggered in a udev RUN= statement rather then
IMPORT=. This would probably require a new systemd service, because
it's not just "pvscan" alone. But the result would be more robust then
what we currently have.

[1] I assume that commit 3b0f9ce has been created to work around some
problem. I'd appreciate if multipath maintainers were involved in
issues like this. If I'd been involved, I would have told you that I
believe the approach of 3b0f9ce is wrong, and I'm pretty sure we would
have found a solution that respects the udev properties.

[2] meaning that previous attempts to set up a multipath map on the
device have failed.

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Peter Rajnoha]

On 11/13/23 12:52, Martin Wilck wrote:
> I can't stress enough that this is *the only mechanism* that works
> correctly. udev serves as central hub to retrieve device properties
> from, and this is how it ought to be.

Indeed, that was exactly one of the primary reasons the
'external_device_info_source="udev"' was added. Reimplementing other
subsystem's logic is wrong because there's always some intrinsic logic
there that may be omitted. Sooner or later, such approach will fail.

I agree with you here.

If not sharing information through udev due to various kinds of hatred
about this part of the system, then alternatively, at least, calling a
multipath library function from LVM to do the job.

-- 
Peter

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Mon, Nov 13, 2023 at 02:52:39PM +0100, Peter Rajnoha wrote:
> On 11/13/23 12:52, Martin Wilck wrote:
> > I can't stress enough that this is *the only mechanism* that works
> > correctly. udev serves as central hub to retrieve device properties
> > from, and this is how it ought to be.
> 
> Indeed, that was exactly one of the primary reasons the
> 'external_device_info_source="udev"' was added. Reimplementing other
> subsystem's logic is wrong because there's always some intrinsic logic
> there that may be omitted. Sooner or later, such approach will fail.
> 
> I agree with you here.
> 
> If not sharing information through udev due to various kinds of hatred
> about this part of the system, then alternatively, at least, calling a
> multipath library function from LVM to do the job.

Hi, a number of things haven't been mentioned or noticed:

- In general, it's old versions of lvm that we're discussing here.
Current lvm uses system.devices by default, where none of this is
relevant.  We can just turn off a number of filters when system.devices is
in use, including filter-mpath and filter-md.  It may still be interesting
to look at improvements, but the context for that is older stable,
released versions.

- In RHEL8 (no system.devices by default, and some older systemd/udev),
lvm can't touch udev without causing timeouts booting with many devices.
Nothing I tried apart from reading wwids would help.  Boot timeouts were a
big support burden, and the motivation for the changes.  (All of this
added motivation for system.devices.)

- I tried many things before reading wwids, including asking for a
multipath lib API for reading the wwids.  The provided API that I tried
also used udev, so it was no better.  I'd still prefer to use an API to
check a wwid and avoid reading /etc/multipath/wwids directly.  Reading
wwids looked like the least worst practical solution.  I consider it a
temporary hack/workaround until system.devices eliminates the issue.

- When making the changes to avoid udev by default, I made a subtle change
to the meaning of external_device_info_source=udev.  I made that info
supplementary to the native info rather than an alternative to the native
info.  The idea was: why not use all the info we have available?  However,
for filter-mpath, the supplementary approach will be a problem if reading
wwids gives a false positive.

- To make filter-mpath use udev info, and prevent lvm from using wwids,
you can set external_device_info_source=udev and multipath_wwids_file="".
I think this will be an effective way to avoid any false positive
problems from native detection.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Peter Rajnoha]

On 11/13/23 19:38, David Teigland wrote:
> On Mon, Nov 13, 2023 at 02:52:39PM +0100, Peter Rajnoha wrote:
>> On 11/13/23 12:52, Martin Wilck wrote:
>>> I can't stress enough that this is *the only mechanism* that works
>>> correctly. udev serves as central hub to retrieve device properties
>>> from, and this is how it ought to be.
>>
>> Indeed, that was exactly one of the primary reasons the
>> 'external_device_info_source="udev"' was added. Reimplementing other
>> subsystem's logic is wrong because there's always some intrinsic logic
>> there that may be omitted. Sooner or later, such approach will fail.
>>
>> I agree with you here.
>>
>> If not sharing information through udev due to various kinds of hatred
>> about this part of the system, then alternatively, at least, calling a
>> multipath library function from LVM to do the job.
> 
> Hi, a number of things haven't been mentioned or noticed:
> 
> - In general, it's old versions of lvm that we're discussing here.
> Current lvm uses system.devices by default, where none of this is
> relevant.  We can just turn off a number of filters when system.devices is
> in use, including filter-mpath and filter-md.  It may still be interesting
> to look at improvements, but the context for that is older stable,
> released versions.

Hmm, but to generate or update the system.devices file and to add a new
entry there, we still need to check first whether it's a suitable device
or not, right? Like not being a multipath component...

-- 
Peter

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 14, 2023 at 08:55:39AM +0100, Peter Rajnoha wrote:
> On 11/13/23 19:38, David Teigland wrote:
> > - In general, it's old versions of lvm that we're discussing here.
> > Current lvm uses system.devices by default, where none of this is
> > relevant.  We can just turn off a number of filters when system.devices is
> > in use, including filter-mpath and filter-md.  It may still be interesting
> > to look at improvements, but the context for that is older stable,
> > released versions.
> 
> Hmm, but to generate or update the system.devices file and to add a new
> entry there, we still need to check first whether it's a suitable device
> or not, right? Like not being a multipath component...

If they know what they're doing, a user won't add a multipath/md component
to system.devices (or other unusable devices.)  However, it makes sense to
apply these checks when a device is added to system.devices to catch user
mistakes.  Also for vgimportdevices -a where a specific device is not
named.  The checks become a one-time thing, done when adding the device.
The checks are not needed by ordinary lvm commands, and are not being run
at tricky times like boot or for event handling.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Peter Rajnoha]

On 11/14/23 17:30, David Teigland wrote:
> On Tue, Nov 14, 2023 at 08:55:39AM +0100, Peter Rajnoha wrote:
>> On 11/13/23 19:38, David Teigland wrote:
>>> - In general, it's old versions of lvm that we're discussing here.
>>> Current lvm uses system.devices by default, where none of this is
>>> relevant.  We can just turn off a number of filters when system.devices is
>>> in use, including filter-mpath and filter-md.  It may still be interesting
>>> to look at improvements, but the context for that is older stable,
>>> released versions.
>>
>> Hmm, but to generate or update the system.devices file and to add a new
>> entry there, we still need to check first whether it's a suitable device
>> or not, right? Like not being a multipath component...
> 
> If they know what they're doing, a user won't add a multipath/md component
> to system.devices (or other unusable devices.)  However, it makes sense to
> apply these checks when a device is added to system.devices to catch user
> mistakes.  Also for vgimportdevices -a where a specific device is not
> named.  The checks become a one-time thing, done when adding the device.
> The checks are not needed by ordinary lvm commands, and are not being run
> at tricky times like boot or for event handling.

Sure, if we already have that in the devices file, we don't need to
rerun the filters again and again. But I think that for the very first
check we do when inserting new entry to the devices file, we should
really be using information that is *shared* by the other subsystem that
owns it. In case of multipath, we shouldn't be reading any of their
config or trying to reimplement subset of their logic (the config can
change format, the logic can change, new features can be added, various
settings can be applied which we omit in our own internal simplified
version of the logic).

If I would need to compare the situation to something, imagine that some
subsystem, MD for example, would be parsing our system.devices file to
check if it can or can not access certain device. Then checking
multipath config, then other config and so on... So that way we would
have each subsystem checking other subsystems by reimplementing these
checks again and again (with the risk of not even being correct).

We should be using dedicated APIs or udev to share the information among
various subsystems. Or, if nothing else, even calling out to the
subsystem's binary if it doesn't provide the API yet.

I don't see an issue with the devices file as such, I see the issue in
the way we gather information, reimplementing the logic that is simply
not in our domain.

I think that's also what Martin and Heming have in mind.

-- 
Peter

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Heming Zhao]

On 11/15/23 16:51, Peter Rajnoha wrote:
> On 11/14/23 17:30, David Teigland wrote:
>> On Tue, Nov 14, 2023 at 08:55:39AM +0100, Peter Rajnoha wrote:
>>> On 11/13/23 19:38, David Teigland wrote:
>>>> - In general, it's old versions of lvm that we're discussing here.
>>>> Current lvm uses system.devices by default, where none of this is
>>>> relevant.  We can just turn off a number of filters when system.devices is
>>>> in use, including filter-mpath and filter-md.  It may still be interesting
>>>> to look at improvements, but the context for that is older stable,
>>>> released versions.

 From the lvm2 code, above description is wrong. By default, system/devices is
disabled status. Because lvm.conf option "use_devicesfile = 0" by default.
under "use_devicesfile = 0", any pvcreate/vgcreate/lvmdevices/vgimportdevices
won't generate/create system/devices file.
If my watching is correct, part discussion of this thread may invalid.

@David could you confirm my writing?

Thanks,
Heming

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Wed, Nov 15, 2023 at 07:36:57PM +0800, Heming Zhao wrote:
> From the lvm2 code, above description is wrong. By default, system/devices is
> disabled status. Because lvm.conf option "use_devicesfile = 0" by default.
> under "use_devicesfile = 0", any pvcreate/vgcreate/lvmdevices/vgimportdevices
> won't generate/create system/devices file.
> If my watching is correct, part discussion of this thread may invalid.

The lvm default is use_devicesfile=1, as shown in this commit:

commit 6c22392a3f903d6c086f7cc94978bdf8b072da6e
Author: David Teigland <teigland@redhat.com>
Date:   Tue Mar 16 09:52:13 2021 -0500

    config: change default use_devicesfile to 1

diff --git a/lib/config/defaults.h b/lib/config/defaults.h
index 66eece53aa33..a7a2a06df654 100644
--- a/lib/config/defaults.h
+++ b/lib/config/defaults.h
@@ -322,7 +322,7 @@
 
 #define DEFAULT_MD_COMPONENT_CHECKS "auto"
 
-#define DEFAULT_USE_DEVICES_FILE 0
+#define DEFAULT_USE_DEVICES_FILE 1
 #define DEFAULT_DEVICES_FILE "system.devices"


Later commits that removed DEFAULT_USE_DEVICES_FILE, and tried to replace
it with a configure option, was a failed attempt at building that lvm
source for RHEL8.  It's caused no end of problems and I plan to revert it
and restore define DEFAULT_USE_DEVICES_FILE 1.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Zdenek Kabelac]

Dne 15. 11. 23 v 20:12 David Teigland napsal(a):
> On Wed, Nov 15, 2023 at 07:36:57PM +0800, Heming Zhao wrote:
>>  From the lvm2 code, above description is wrong. By default, system/devices is
>> disabled status. Because lvm.conf option "use_devicesfile = 0" by default.
>> under "use_devicesfile = 0", any pvcreate/vgcreate/lvmdevices/vgimportdevices
>> won't generate/create system/devices file.
>> If my watching is correct, part discussion of this thread may invalid.
> The lvm default is use_devicesfile=1, as shown in this commit:
>
> commit 6c22392a3f903d6c086f7cc94978bdf8b072da6e
> Author: David Teigland <teigland@redhat.com>
> Date:   Tue Mar 16 09:52:13 2021 -0500
>
>      config: change default use_devicesfile to 1
>
> diff --git a/lib/config/defaults.h b/lib/config/defaults.h
> index 66eece53aa33..a7a2a06df654 100644
> --- a/lib/config/defaults.h
> +++ b/lib/config/defaults.h
> @@ -322,7 +322,7 @@
>   
>   #define DEFAULT_MD_COMPONENT_CHECKS "auto"
>   
> -#define DEFAULT_USE_DEVICES_FILE 0
> +#define DEFAULT_USE_DEVICES_FILE 1
>   #define DEFAULT_DEVICES_FILE "system.devices"
>
>
> Later commits that removed DEFAULT_USE_DEVICES_FILE, and tried to replace
> it with a configure option, was a failed attempt at building that lvm
> source for RHEL8.  It's caused no end of problems and I plan to revert it
> and restore define DEFAULT_USE_DEVICES_FILE 1.


There is no problem with configuring DEFAULT_USE_DEVICES_FILE with 'configure 
--with-default-use-devices-file= 0/1' and thus no need to change anything here.

Current upstream has set this default value as 0  (in configure.ac)

RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).

The major problem with turning this  to 1 is the distribution must be 'ready' 
with such relatively invasive change as it changes also requirements on how 
the boot image is created  (devicesfile must be copied to ramdisk).

So Suse  may try to set this setting  to 1  and see what all breaks.


Zdenek

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Thu, 2023-11-16 at 14:37 +0100, Zdenek Kabelac wrote:
> 
> Current upstream has set this default value as 0  (in configure.ac)
> 
> RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).
> 
> The major problem with turning this  to 1 is the distribution must be
> 'ready' 
> with such relatively invasive change as it changes also requirements
> on how 
> the boot image is created  (devicesfile must be copied to ramdisk).
> 
> So Suse  may try to set this setting  to 1  and see what all breaks.

... and the man page may need fixing?

Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Zdenek Kabelac]

Dne 16. 11. 23 v 14:46 Martin Wilck napsal(a):
> On Thu, 2023-11-16 at 14:37 +0100, Zdenek Kabelac wrote:
>> Current upstream has set this default value as 0  (in configure.ac)
>>
>> RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).
>>
>> The major problem with turning this  to 1 is the distribution must be
>> 'ready'
>> with such relatively invasive change as it changes also requirements
>> on how
>> the boot image is created  (devicesfile must be copied to ramdisk).
>>
>> So Suse  may try to set this setting  to 1  and see what all breaks.
> ... and the man page may need fixing?


Yep - quite likely it would deserve some chapter in 'lvm' man page.

Worth to be noted - existing filters are not going anywhere out -  devicesfile 
is jiust another mechanism how to limit device set.

For some cases  devicesfile present a more simple way - for some others - 
filters are clearly a better mechanism which doesn't require updating files in 
the filesystem.

Both filtering systems should be equivalent and work equally well - there is 
no plan to phase out existing filter logic.


Zdenek

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Thu, 2023-11-16 at 15:03 +0100, Zdenek Kabelac wrote:
> 
> Worth to be noted - existing filters are not going anywhere out - 
> devicesfile 
> is jiust another mechanism how to limit device set.
> 
> For some cases  devicesfile present a more simple way - for some
> others - 
> filters are clearly a better mechanism which doesn't require updating
> files in 
> the filesystem.
> 
> Both filtering systems should be equivalent and work equally well -
> there is 
> no plan to phase out existing filter logic.

Good to know - that sounded a bit different one of David's previous
posts in this  thread.

Cheers,
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Thu, Nov 16, 2023 at 03:29:42PM +0000, Martin Wilck wrote:
> On Thu, 2023-11-16 at 15:03 +0100, Zdenek Kabelac wrote:
> > 
> > Worth to be noted - existing filters are not going anywhere out - 
> > devicesfile is jiust another mechanism how to limit device set.
> > 
> > For some cases  devicesfile present a more simple way - for some
> > others - filters are clearly a better mechanism which doesn't require
> > updating files in the filesystem.
> > 
> > Both filtering systems should be equivalent and work equally well -
> > there is no plan to phase out existing filter logic.

That's just vague enough that I can both agree with it and disagree with
it at the same time :)

The devices file offers significant advantages over regex filters, which
could be described as "working better."  On the other hand, we're
obviously not making the regex filter work worse, and I'm sure it will
exist forever given how long it's been part of lvm.

> Good to know - that sounded a bit different one of David's previous
> posts in this  thread.

I like to be accurate, so to be clear, while the devices file is being
used, the regex filter cannot be used.  lvm commands will print:
"Please remove the lvm.conf filter, it is ignored with the devices file."

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Thu, Nov 16, 2023 at 02:37:13PM +0100, Zdenek Kabelac wrote:
> > and restore define DEFAULT_USE_DEVICES_FILE 1.
> 
> There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
> 'configure --with-default-use-devices-file= 0/1' and thus no need to change
> anything here.
> 
> Current upstream has set this default value as 0  (in configure.ac)

I didn't realize you'd changed the default, please set it back to 1.
(Even better put back DEFAULT_USE_DEVICES_FILE; configured settings
are a pain to deal with, and the complexity causes real problems.)

> The major problem with turning this  to 1 is the distribution must be
> 'ready' with such relatively invasive change as it changes also requirements
> on how the boot image is created  (devicesfile must be copied to ramdisk).

While a distribution does want to be aware of the change (discussed
earlier in the thread), what you're saying specifically is not true.
RHEL, which has this enabled, doesn't even have system.devices in the
ramdisk.  It simply means that it's not used for activating the root LV.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Thu, 2023-11-16 at 09:10 -0600, David Teigland wrote:
> On Thu, Nov 16, 2023 at 02:37:13PM +0100, Zdenek Kabelac wrote:
> > > and restore define DEFAULT_USE_DEVICES_FILE 1.
> > 
> > There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
> > 'configure --with-default-use-devices-file= 0/1' and thus no need
> > to change
> > anything here.
> > 
> > Current upstream has set this default value as 0  (in configure.ac)
> 
> I didn't realize you'd changed the default, please set it back to 1.
> (Even better put back DEFAULT_USE_DEVICES_FILE; configured settings
> are a pain to deal with, and the complexity causes real problems.)

The problem I see with using system.devices by default is that it
requires installer support, as discussed previously. pvcreate will
create the file if it doesn't find existing VGs or PVs, which means
that the way LVM behaves depends on the system's history:

- If it was updated from an earlier LVM version, VGs are likely to
exist, and system.devices will not be created.
- If VGs were created by the installer without copying system.devices
into the installed system, system.device won't be created, either.
- If the users installs without LVM and uses pvcreate later on, LVM
will create system.devices, and switch to system.devices mode
- Not sure what happens if the system is running in "legacy" filtering
/ multipath component detection mode and something goes wrong during
boot, causing the PVs and VGs not to show up. If pvcreate is run in a
situation like that, would it also create system.devices and switch
modes?

Distributions can of of course flip the default, but unless I'm
mistaken RHEL9 (and possibly Fedora) are the only distros that fully
support system.devices mode just yet. Thus I vote for leaving it off by
default just now.

Regards
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Zdenek Kabelac]

Dne 16. 11. 23 v 16:40 Martin Wilck napsal(a):
> On Thu, 2023-11-16 at 09:10 -0600, David Teigland wrote:
>> On Thu, Nov 16, 2023 at 02:37:13PM +0100, Zdenek Kabelac wrote:
>>>> and restore define DEFAULT_USE_DEVICES_FILE 1.
>>> There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
>>> 'configure --with-default-use-devices-file= 0/1' and thus no need
>>> to change
>>> anything here.
>>>
>>> Current upstream has set this default value as 0  (in configure.ac)
>> I didn't realize you'd changed the default, please set it back to 1.
>> (Even better put back DEFAULT_USE_DEVICES_FILE; configured settings
>> are a pain to deal with, and the complexity causes real problems.)
> The problem I see with using system.devices by default is that it
> requires installer support, as discussed previously. pvcreate will
> create the file if it doesn't find existing VGs or PVs, which means
> that the way LVM behaves depends on the system's history:
>
> - If it was updated from an earlier LVM version, VGs are likely to
> exist, and system.devices will not be created.
> - If VGs were created by the installer without copying system.devices
> into the installed system, system.device won't be created, either.
> - If the users installs without LVM and uses pvcreate later on, LVM
> will create system.devices, and switch to system.devices mode
> - Not sure what happens if the system is running in "legacy" filtering
> / multipath component detection mode and something goes wrong during
> boot, causing the PVs and VGs not to show up. If pvcreate is run in a
> situation like that, would it also create system.devices and switch
> modes?
>
> Distributions can of of course flip the default, but unless I'm
> mistaken RHEL9 (and possibly Fedora) are the only distros that fully
> support system.devices mode just yet. Thus I vote for leaving it off by
> default just now.


Yep - it needs to be fully understood and supported by distro - so changing 
default may cause significant troubles - and took us quite some time to figure 
out various details in RHEL - and I think we still have few opened issues to 
solve. So even if we would flip default - configure process should be giving 
some major message notification about such change and how to easily 'switch' 
to previous 'fillter' logic.   So 'dstro' maintainer then just adds  configure 
option and keeps it in older state until distro gets ready for such changes.

Note - it's  somewhat easier to have the change present in the installation 
compared with  'upgrading' path with already existing  lvm.conf...

Zdenek

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Thu, Nov 16, 2023 at 04:48:06PM +0100, Zdenek Kabelac wrote:
> Dne 16. 11. 23 v 16:40 Martin Wilck napsal(a):
> > The problem I see with using system.devices by default is that it
> > requires installer support, as discussed previously.

Yes

> Yep - it needs to be fully understood and supported by distro - so changing
> default may cause significant troubles - and took us quite some time to
> figure out various details in RHEL 

Yes, as mentioned earlier, "images" are a unique case that's not entirely
solved yet.

> issues to solve. So even if we would flip default - configure process should
> be giving some major message notification about such change and how to
> easily 'switch' to previous 'fillter' logic.   So 'dstro' maintainer then
> just adds  configure option and keeps it in older state until distro gets
> ready for such changes.

I pity the distro maintainer who has to figure out all the lvm configure
options... I can hardly figure them out myself.  If lvm wanted to be
helpful, we'd provide pre-defined sets of recommended configure options
for various distros.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Zdenek Kabelac]

Dne 16. 11. 23 v 16:10 David Teigland napsal(a):
> On Thu, Nov 16, 2023 at 02:37:13PM +0100, Zdenek Kabelac wrote:
>>> and restore define DEFAULT_USE_DEVICES_FILE 1.
>> There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
>> 'configure --with-default-use-devices-file= 0/1' and thus no need to change
>> anything here.
>>
>> Current upstream has set this default value as 0  (in configure.ac)
> I didn't realize you'd changed the default, please set it back to 1.
> (Even better put back DEFAULT_USE_DEVICES_FILE; configured settings
> are a pain to deal with, and the complexity causes real problems.)

configure is the right way how to set 'default' parameter for compilation.

And since different distribution needs different defaults - it's way better 
then letting every distro maintain some patch sets separately.

Even i.e. building of RHEL8 and RHEL9 have different defaults.

Once the option become supported properly across wide distribution set  - it's 
very easy to flip default in 'configure.ac'  from 0 to 1.

There is no need to change anything else here.

RHEL & Fedora builds are prepared and use proper configure set and build 
proper packages

>> The major problem with turning this  to 1 is the distribution must be
>> 'ready' with such relatively invasive change as it changes also requirements
>> on how the boot image is created  (devicesfile must be copied to ramdisk).
> While a distribution does want to be aware of the change (discussed
> earlier in the thread), what you're saying specifically is not true.
> RHEL, which has this enabled, doesn't even have system.devices in the
> ramdisk.  It simply means that it's not used for activating the root LV.
>

Since lvm2  without 'devicesfiles' will fallback to filters, which by default 
accept 'every' device, this in most cases  will lead to the situation,  that 
if there are no 'specific' needs to filter something out  usually the right LV 
will be activated.

So in most cases even if the 'devicesfile' is not being copied it will appear 
like it's working.

But there will be situation where the systems where lvm2  will be accessing 
devices it should not access - and this is a problem that should be addressed 
(i.e. duplicate devices present in the system).


Zdenek

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 2556 bytes --]

On Thu, Nov 16, 2023 at 02:37:13PM +0100, Zdenek Kabelac wrote:
> Dne 15. 11. 23 v 20:12 David Teigland napsal(a):
> > On Wed, Nov 15, 2023 at 07:36:57PM +0800, Heming Zhao wrote:
> > >  From the lvm2 code, above description is wrong. By default, system/devices is
> > > disabled status. Because lvm.conf option "use_devicesfile = 0" by default.
> > > under "use_devicesfile = 0", any pvcreate/vgcreate/lvmdevices/vgimportdevices
> > > won't generate/create system/devices file.
> > > If my watching is correct, part discussion of this thread may invalid.
> > The lvm default is use_devicesfile=1, as shown in this commit:
> > 
> > commit 6c22392a3f903d6c086f7cc94978bdf8b072da6e
> > Author: David Teigland <teigland@redhat.com>
> > Date:   Tue Mar 16 09:52:13 2021 -0500
> > 
> >      config: change default use_devicesfile to 1
> > 
> > diff --git a/lib/config/defaults.h b/lib/config/defaults.h
> > index 66eece53aa33..a7a2a06df654 100644
> > --- a/lib/config/defaults.h
> > +++ b/lib/config/defaults.h
> > @@ -322,7 +322,7 @@
> >   #define DEFAULT_MD_COMPONENT_CHECKS "auto"
> > -#define DEFAULT_USE_DEVICES_FILE 0
> > +#define DEFAULT_USE_DEVICES_FILE 1
> >   #define DEFAULT_DEVICES_FILE "system.devices"
> > 
> > 
> > Later commits that removed DEFAULT_USE_DEVICES_FILE, and tried to replace
> > it with a configure option, was a failed attempt at building that lvm
> > source for RHEL8.  It's caused no end of problems and I plan to revert it
> > and restore define DEFAULT_USE_DEVICES_FILE 1.
> 
> 
> There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
> 'configure --with-default-use-devices-file= 0/1' and thus no need to change
> anything here.
> 
> Current upstream has set this default value as 0  (in configure.ac)
> 
> RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).
> 
> The major problem with turning this  to 1 is the distribution must be
> 'ready' with such relatively invasive change as it changes also requirements
> on how the boot image is created  (devicesfile must be copied to ramdisk).
> 
> So Suse  may try to set this setting  to 1  and see what all breaks.
> 
> 
> Zdenek

Does this mean that distributions *should* set this to 1 and update
their installers?

This would have saved Qubes OS a security bulletin (LVM was scanning
VM-controlled block devices) and might make LVM faster (since it would
no longer need to scan every device on the system).
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Thu, Nov 16, 2023 at 08:47:22PM -0500, Demi Marie Obenour wrote:
> > > commit 6c22392a3f903d6c086f7cc94978bdf8b072da6e
> > > Author: David Teigland <teigland@redhat.com>
> > > Date:   Tue Mar 16 09:52:13 2021 -0500
> > > 
> > >      config: change default use_devicesfile to 1
> > > 
> > > diff --git a/lib/config/defaults.h b/lib/config/defaults.h
> > > index 66eece53aa33..a7a2a06df654 100644
> > > --- a/lib/config/defaults.h
> > > +++ b/lib/config/defaults.h
> > > @@ -322,7 +322,7 @@
> > >   #define DEFAULT_MD_COMPONENT_CHECKS "auto"
> > > -#define DEFAULT_USE_DEVICES_FILE 0
> > > +#define DEFAULT_USE_DEVICES_FILE 1
> > >   #define DEFAULT_DEVICES_FILE "system.devices"
> > > 
> > > 
> > > Later commits that removed DEFAULT_USE_DEVICES_FILE, and tried to replace
> > > it with a configure option, was a failed attempt at building that lvm
> > > source for RHEL8.  It's caused no end of problems and I plan to revert it
> > > and restore define DEFAULT_USE_DEVICES_FILE 1.
> > 
> > 
> > There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
> > 'configure --with-default-use-devices-file= 0/1' and thus no need to change
> > anything here.
> > 
> > Current upstream has set this default value as 0  (in configure.ac)
> > 
> > RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).
> > 
> > The major problem with turning this  to 1 is the distribution must be
> > 'ready' with such relatively invasive change as it changes also requirements
> > on how the boot image is created  (devicesfile must be copied to ramdisk).
> > 
> > So Suse  may try to set this setting  to 1  and see what all breaks.
> > 
> > 
> > Zdenek
> 
> Does this mean that distributions *should* set this to 1 and update
> their installers?

Yes, the devices file is intended to be a default feature of lvm since the
commit above.

> This would have saved Qubes OS a security bulletin (LVM was scanning
> VM-controlled block devices) and might make LVM faster (since it would
> no longer need to scan every device on the system).

These are both advantages of using it.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 2247 bytes --]

On Fri, Nov 17, 2023 at 09:25:50AM -0600, David Teigland wrote:
> On Thu, Nov 16, 2023 at 08:47:22PM -0500, Demi Marie Obenour wrote:
> > > > commit 6c22392a3f903d6c086f7cc94978bdf8b072da6e
> > > > Author: David Teigland <teigland@redhat.com>
> > > > Date:   Tue Mar 16 09:52:13 2021 -0500
> > > > 
> > > >      config: change default use_devicesfile to 1
> > > > 
> > > > diff --git a/lib/config/defaults.h b/lib/config/defaults.h
> > > > index 66eece53aa33..a7a2a06df654 100644
> > > > --- a/lib/config/defaults.h
> > > > +++ b/lib/config/defaults.h
> > > > @@ -322,7 +322,7 @@
> > > >   #define DEFAULT_MD_COMPONENT_CHECKS "auto"
> > > > -#define DEFAULT_USE_DEVICES_FILE 0
> > > > +#define DEFAULT_USE_DEVICES_FILE 1
> > > >   #define DEFAULT_DEVICES_FILE "system.devices"
> > > > 
> > > > 
> > > > Later commits that removed DEFAULT_USE_DEVICES_FILE, and tried to replace
> > > > it with a configure option, was a failed attempt at building that lvm
> > > > source for RHEL8.  It's caused no end of problems and I plan to revert it
> > > > and restore define DEFAULT_USE_DEVICES_FILE 1.
> > > 
> > > 
> > > There is no problem with configuring DEFAULT_USE_DEVICES_FILE with
> > > 'configure --with-default-use-devices-file= 0/1' and thus no need to change
> > > anything here.
> > > 
> > > Current upstream has set this default value as 0  (in configure.ac)
> > > 
> > > RHEL builds use this setting as 1.  (Possibly even Fedora nowadays).
> > > 
> > > The major problem with turning this  to 1 is the distribution must be
> > > 'ready' with such relatively invasive change as it changes also requirements
> > > on how the boot image is created  (devicesfile must be copied to ramdisk).
> > > 
> > > So Suse  may try to set this setting  to 1  and see what all breaks.
> > > 
> > > 
> > > Zdenek
> > 
> > Does this mean that distributions *should* set this to 1 and update
> > their installers?
> 
> Yes, the devices file is intended to be a default feature of lvm since the
> commit above.

Could LVM issue a deprecation warning if DEFAULT_USE_DEVICES_FILE is set
to 0 or system.devices is not set at runtime?
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Fri, Nov 17, 2023 at 02:57:35PM -0500, Demi Marie Obenour wrote:
> > Yes, the devices file is intended to be a default feature of lvm since the
> > commit above.
> 
> Could LVM issue a deprecation warning if DEFAULT_USE_DEVICES_FILE is set
> to 0 or system.devices is not set at runtime?

No, deprecated implies we're planning to remove it, which we probably
never will.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Demi Marie Obenour]

[-- Attachment #1: Type: text/plain, Size: 605 bytes --]

On Fri, Nov 17, 2023 at 02:16:48PM -0600, David Teigland wrote:
> On Fri, Nov 17, 2023 at 02:57:35PM -0500, Demi Marie Obenour wrote:
> > > Yes, the devices file is intended to be a default feature of lvm since the
> > > commit above.
> > 
> > Could LVM issue a deprecation warning if DEFAULT_USE_DEVICES_FILE is set
> > to 0 or system.devices is not set at runtime?
> 
> No, deprecated implies we're planning to remove it, which we probably
> never will.

Still, distros need to know that they should make the change.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Fri, 2023-11-17 at 16:03 -0500, Demi Marie Obenour wrote:
> On Fri, Nov 17, 2023 at 02:16:48PM -0600, David Teigland wrote:
> > On Fri, Nov 17, 2023 at 02:57:35PM -0500, Demi Marie Obenour wrote:
> > > > Yes, the devices file is intended to be a default feature of
> > > > lvm since the
> > > > commit above.
> > > 
> > > Could LVM issue a deprecation warning if DEFAULT_USE_DEVICES_FILE
> > > is set
> > > to 0 or system.devices is not set at runtime?
> > 
> > No, deprecated implies we're planning to remove it, which we
> > probably
> > never will.
> 
> Still, distros need to know that they should make the change.

I think David and Zdenek did not say distros _should_ make the change.
They can, but it requires substantial adaptations and testing.

Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Zdenek Kabelac]

Dne 17. 11. 23 v 22:05 Martin Wilck napsal(a):
> On Fri, 2023-11-17 at 16:03 -0500, Demi Marie Obenour wrote:
>> On Fri, Nov 17, 2023 at 02:16:48PM -0600, David Teigland wrote:
>>> On Fri, Nov 17, 2023 at 02:57:35PM -0500, Demi Marie Obenour wrote:
>>>>> Yes, the devices file is intended to be a default feature of
>>>>> lvm since the
>>>>> commit above.
>>>> Could LVM issue a deprecation warning if DEFAULT_USE_DEVICES_FILE
>>>> is set
>>>> to 0 or system.devices is not set at runtime?
>>> No, deprecated implies we're planning to remove it, which we
>>> probably
>>> never will.
>> Still, distros need to know that they should make the change.
> I think David and Zdenek did not say distros _should_ make the change.
> They can, but it requires substantial adaptations and testing.


There is quite 'noticeable' difference in behavior of lvm2 with this change.

So having users without support behind their backs facing the sudden change 
where lvm2 does no longer sees all devices as it used to is somewhat 
problematic - we are adding changes and messages to the tool to improve this 
as we meet the real life situation. Once there will be enough coverage - the 
change will likely happen.

But as mentioned earlier, there are situation where the original filtering is 
simply easier to deal with. So both variants will stay in place.


Zdenek

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Wed, Nov 15, 2023 at 09:51:19AM +0100, Peter Rajnoha wrote:
> Sure, if we already have that in the devices file, we don't need to
> rerun the filters again and again. But I think that for the very first
> check we do when inserting new entry to the devices file, we should
> really be using information that is *shared* by the other subsystem that
> owns it. In case of multipath, we shouldn't be reading any of their
> config or trying to reimplement subset of their logic (the config can
> change format, the logic can change, new features can be added, various
> settings can be applied which we omit in our own internal simplified
> version of the logic).
> 
> If I would need to compare the situation to something, imagine that some
> subsystem, MD for example, would be parsing our system.devices file to
> check if it can or can not access certain device. Then checking
> multipath config, then other config and so on... So that way we would
> have each subsystem checking other subsystems by reimplementing these
> checks again and again (with the risk of not even being correct).
> 
> We should be using dedicated APIs or udev to share the information among
> various subsystems. Or, if nothing else, even calling out to the
> subsystem's binary if it doesn't provide the API yet.
> 
> I don't see an issue with the devices file as such, I see the issue in
> the way we gather information, reimplementing the logic that is simply
> not in our domain.
> 
> I think that's also what Martin and Heming have in mind.

I'm not sure if you're talking about things in practice or in principle.
This is a situation where those two don't entirely line up.

In practice, you can already do what's requested with:
  external_device_info_source="udev"
  multipath_wwid_files=""

I've suggested another way to do that which could be more explicit, and
gives more control over exactly what info is used for multipath, e.g.

  multipath_info_sources = [ "option1", "option2", "option3" ]

where options in the list are used in order, and can reference any number
of methods... udev, sysfs, wwids file, some new multipath lib, etc.

The context of using system.devices or not also makes a difference in
these choices, and I haven't thought through that entirely yet.  Perhaps
different defaults should apply when system.devices is in use (i.e. that
multipath_info_sources list may want to depend on using system.devices.)

In principle, the multipath wwids option is an unfortunate, temporary
workaround we tried to avoid.  Principles sometimes give way to real world
problems like systems not booting.  Ideally, a multipath lib API would
have been available to check the wwids (which didn't involve udev.)
There are a couple threads from 2021 about that, initially in

https://listman.redhat.com/archives/lvm-devel/2021-January/022915.html

I didn't have a complete understanding of the mess we were in during that
thread, but it prompted me to begin sorting it all out.  I think it's sane
now, but fine-grained control over what info is used where could be added.
And, system.devices may now let us make some different choices.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Wed, 2023-11-15 at 15:02 -0600, David Teigland wrote:
> On Wed, Nov 15, 2023 at 09:51:19AM +0100, Peter Rajnoha wrote:
> 
> 
> I've suggested another way to do that which could be more explicit,
> and
> gives more control over exactly what info is used for multipath, e.g.
>  
>   multipath_info_sources = [ "option1", "option2", "option3" ]
> 
> where options in the list are used in order, and can reference any
> number
> of methods... udev, sysfs, wwids file, some new multipath lib, etc.

Ugh. That adds even more complexity without improving matters.
How would a user make a reasonable choice for such a setting?
I think even you and I would have a hard time suggesting the "right"
setting. 
Well, obviously my personal suggestion would be to use udev, and udev
alone. :-)

I guess it's too late now, but I'd still be willing to try and help
sorting out the issues that you guys were having with udev's multipath
component detection logic.

> The context of using system.devices or not also makes a difference in
> these choices, and I haven't thought through that entirely yet. 
> Perhaps
> different defaults should apply when system.devices is in use (i.e.
> that
> multipath_info_sources list may want to depend on using
> system.devices.)
> 
> In principle, the multipath wwids option is an unfortunate, temporary
> workaround we tried to avoid.  Principles sometimes give way to real
> world
> problems like systems not booting.  Ideally, a multipath lib API
> would
> have been available to check the wwids (which didn't involve udev.)

There is libmpathvalid which was created for this purpose, have you
tried it? AFAICT it should give the answers that udev would (Ben would
know better though).

Regards
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Wed, Nov 15, 2023 at 09:46:35PM +0000, Martin Wilck wrote:
> Ugh. That adds even more complexity without improving matters.

I have to agree.  The point was, you should be able to configure this to
work as you need it to.  If the settings we've mentioned don't work well
enough, then we can find a simpler option.

> I guess it's too late now, but I'd still be willing to try and help
> sorting out the issues that you guys were having with udev's multipath
> component detection logic.

Right, not much can be done with old releases.

> There is libmpathvalid which was created for this purpose, have you
> tried it? AFAICT it should give the answers that udev would (Ben would
> know better though).

I tried that early on and it was no help because it was touching udev
itself, which is what we were trying to avoid.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Mon, 2023-11-13 at 12:38 -0600, David Teigland wrote:
> On Mon, Nov 13, 2023 at 02:52:39PM +0100, Peter Rajnoha wrote:
> > On 11/13/23 12:52, Martin Wilck wrote:
> > > I can't stress enough that this is *the only mechanism* that
> > > works
> > > correctly. udev serves as central hub to retrieve device
> > > properties
> > > from, and this is how it ought to be.
> > 
> > Indeed, that was exactly one of the primary reasons the
> > 'external_device_info_source="udev"' was added. Reimplementing
> > other
> > subsystem's logic is wrong because there's always some intrinsic
> > logic
> > there that may be omitted. Sooner or later, such approach will
> > fail.
> > 
> > I agree with you here.
> > 
> > If not sharing information through udev due to various kinds of
> > hatred
> > about this part of the system, then alternatively, at least,
> > calling a
> > multipath library function from LVM to do the job.
> 
> Hi, a number of things haven't been mentioned or noticed:
> 
> - In general, it's old versions of lvm that we're discussing here.
> Current lvm uses system.devices by default, where none of this is
> relevant.  We can just turn off a number of filters when
> system.devices is
> in use, including filter-mpath and filter-md.  It may still be
> interesting
> to look at improvements, but the context for that is older stable,
> released versions.

I haven't looked into "system.devices" so far. Is there a high-level
description of this feature available somewhere? A link would be
appreciated. On (open)SUSE, afaik, system.device is not used (Heming,
correct me if I am wrong).

> - In RHEL8 (no system.devices by default, and some older
> systemd/udev),
> lvm can't touch udev without causing timeouts booting with many
> devices.
> Nothing I tried apart from reading wwids would help.  Boot timeouts
> were a
> big support burden, and the motivation for the changes.  (All of this
> added motivation for system.devices.)

We used to see such phenomena, too, but haven't seen them any more
since SLE15-SP4 (~1.5y). Perhaps the most important change at the time
was to start multipathd early, before udev, during boot rather than
waiting for "udevadm settle". But there have been lots of updates in
the various components involved (see list in my last post), so that's
just a guess.

> - To make filter-mpath use udev info, and prevent lvm from using
> wwids,
> you can set external_device_info_source=udev and
> multipath_wwids_file="".
> I think this will be an effective way to avoid any false positive
> problems from native detection.

Good to know, but that's kind of counter-intuitive. I'd prefer
if external_device_info_source=udev did what the name says, and a new
mode ("external_device_info_source=hybrid", say) was introduced for the
current behavior.

Can a distribution set multipath_wwids_file="" by default without
undesired side effects?

Thanks,
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Heming Zhao]

On 11/14/23 18:44, Martin Wilck wrote:
> On Mon, 2023-11-13 at 12:38 -0600, David Teigland wrote:
>> On Mon, Nov 13, 2023 at 02:52:39PM +0100, Peter Rajnoha wrote:
>>> On 11/13/23 12:52, Martin Wilck wrote:
>>>> I can't stress enough that this is *the only mechanism* that
>>>> works
>>>> correctly. udev serves as central hub to retrieve device
>>>> properties
>>>> from, and this is how it ought to be.
>>>
>>> Indeed, that was exactly one of the primary reasons the
>>> 'external_device_info_source="udev"' was added. Reimplementing
>>> other
>>> subsystem's logic is wrong because there's always some intrinsic
>>> logic
>>> there that may be omitted. Sooner or later, such approach will
>>> fail.
>>>
>>> I agree with you here.
>>>
>>> If not sharing information through udev due to various kinds of
>>> hatred
>>> about this part of the system, then alternatively, at least,
>>> calling a
>>> multipath library function from LVM to do the job.
>>
>> Hi, a number of things haven't been mentioned or noticed:
>>
>> - In general, it's old versions of lvm that we're discussing here.
>> Current lvm uses system.devices by default, where none of this is
>> relevant.  We can just turn off a number of filters when
>> system.devices is
>> in use, including filter-mpath and filter-md.  It may still be
>> interesting
>> to look at improvements, but the context for that is older stable,
>> released versions.
> 
> I haven't looked into "system.devices" so far. Is there a high-level
> description of this feature available somewhere? A link would be
> appreciated. On (open)SUSE, afaik, system.device is not used (Heming,
> correct me if I am wrong).

In my view, the system.device was introduced by fixing huge number (>1K)
devices booting timeout issue, which we also discussed 2 or 3 years ago.
Relative to the old methods (event/direct activation), the lvm2 maintainers
hope to use system.device unifying the lvm2 boot code.

In open(SUSE), by default, if the user does not specifically request it,
we do not encourage the user to use system.device. Because we think
system.device is a new feature, it is necessary to wait for a period of time
before recommending it to customers.

> 
>> - In RHEL8 (no system.devices by default, and some older
>> systemd/udev),
>> lvm can't touch udev without causing timeouts booting with many
>> devices.
>> Nothing I tried apart from reading wwids would help.  Boot timeouts
>> were a
>> big support burden, and the motivation for the changes.  (All of this
>> added motivation for system.devices.)
> 
> We used to see such phenomena, too, but haven't seen them any more
> since SLE15-SP4 (~1.5y). Perhaps the most important change at the time
> was to start multipathd early, before udev, during boot rather than
> waiting for "udevadm settle". But there have been lots of updates in
> the various components involved (see list in my last post), so that's
> just a guess.
> 
>> - To make filter-mpath use udev info, and prevent lvm from using
>> wwids,
>> you can set external_device_info_source=udev and
>> multipath_wwids_file="".
>> I think this will be an effective way to avoid any false positive
>> problems from native detection.
> 
> Good to know, but that's kind of counter-intuitive. I'd prefer
> if external_device_info_source=udev did what the name says, and a new
> mode ("external_device_info_source=hybrid", say) was introduced for the
> current behavior.

Yes, hybird is more suitable option value. Obviously, the current mpio filter
behavior is not compatible with the previous one. the udev is not enough
to describe.

Thanks,
Heming

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 14, 2023 at 08:18:14PM +0800, Heming Zhao wrote:
> In my view, the system.device was introduced by fixing huge number (>1K)
> devices booting timeout issue, which we also discussed 2 or 3 years ago.

It does help in that case.  It's also easier to use than creating filters.
It also largely solves the problem of "duplicate PVs" which appear if a
device is cloned/snapshotted/copied, where lvm can't otherwise know which
one is correct.  And the subject of this email is an advantage,
eliminating the problems of multipath/md component detection.

But, I'd say the primary advantage of system.devices is preventing
unexpected/accidental use of PVs that are attached to a system, but not
meant to be used by the system.  It seems more common these days for a LUN
to be visible to a host that should not be used by the host.  This happens
frequently with LUNs passed to VM's, in which case the host should not be
seeing or using that LUN.  In the worst case, the host may actually
autoactivate LVs from a LUN that belongs to a VM, which is also activating
and using those LVs.

> > if external_device_info_source=udev did what the name says, and a new
> > mode ("external_device_info_source=hybrid", say) was introduced for the
> > current behavior.
> 
> Yes, hybird is more suitable option value. Obviously, the current mpio filter
> behavior is not compatible with the previous one. the udev is not enough
> to describe.

As I mentioned in the previous mail, external_device_info_source does not
only apply to multipath, so it's not quite so simple.  I'm pretty sure
that a new setting will be necessary, not just a new value, and perhaps
multipath_wwids_file will work for that already?

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 14, 2023 at 10:44:02AM +0000, Martin Wilck wrote:
> I haven't looked into "system.devices" so far. Is there a high-level
> description of this feature available somewhere? A link would be
> appreciated. On (open)SUSE, afaik, system.device is not used (Heming,
> correct me if I am wrong).

https://man7.org/linux/man-pages/man8/lvmdevices.8.html

If that's too low-level let me know and I'll look at writing something
more conceptual and introductory.

> > - To make filter-mpath use udev info, and prevent lvm from using
> > wwids, you can set external_device_info_source=udev and
> > multipath_wwids_file="".
> 
> Good to know, but that's kind of counter-intuitive. I'd prefer
> if external_device_info_source=udev did what the name says, and a new
> mode ("external_device_info_source=hybrid", say) was introduced for the
> current behavior.

I'd also thought about adding a new option for external_device_info_source
that would mean "udev info only", rather than "both native and udev info."

The complication is that external_device_info_source applies to a few
different kinds of device info, not just multipath, and I doubt we can
apply only udev info to all those cases.  This means adding another config
option... not much different from just using multipath_wwids_file="".

> Can a distribution set multipath_wwids_file="" by default without
> undesired side effects?

Yes, it just causes lvm to skip the use of /etc/multipath/wwids for
checking if a device is a multipath component.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Tue, 2023-11-14 at 11:00 -0600, David Teigland wrote:
> On Tue, Nov 14, 2023 at 10:44:02AM +0000, Martin Wilck wrote:
> > I haven't looked into "system.devices" so far. Is there a high-
> > level
> > description of this feature available somewhere? A link would be
> > appreciated. On (open)SUSE, afaik, system.device is not used
> > (Heming,
> > correct me if I am wrong).
> 
> https://man7.org/linux/man-pages/man8/lvmdevices.8.html
> 
> If that's too low-level let me know and I'll look at writing
> something
> more conceptual and introductory.

It looks allright.

       "If the system devices file does not yet exist, the pvcreate or
       vgcreate commands will create it if they see no existing VGs on
       the system."

That basically means that every new installation (where PVs and VGs
will usually have to be created first) will use system.devices, and
thus disable use of filtering and traditional-style multipath component
detection. Right?
And if a user wanted to use the traditional way of determining devices,
she would need to delete the system.devices file?

> > > - To make filter-mpath use udev info, and prevent lvm from using
> > > wwids, you can set external_device_info_source=udev and
> > > multipath_wwids_file="".
> > 
> > Good to know, but that's kind of counter-intuitive. I'd prefer
> > if external_device_info_source=udev did what the name says, and a
> > new
> > mode ("external_device_info_source=hybrid", say) was introduced for
> > the
> > current behavior.
> 
> I'd also thought about adding a new option for
> external_device_info_source
> that would mean "udev info only", rather than "both native and udev
> info."
> 
> The complication is that external_device_info_source applies to a few
> different kinds of device info, not just multipath, and I doubt we
> can
> apply only udev info to all those cases.

AFAICS the other use case is MD RAID, no? I think that LVM could rely
on udev for that as well. 

>   This means adding another config
> option... not much different from just using multipath_wwids_file="".

For multipath, external_device_info_source=udev used to mean "udev
only" until recently, unless I'm mistaken. The semantics of this
settings has changed. I don't know what the situation was for MD or
possible other subsystems. I'm fine with calling the algorithm "udev"
if the respective semantics are still the same that used to be called
"udev" in the past.

> > Can a distribution set multipath_wwids_file="" by default without
> > undesired side effects?
> 
> Yes, it just causes lvm to skip the use of /etc/multipath/wwids for
> checking if a device is a multipath component.

Thanks. I'm still not happy about it, but if we find no other solution,
I guess it will be ok.

Regards,
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Tue, 2023-11-14 at 18:48 +0100, Martin Wilck wrote:
> On Tue, 2023-11-14 at 11:00 -0600, David Teigland wrote:
> > On Tue, Nov 14, 2023 at 10:44:02AM +0000, Martin Wilck wrote:
> > > I haven't looked into "system.devices" so far. Is there a high-
> > > level
> > > description of this feature available somewhere? A link would be
> > > appreciated. On (open)SUSE, afaik, system.device is not used
> > > (Heming,
> > > correct me if I am wrong).
> > 
> > https://man7.org/linux/man-pages/man8/lvmdevices.8.html
> > 
> > If that's too low-level let me know and I'll look at writing
> > something
> > more conceptual and introductory.
> 
> It looks allright.

    "mpath_uuid is used for dm multipath devices, reported by sysfs."

So users are supposed to use "mpath_uuid" if they want to use multipath
devices as PVs? What if the user simply uses "sys_wwid" or "sys_serial"
for a multipath device? Will LVM perform any kind of multipath
detection, and if yes, how?

Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 14, 2023 at 05:58:52PM +0000, Martin Wilck wrote:
>     "mpath_uuid is used for dm multipath devices, reported by sysfs."
> 
> So users are supposed to use "mpath_uuid" if they want to use multipath
> devices as PVs? What if the user simply uses "sys_wwid" or "sys_serial"
> for a multipath device? Will LVM perform any kind of multipath
> detection, and if yes, how?

LVM will automatically, and only, allow mpath_uuid for multipath devices.

There is a special case that we look for in which a device is used by
itself originally, and included in system.devices with its wwid.  Later,
multipath is set up on that device.  When the multipath device is added to
system.devices, the component device is automatically removed.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Tue, 2023-11-14 at 15:02 -0600, David Teigland wrote:
> 
> LVM will automatically, and only, allow mpath_uuid for multipath
> devices.
> 
> There is a special case that we look for in which a device is used by
> itself originally, and included in system.devices with its wwid. 
> Later,
> multipath is set up on that device.  When the multipath device is
> added to
> system.devices, the component device is automatically removed.

How would that addition of the multipath device come to pass?
By someone explicitly calling "lvmdevices" on it?

1 - someone creates a PV on a normal single-path SCSI device
2 - on subsequent boot, a new path has been added (e.g. by a Fibre
Channel zoning change)
3 - what happens before the user makes any change to system.devices?
    a) LVM attempts to add one of the SCSI (path) devices as PV
       (which might be grabbed by multipathd already)
    b) LVM attempts to add the multipath device automatically,
       somehow figuring out what happened
    c) neither is attempted, and the PV is missing

Thanks,
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Wed, Nov 15, 2023 at 07:35:49AM +0000, Martin Wilck wrote:
> How would that addition of the multipath device come to pass?
> By someone explicitly calling "lvmdevices" on it?

Yes, by running lvmdevices --adddev /dev/mapper/mpatha, or by running
lvmdevices --update, which replaces a componenet device with the multipath
device.

> 1 - someone creates a PV on a normal single-path SCSI device
> 2 - on subsequent boot, a new path has been added (e.g. by a Fibre
> Channel zoning change)
> 3 - what happens before the user makes any change to system.devices?
>     a) LVM attempts to add one of the SCSI (path) devices as PV
>        (which might be grabbed by multipathd already)
>     b) LVM attempts to add the multipath device automatically,
>        somehow figuring out what happened
>     c) neither is attempted, and the PV is missing

If a scsi device suddenly becomes a multipath component, then C.  The
component device is ignored, the PV is missing, and lvm prints:

  WARNING: devices file is missing /dev/mapper/mpatha (253:1) using multipath component /dev/sda.
  See lvmdevices --update for devices file update.

https://sourceware.org/git/?p=lvm2.git;a=blob;f=lib/filters/filter-mpath.c;h=854c26a7c7284c9e566dbd89ea899e554a1c8ed9;hb=HEAD#l37

Maybe there's something better we could do there.
Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Benjamin Marzinski]

On Thu, Nov 16, 2023 at 10:34:51AM -0600, David Teigland wrote:
> On Wed, Nov 15, 2023 at 07:35:49AM +0000, Martin Wilck wrote:
> > How would that addition of the multipath device come to pass?
> > By someone explicitly calling "lvmdevices" on it?
> 
> Yes, by running lvmdevices --adddev /dev/mapper/mpatha, or by running
> lvmdevices --update, which replaces a componenet device with the multipath
> device.
> 
> > 1 - someone creates a PV on a normal single-path SCSI device
> > 2 - on subsequent boot, a new path has been added (e.g. by a Fibre
> > Channel zoning change)
> > 3 - what happens before the user makes any change to system.devices?
> >     a) LVM attempts to add one of the SCSI (path) devices as PV
> >        (which might be grabbed by multipathd already)
> >     b) LVM attempts to add the multipath device automatically,
> >        somehow figuring out what happened
> >     c) neither is attempted, and the PV is missing
> 
> If a scsi device suddenly becomes a multipath component, then C.  The
> component device is ignored, the PV is missing, and lvm prints:
> 
>   WARNING: devices file is missing /dev/mapper/mpatha (253:1) using multipath component /dev/sda.
>   See lvmdevices --update for devices file update.
> 
> https://sourceware.org/git/?p=lvm2.git;a=blob;f=lib/filters/filter-mpath.c;h=854c26a7c7284c9e566dbd89ea899e554a1c8ed9;hb=HEAD#l37
> 
> Maybe there's something better we could do there.

I'm assuming the reality might be different depending on the multipath's
find_multipaths setting. The only values where multipath's behavior can
change depending on the number of paths present are "yes" and "smart".
If "smart" is set, then everything may turn out as you say, but the only
user of find_multipaths "smart" that I know of is the Anaconda installer
for RHEL and Fedora. If find_multipaths is set to "yes", then initially,
multipath won't claim the device, and LVM may well end up trying to use
it and racing with multipathd (possibility A from above).

This is pretty unavoidable if you want multipath to automatically claim
devices only once multiple paths have appeared.

-Ben

> Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 14, 2023 at 05:48:45PM +0000, Martin Wilck wrote:
>        "If the system devices file does not yet exist, the pvcreate or
>        vgcreate commands will create it if they see no existing VGs on
>        the system."
> 
> That basically means that every new installation (where PVs and VGs
> will usually have to be created first) will use system.devices, and
> thus disable use of filtering and traditional-style multipath component
> detection. Right?

Yes, a new install with no visible, existing VGs would run pvcreate or
vgcreate which would create a new system.devices.  That means the regex
filter is disabled, and multipath component detection is unnecessary (we'd
still check when a user adds a new device to system.devices to verify they
aren't doing something dumb.)

The installer should probably decide ahead of time how it wants lvm to
look in the end, and then make that happen, rather than relying on lvm to
adapt to the installer environment.  In the case of anaconda, they set up
new PVs without using system.devices, then at the end it runs
"vgimportdevices -a" to create a new system.devices from the PVs it
created, plus any others that are attached to the system when the
installer is running.  (We've had problems with not planning ahead
sufficiently when it comes to OS images where the "install" doesn't happen
on the actual system.)

The sentence you quoted is not mainly about installation, but rather that
lvm needs to have some kind of defined behavior for itself that's
independent of an installer.

> AFAICS the other use case is MD RAID, no? I think that LVM could rely
> on udev for that as well. 

Right, I don't think we'd risk relying only on udev to detect that when
AFAIK we've always included native detection to some degree.  The
consequences of using a raid image directly can be a bigger problem.  The
other case is detecting if a disk is partitioned (so we can ignore the
whole disk.)

> > This means adding another config
> > option... not much different from just using multipath_wwids_file="".
> 
> For multipath, external_device_info_source=udev used to mean "udev
> only" until recently, unless I'm mistaken. 

Yes, although the default external_device_info_source has always been
"none"... complicated by the fact that at various points in time some
parts of lvm made ad hoc calls to check the udev state regardless of the
none|udev value... it was a mess until I overhauled it all, the settings
were not entirely followed, and it was not possible to control entirely.

> The semantics of this settings has changed. I don't know what the
> situation was for MD or possible other subsystems. I'm fine with calling
> the algorithm "udev" if the respective semantics are still the same that
> used to be called "udev" in the past.

Hm, that past was not as ideal as you imagine, there is no coherent
behavior to restore.  There is the option I offered of a new setting that
means exactly what you need: use only udev for detecting multipath
components.

> > Yes, it just causes lvm to skip the use of /etc/multipath/wwids for
> > checking if a device is a multipath component.
> 
> Thanks. I'm still not happy about it, but if we find no other solution,
> I guess it will be ok.

Could you try multipath_wwids_file="" and see if that's sufficient before
we add a new setting that's nearly the same?

Thanks,
Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Heming Zhao]

On 11/15/23 04:51, David Teigland wrote:
> 
>>> Yes, it just causes lvm to skip the use of /etc/multipath/wwids for
>>> checking if a device is a multipath component.
>>
>> Thanks. I'm still not happy about it, but if we find no other solution,
>> I guess it will be ok.
> 
> Could you try multipath_wwids_file="" and see if that's sufficient before
> we add a new setting that's nearly the same?
> 

I did some test.
1. multipath_wwids_file="" failed boot if without commit 17a3585cbb55d9a15
    So empty value is not enough, or some code logic changed which makes lvm2
    denying multipath dev.
2. using system.devices, machine could boot successfully.

Thanks,
Heming

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Wed, 2023-11-15 at 13:15 +0800, Heming Zhao wrote:
> On 11/15/23 04:51, David Teigland wrote:
> > 
> > > 
> > 
> > Could you try multipath_wwids_file="" and see if that's sufficient
> > before
> > we add a new setting that's nearly the same?
> > 
> 
> I did some test.
> 1. multipath_wwids_file="" failed boot if without commit
> 17a3585cbb55d9a15
>     So empty value is not enough, or some code logic changed which
> makes lvm2
>     denying multipath dev.

Well, 17a3585 ("pvscan: use alternate device names from DEVLINKS to
check filter") fixes a different issue. I had a remark about it in my
first post in this thread, but nobody has replied to that yet (hint:
I'd appreciate if someone did).

The "multipath_wwids_file" test makes most sense with an existing WWIDs
file and "multipath=off" boot parameter, or multipathd disabled in some
other way.

Regards
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Mon, 2023-11-13 at 11:52 +0000, Martin Wilck wrote:
> 
> PS: Here's a related remark about 17a3585 ("pvscan: use alternate
> device names from DEVLINKS to check filter"). I can see why this was
> necessary, but I don't understand why this is found to be necessary
> _now_; the same issue should have always existed if "pvscan" is
> running
> during a "change" event for any given device. The solution of 17a3585
> "worked" for us, but it looks only semi-ok to me. Other udev rules
> may
> modify the DEVLINKS list after pvscan had been running. A correct
> solution must make sure that pvscan runs after all udev rules. IOW,
> pvscan should be triggered in a udev RUN= statement rather then
> IMPORT=. This would probably require a new systemd service, because
> it's not just "pvscan" alone. But the result would be more robust
> then
> what we currently have.

I have seen no responses to this remark ... is it just nonsense?

Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

> PS: Here's a related remark about 17a3585 ("pvscan: use alternate
> device names from DEVLINKS to check filter"). I can see why this was
> necessary, but I don't understand why this is found to be necessary
> _now_; the same issue should have always existed if "pvscan" is running
> during a "change" event for any given device. The solution of 17a3585
> "worked" for us, but it looks only semi-ok to me. Other udev rules may
> modify the DEVLINKS list after pvscan had been running. A correct
> solution must make sure that pvscan runs after all udev rules. IOW,
> pvscan should be triggered in a udev RUN= statement rather then
> IMPORT=. This would probably require a new systemd service, because
> it's not just "pvscan" alone. But the result would be more robust
> then what we currently have.

To be sure we're talking about the same new udev rule, the context is
69-dm-lvm.rules, which does IMPORT from:

  pvscan --cache --listvg --checkcomplete --vgonline \
         --autoactivation event --udevoutput --journal=output /dev/foo

That pvscan prints LVM_VG_NAME_COMPLETE='vgname', which is then passed to
the actual activation command, executed through systemd-run:

  vgchange -aay --autoactivation event $LVM_VG_NAME_COMPLETE

In our environments, this new autoactivation method coincides with default
system.devices.  So, we don't usually see this autoactivation method
combined with the regex filter, and commit 17a3585 doesn't come into play.
However, when the devices file is disabled and users have regex filters
containing /dev/disk/ symlinks, then commit 17a3585 is necessary.
(Avoiding this difficulty is another advantage of system.devices.)

As for the udev rule specifics, the RUN vs IMPORT discussion occured
previously here:
https://listman.redhat.com/archives/linux-lvm/2021-October/026001.html

The answer was that RUN doesn't allow pvscan to provide vgname
(LVM_VG_NAME_COMPLETE) to vgchange.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Tue, 2023-11-21 at 11:56 -0600, David Teigland wrote:
> > PS: Here's a related remark about 17a3585 ("pvscan: use alternate
> > device names from DEVLINKS to check filter"). I can see why this
> > was
> > necessary, but I don't understand why this is found to be necessary
> > _now_; the same issue should have always existed if "pvscan" is
> > running
> > during a "change" event for any given device. The solution of
> > 17a3585
> > "worked" for us, but it looks only semi-ok to me. Other udev rules
> > may
> > modify the DEVLINKS list after pvscan had been running. A correct
> > solution must make sure that pvscan runs after all udev rules. IOW,
> > pvscan should be triggered in a udev RUN= statement rather then
> > IMPORT=. This would probably require a new systemd service, because
> > it's not just "pvscan" alone. But the result would be more robust
> > then what we currently have.
> 
> To be sure we're talking about the same new udev rule, the context is
> 69-dm-lvm.rules, which does IMPORT from:
> 
>   pvscan --cache --listvg --checkcomplete --vgonline \
>          --autoactivation event --udevoutput --journal=output
> /dev/foo
> 
> That pvscan prints LVM_VG_NAME_COMPLETE='vgname', which is then
> passed to
> the actual activation command, executed through systemd-run:
> 
>   vgchange -aay --autoactivation event $LVM_VG_NAME_COMPLETE
> 
> In our environments, this new autoactivation method coincides with
> default
> system.devices.  So, we don't usually see this autoactivation method
> combined with the regex filter, and commit 17a3585 doesn't come into
> play.
> However, when the devices file is disabled and users have regex
> filters
> containing /dev/disk/ symlinks, then commit 17a3585 is necessary.
> (Avoiding this difficulty is another advantage of system.devices.)
> 
> As for the udev rule specifics, the RUN vs IMPORT discussion occured
> previously here:
> https://listman.redhat.com/archives/linux-lvm/2021-October/026001.html
> 
> The answer was that RUN doesn't allow pvscan to provide vgname
> (LVM_VG_NAME_COMPLETE) to vgchange.

Thanks for digging this up. You're of course right, passing this
environment variable back into the udev rule is impossible, given that
RUN is executed after the udev rule processing has finished.

But I wonder if this is required. You could create a script that
calls pvscan, and then systemd-run / vgchange with the output of the
pvscan command, and execute this script in RUN. That was roughly my
idea. That way you wouldn't loose any DEVLINKS added after 69-dm-
lvm.rules.

Thanks
Martin

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[David Teigland]

On Tue, Nov 21, 2023 at 06:10:12PM +0000, Martin Wilck wrote:
> But I wonder if this is required. You could create a script that
> calls pvscan, and then systemd-run / vgchange with the output of the
> pvscan command, and execute this script in RUN. That was roughly my
> idea. That way you wouldn't loose any DEVLINKS added after 69-dm-
> lvm.rules.

I don't see any immediate problem with that idea, although using a script
doesn't sound nice if it can be avoided.  It seems academic since I'm not
seeing a problem with the current method.

Dave

Re: discuss about commit 3b0f9ce: filter-mpath: get wwids from sysfs vpd_pg83

[Martin Wilck]

On Tue, 2023-11-21 at 12:25 -0600, David Teigland wrote:
> On Tue, Nov 21, 2023 at 06:10:12PM +0000, Martin Wilck wrote:
> > But I wonder if this is required. You could create a script that
> > calls pvscan, and then systemd-run / vgchange with the output of
> > the
> > pvscan command, and execute this script in RUN. That was roughly my
> > idea. That way you wouldn't loose any DEVLINKS added after 69-dm-
> > lvm.rules.
> 
> I don't see any immediate problem with that idea, although using a
> script
> doesn't sound nice if it can be avoided.  It seems academic since I'm
> not
> seeing a problem with the current method.

It's sort of academic, agreed. It would be easy to construct a case
where the DEVLINKS approach would fail, but that's unlikely to occur in
practice.

Martin