lvm devices file configuration #27

vjuranek · 2022-01-05T14:29:11Z

LVM devices file is a new way how specify devices which should be used by LVM and is a replacement for LVM filter. LVM devides file is optional on RHEL 8.5 and the default on RHEL 9. However, LVM filter can still be used if needed. Once LVM is configured to use devices file, LVM filter is ignored, even if explicitly specified.

This enhancement will implement LVM devices file configuration and usage in vdsm and is tracked by BZ #2012830.
As it's very hard to configure the LVM filter in vdsm and in some case we have to ask user to do it manually, using devices files allows us to simplify host configuration and upgrade flows and therefore is preferred way how to specify LVM devices. As mentioned above, LVM filter still can be used, but to simplify the code, vdsm will require to use LVM devices file instead of LVM filter. To make transition more smooth and to have an option how to switch back to LVM fitler when needed, new configuration option will be added into vdsm config. This option will specify, if the LVM should use filter or devices file. This information is also important for executing LVM commands as vdsm uses defensive approach and for each LVM command limits the command scope by specifying the device(s) on which command should be executed. To continue with it, vdsm has to know if it should use filter or devices file.

Host subsystems are configured during installation of upgrade by vdsm configurators. The only exception is LVM filter, which is for historical reasons configured by vdsm-tool config-lvm-filter called directly from Ansible playbook. The required order of operations is as follows:

configure multipath blacklist
configure LVM filter of devices file
configure remaining subsystems

The important point here is to configure LVM devices files before configuring and starting multipath. If we fail to do that, multipath can grab local device and use show them as iSCSI devices in engine. This flow can be implemented using vdsm configurators, but to lower number of changes, current approach is reused and LVM devices configuration is done in config-lvm-filter.

This PR introduces LVM devices configuration:

adding new vdsm configuration option, which determines if LVM should use filter or devices file
enabling LVM devices file in LVM configuration
checking mounted devices on given host and detecting VGs which use these devices
creating initial LVM devices file from these VGs
removing LVM fitler from LVM configuration, if there is any

Next steps:

As mentioned above, vdsm specifies device for each command, using LVM filter. This needs to be replaced by using devices file if vdsm is configured to use devices file.
Check existing code and remove unneeded stuf related to LVM filter.

nirs

There a lot of good changes in this pull request, but I think we need a different design.

Be able to use lvm filter or lvm devices, so we can introduce this feature gradually, and revert back to lvm filter if we find a critical issue with lvm devices. I think using vdsm configuration for this is the most useful way, since vdsm needs to know how to work with lvm.
Do the configuration in the config-lvm-filter tool, which runs before "vdsm-tool configure"
Keep all the code needed to configure the devices file in a separate module, like vdsm.storage.lvmfilter.
Code for getting lvm mounts in vdsm.storage.lvmfilter mdoule should be enhanced and since we need it in 2 different modules, maybe it should move to another module, so both lvmfilter and lvmdevices can use it without stange dependencies between them.
Since both lvmfilter and lvmdevices need similar code, and may need to know about each other (e.g. convert from lvmfilter to lvmdevices or from lvmdevices to lvmfilter), maybe the aditional code should be added to lvmfilter module?
Vdsm need to know how to work with lvm filter or lvm devices. The code for using lvm filter may be removed in a future release, when we are confident about lvm devices.

lib/vdsm/tool/configurators/lvm.py

lib/vdsm/storage/lvmfilter.py

lib/vdsm/tool/configurators/lvm.py

vjuranek · 2022-01-12T11:07:01Z

Accidentally pushed into wrong branch (not my private used for testing), not ready for review yet.

nirs · 2022-01-12T12:28:00Z

@vjuranek maybe convert the PR to Draft? This way the status of the PR is more clear.

Another meta note: This PR comment is not helpful. The only information we have is a link the bug.

In gerrit we did not have the concept of PR comment, so we used to abuse the commit message of the
first patch or use external documents for the same purpose. Lets have a more useful PR comment that:

Explain the big picture
Explain the solution this PR proposes
Mention other solution considered
If this PR is only one step in a bigger work, explain how it fits in the big picture
Report how the PR was tested
Link to external resources
What is the status of the PR (draft that is not ready for review, draft for early review, partial work, complete work, ready/not ready for merge)

For this specific PR, I think we have an issue with the initial design as described in the linked bug.
We need to resolve the design first - it can be done on the bug, or maybe better start this PR with
a document (e.g. doc/lvm-config.md) documenting how configuring lvm filter and devices works.
This document will make it clear where do we want to go and how the system should behave when
this PR is merged.

vjuranek · 2022-01-12T20:40:55Z

Tested manually with vdsm-tool config-lvm-filter on exiting oVirt host.

lib/vdsm/storage/lvmdevices.py

lib/vdsm/tool/config_lvm_filter.py

lib/vdsm/common/config.py.in

vjuranek · 2022-01-25T15:59:01Z

Not tested yet (therefore not requesting review yet).

vjuranek · 2022-01-26T09:23:20Z

Tested manually, configuration with user confirmation:

[root@localhost ~]# cat /etc/vdsm/vdsm.conf.d/99-lvm.conf
[lvm]
config_method= devices
[root@localhost ~]# 
[root@localhost ~]# 
[root@localhost ~]# 
[root@localhost ~]# vdsm-tool config-lvm-filter
Analyzing host...
Configuring LVM to use devices file instead of LVM filter.
Devices for following VGs will be imported:
    
   cs

Found these mounted logical volumes on this host:

  logical volume:  /dev/mapper/cs-root
  mountpoint:      /
  devices:         /dev/vda2

  logical volume:  /dev/mapper/cs-swap
  mountpoint:      [SWAP]
  devices:         /dev/vda2

Configure host? [yes,NO] yes
Configuration completed successfully!

Please reboot to verify the configuration.
    
[root@localhost ~]#      
[root@localhost ~]# 
[root@localhost ~]# 
[root@localhost ~]# grep use_devicesfile /etc/lvm/lvm.conf
        # Configuration option devices/use_devicesfile.
        # use_devicesfile = 1
use_devicesfile = 1
[root@localhost ~]# 
[root@localhost ~]# 
[root@localhost ~]# cat /etc/lvm/devices/system.devices 
# LVM uses devices listed in this file.
# Created by LVM command vgimportdevices pid 4377 at Wed Jan 26 04:04:59 2022
VERSION=1.1.2
IDTYPE=devname IDNAME=/dev/vda2 DEVNAME=/dev/vda2 PVID=ITbnR9fozXVmq5e6iz8WhQxfGqR4pdrd PART=2

Configuration without user confirmation:

[root@localhost ~]# vdsm-tool config-lvm-filter -y
Analyzing host...
Configuring LVM to use devices file instead of LVM filter.
Devices for following VGs will be imported:
    
   cs

Found these mounted logical volumes on this host:

  logical volume:  /dev/mapper/cs-root
  mountpoint:      /
  devices:         /dev/vda2

  logical volume:  /dev/mapper/cs-swap
  mountpoint:      [SWAP]
  devices:         /dev/vda2

Configuration completed successfully!

Please reboot to verify the configuration.

Running config on already configured host:

[root@localhost ~]# vdsm-tool config-lvm-filter -y
Analyzing host...
  Please remove the lvm.conf filter, it is ignored with the devices file.
  Please remove the lvm.conf filter, it is ignored with the devices file.
  Please remove the lvm.conf filter, it is ignored with the devices file.
  Please remove the lvm.conf filter, it is ignored with the devices file.
LVM devices already configured for vdsm

Not sure if we can do anything with messages Please remove the lvm.conf filter, it is ignored with the devices file.. It comes from lvmfilter.vg_devices() when searching for mounts. We have to use filter when configuring devices, so the only way to avoid this message is to first check if host needs config and find mounts only when config is needed. This would require further lvm filter refactroing and not sure if this worth the effort.

vjuranek · 2022-02-01T13:20:42Z

Tested also with host upgrade, after upgrade (and setting config_method= devices before upgrade):

[root@localhost ~]# cat /etc/lvm/devices/system.devices 
# LVM uses devices listed in this file.
# Created by LVM command vgimportdevices pid 12570 at Tue Feb  1 08:06:20 2022
VERSION=1.1.1
IDTYPE=devname IDNAME=/dev/vda2 DEVNAME=/dev/vda2 PVID=ITbnR9fozXVmq5e6iz8WhQxfGqR4pdrd PART=2
[root@localhost ~]# grep use_devicesfile /etc/lvm/lvm.conf
        # Configuration option devices/use_devicesfile.
        # use_devicesfile = 1
use_devicesfile = 1

nirs · 2022-02-01T16:38:27Z

Tested manually, configuration with user confirmation:
...
[root@localhost ~]# vdsm-tool config-lvm-filter
Analyzing host...
Configuring LVM to use devices file instead of LVM filter.
Devices for following VGs will be imported:

cs

Found these mounted logical volumes on this host:

logical volume: /dev/mapper/cs-root
mountpoint: /
devices: /dev/vda2

logical volume: /dev/mapper/cs-swap
mountpoint: [SWAP]
devices: /dev/vda2

Why do show the list of mounted logical volumes before the list of vgs?

Otherwise this looks good.

lib/vdsm/storage/lvmdevices.py

lib/vdsm/tool/config_lvm_filter.py

vjuranek

Output after the changes:

[root@localhost ~]# vdsm-tool config-lvm-filter -y
Analyzing host...
Found these mounted logical volumes on this host:

  logical volume:  /dev/mapper/cs-root
  mountpoint:      /
  devices:         /dev/vda2

  logical volume:  /dev/mapper/cs-swap
  mountpoint:      [SWAP]
  devices:         /dev/vda2

Configuring LVM system.devices.
Devices for following VGs will be imported:
    
 cs

Configuration completed successfully!

Please reboot to verify the configuration.

lib/vdsm/storage/lvmdevices.py

lib/vdsm/tool/config_lvm_filter.py

lib/vdsm/storage/lvmdevices.py

nirs

Thanks! please rebase and run ost.

See #64 (comment) for example how to run ost on the pull request.

nirs · 2022-02-02T18:50:20Z

Thanks! please rebase and run ost.

@vjuranek ost integration does not work yet. If this is verifed, please add the
"verified" label to make it easy to see the status of the pr.

Create new module which will contain functions related to lvm devices file. Add a function which checks if lvm devices file is configured. To use devices file, lvm requires to enable devices/use_devicesfile in lvm configuration and devices file must exists. Change-Id: I75b63f7622fe72fc71f1e3cad5633721ce371ee4 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Add function to configure lvm to enable or disable devices file. Change-Id: I7a0b85abb8089a38f0a41112a84a3b5c7517d93e Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

If lvm devices file wasn't enabled, after enabling it we need to create initial devices file. Find all the mounts and corresponding VGs and import the devices using `vgimportdevices`. To avoid misconfigured lvm filter, which is taken into account during vgimportdevices, enable all the devices. Change-Id: Ib923f0b8c4bc2e5dba79bd8b1e2dfa6d6c0f7c94 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Add function for configuration of lvm devices file. It enables lvm devices and creates initial devices file. As we call `vgimportdevices` with cmd line config option which enables devices file, we can create devices file first and when we succeed, we can enable devices file in lvm config. Change-Id: I86bb113b7792f8fad5aedf6714c1e63748dbcf97 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Run lvmdevices --check after configuring devices file. As, according to LVM developers, the behavior of this functionality is not entirely or strictly well defined yet, don't raise any exception, just log the warning. Change-Id: I098ff50ee5cd9715e0206256b8eed90e6e50cb66 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Once we configure lvm to use devices we will eventually remove exiting lvm filter. Add function for removing filter from lvm config file. Change-Id: I96f9806e32e52e7ab502b6bd1741150bc68ab925 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

For creating lvm devices, we need VGs of mounted devices. This is what lvmfilter.find_lvm_mounts() does. Reuse this code and return also VG names, so we can use it also for lvm devices file. Change-Id: I02d5b4a3610f8372f6e1c66b5ebf69edd756f1e6 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Move various print outs into separate function so the code flow is more clear and some print outs can be later reused. Move filter configuration into separate function as well, as later on another configuration possibility (using devices files) will be added. Change-Id: I9b3bc9c2ffd9cc119e64fa0c55c864c80bc12911 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Change-Id: Iea01e65b53a1740a0e144562c49e93f9da20465a Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Add function for configuration of lvm devices file and adjust lvmdevices.configure() to include also removing lvm filter. In follow up patches printing info and config summary will be added and also the config option for choosing config method will be added. Change-Id: I9d260833a667e1f853d968ba98c189673d9f5da3 Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Print config message for the user about configuring lvm devices and adjust existing print outs to be able to reuse them also for devices. Change-Id: I08d5bc76dba7fa0aec11650944d307cbfeea95db Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Allow user to choose configuration method. This is usefull for testing but also in cases when somethong gone wrong and we need to fall back to lvm filter configuration. As the switch to devices file is not completed yet, old 'filter' method is still the default. Change-Id: Ib576b5af810bfc1701b6ae2a9c670bbce2e36fea Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Change-Id: I391c0edcbe03d010fbbad7909c72f394051b1b3d Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

Disable devices file when searching for mounts and use filter. This is correct as either called when vdsm is configured to use filter and threfore devices file should be diaabled or vdsm is configured to use devices file, but it's not configured yet. This allows us to avoid lvm warnings that we mix devices file and lvm filter. Such situation happends when use configured host with devices files and later wants revert configuration back to filter. Change-Id: I634c6470f8ccfa1300c105ba2bf4f2e03316e16f Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

If there are any issues with devices file on the host or user wants to use lvm filter for any other reason, always disbale devices file in lvm config. It can be turned on from previous run of vdsm config or it needs to be overridden on CentOS 9 where devices file is enabled by default. Change-Id: I14d2f606f4f2dbd7ce485fdbc9b3270ce09a72d8 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

sonarcloud · 2022-02-02T21:21:35Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
2.4% Duplication

nirs

Looks like nothing was change since I approved.

vjuranek · 2022-02-02T23:03:13Z

Verified manually. Also run OST: https://redir.apps.ovirt.org/dj/job/ds-ost-baremetal_manual/27183 (but it's not much relevant to this PR)

vjuranek requested a review from nirs January 5, 2022 14:29

vjuranek changed the title ~~Bz2012830~~ lvm devices file configuration Jan 5, 2022

nirs added the enhancement Enhancing the system by adding new feature or improving performance or reliability label Jan 9, 2022

nirs reviewed Jan 11, 2022

View reviewed changes

vjuranek force-pushed the BZ2012830 branch from 10b98ae to 5e9f353 Compare January 12, 2022 10:25

vjuranek requested review from almusil, mz-pdm and tinez as code owners January 12, 2022 10:25

vjuranek removed request for tinez, mz-pdm and almusil January 12, 2022 11:07

vjuranek marked this pull request as draft January 12, 2022 12:51

vjuranek force-pushed the BZ2012830 branch from 5e9f353 to 45a4085 Compare January 12, 2022 17:30

vjuranek assigned vjuranek and unassigned vjuranek Jan 12, 2022

vjuranek marked this pull request as ready for review January 12, 2022 20:40

vjuranek requested a review from nirs January 12, 2022 20:41

vjuranek mentioned this pull request Jan 19, 2022

Use lvm devices in lvm commands instead of filter #49

Merged

vjuranek force-pushed the BZ2012830 branch 2 times, most recently from 08d528e to ca81772 Compare January 20, 2022 11:14

nirs requested changes Jan 21, 2022

View reviewed changes

vjuranek force-pushed the BZ2012830 branch 2 times, most recently from 4361487 to 7795763 Compare January 25, 2022 15:56

vjuranek requested a review from nirs January 26, 2022 09:24

nirs reviewed Feb 1, 2022

View reviewed changes

vjuranek commented Feb 2, 2022

View reviewed changes

lib/vdsm/storage/lvmdevices.py Outdated Show resolved Hide resolved

lib/vdsm/tool/config_lvm_filter.py Outdated Show resolved Hide resolved

lib/vdsm/tool/config_lvm_filter.py Show resolved Hide resolved

lib/vdsm/storage/lvmdevices.py Show resolved Hide resolved

vjuranek force-pushed the BZ2012830 branch from fe70841 to 83abee3 Compare February 2, 2022 14:07

vjuranek requested a review from nirs February 2, 2022 14:07

nirs previously approved these changes Feb 2, 2022

View reviewed changes

vjuranek added 15 commits February 2, 2022 22:17

lvm: add function for configuring devices file

751d166

Add function to configure lvm to enable or disable devices file. Change-Id: I7a0b85abb8089a38f0a41112a84a3b5c7517d93e Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

lvm: squash all print outs into one function

8312cf0

Change-Id: Iea01e65b53a1740a0e144562c49e93f9da20465a Bug-Url: https://bugzilla.redhat.com/2012830 Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

lvm: add config print outs

38c44eb

Print config message for the user about configuring lvm devices and adjust existing print outs to be able to reuse them also for devices. Change-Id: I08d5bc76dba7fa0aec11650944d307cbfeea95db Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

lvm: move filters into config_with_filter() function

13bb8cf

Change-Id: I391c0edcbe03d010fbbad7909c72f394051b1b3d Signed-off-by: Vojtěch Juránek <vjuranek@redhat.com>

vjuranek dismissed nirs’s stale review via 102c205 February 2, 2022 21:18

vjuranek force-pushed the BZ2012830 branch from 83abee3 to 102c205 Compare February 2, 2022 21:18

nirs approved these changes Feb 2, 2022

View reviewed changes

vjuranek added the verified Change was tested; please describe how it was tested in the PR label Feb 2, 2022

nirs merged commit ac03f59 into oVirt:master Feb 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lvm devices file configuration #27

lvm devices file configuration #27

vjuranek commented Jan 5, 2022 •

edited

nirs left a comment

vjuranek commented Jan 12, 2022

nirs commented Jan 12, 2022

vjuranek commented Jan 12, 2022

vjuranek commented Jan 25, 2022

vjuranek commented Jan 26, 2022

vjuranek commented Feb 1, 2022

nirs commented Feb 1, 2022

vjuranek left a comment

nirs left a comment

nirs commented Feb 2, 2022

sonarcloud bot commented Feb 2, 2022

nirs left a comment

vjuranek commented Feb 2, 2022

lvm devices file configuration #27

lvm devices file configuration #27

Conversation

vjuranek commented Jan 5, 2022 • edited

nirs left a comment

Choose a reason for hiding this comment

vjuranek commented Jan 12, 2022

nirs commented Jan 12, 2022

vjuranek commented Jan 12, 2022

vjuranek commented Jan 25, 2022

vjuranek commented Jan 26, 2022

vjuranek commented Feb 1, 2022

nirs commented Feb 1, 2022

vjuranek left a comment

Choose a reason for hiding this comment

nirs left a comment

Choose a reason for hiding this comment

nirs commented Feb 2, 2022

sonarcloud bot commented Feb 2, 2022

nirs left a comment

Choose a reason for hiding this comment

vjuranek commented Feb 2, 2022

vjuranek commented Jan 5, 2022 •

edited