Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lvmdevices: always create devices file #324

Merged
merged 2 commits into from
Oct 4, 2022
Merged

lvmdevices: always create devices file #324

merged 2 commits into from
Oct 4, 2022

Conversation

aesteve-rh
Copy link
Member

@aesteve-rh aesteve-rh commented Sep 27, 2022 •
edited
Loading

Create empty devices file even if no lvm
volumes are found with
vdsm-tool config-lvm-filter.

Otherwise configuring LVM to use a devices
file without the file allows lvm commands
to see all volumes, with the associated risk.

Bug-Url: https://bugzilla.redhat.com/2125290
Signed-off-by: Albert Esteve aesteve@redhat.com

@aesteve-rh aesteve-rh force-pushed the aesteve/always-create-devices-file branch from b35856a to dcd3bc1 Compare September 27, 2022 11:24
@aesteve-rh aesteve-rh self-assigned this Sep 27, 2022
@aesteve-rh aesteve-rh added bug Issue is a bug or fix for a bug storage labels Sep 27, 2022
@aesteve-rh aesteve-rh force-pushed the aesteve/always-create-devices-file branch from dcd3bc1 to 659ee17 Compare September 27, 2022 11:26
@aesteve-rh aesteve-rh added the verified Change was tested; please describe how it was tested in the PR label Sep 27, 2022
@aesteve-rh
Copy link
Member Author

aesteve-rh commented Sep 27, 2022 •
edited
Loading

Installed on a host without lvm mounts and no devices file (lvmdevices.is_configured() returns False):

 $ lvs                                                                                                                                                                                                                                                                                                           
  LV                                   VG                                   Attr       LSize   Pool Origin Data%  Meta%  Move Log Cpy%Sync Convert                                                                                                                                                                            
  12388749-b0fe-4cfb-bfa8-4ab10a24c24f 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------  <4.38g                                                                                                                                                                                                                                
  1eae686c-81ec-44cb-88cf-ffed47c649d3 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi------- 128.00m                                                                                                                                                                                                                                
  39a866f9-cf0f-4326-aabb-4ff0416090a2 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------   2.50g                                                                                                                                                                                                                                
  3f345d1e-789a-4494-97f0-a2138d570386 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------   1.62g                                                                                                                                                                                                                                
  b806a1d4-8e56-4796-beda-b09f8a8231f0 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------   4.12g                                                                                                                                                                                                                                
  c82d5439-c21a-4a41-80d4-59615639a99f 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------   2.50g                                                                                                                                                                                                                                
  e54ab135-fc33-4718-a053-864b9ab24410 38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi------- 128.00m                                                                                                                                                                                                                                
  ids                                  38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi------- 128.00m                                                                                                                                                                                                                                
  inbox                                38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi------- 128.00m                                                                                                                                                                                                                                
  leases                               38f0f4d4-3ae1-4314-bc6d-d1939c50637c -wi-------   2.00g
...

If we run vdsm-tool config-lvm-filter, an empty devices file is created:

$ vdsm-tool config-lvm-filter
Analyzing host...
Found these mounted logical volumes on this host:

To properly configure the host, we need to add multipath
blacklist in /etc/multipath/conf.d/vdsm_blacklist.conf:

  blacklist {
      wwid "0QEMU_QEMU_HARDDISK_data-fc3"
  }


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

Please reboot to verify the configuration.

$ cat /etc/lvm/devices/system.devices
$ ll /etc/lvm/devices/system.devices
-rw-r--r--. 1 root root 0 Sep 27 13:33 /etc/lvm/devices/system.devices

And lvm command does not see any volume:

$ lvs
$

nirs
nirs requested changes Sep 27, 2022
View reviewed changes
lib/vdsm/storage/lvmdevices.py Show resolved Hide resolved
lib/vdsm/storage/lvmdevices.py Outdated Show resolved Hide resolved
@aesteve-rh aesteve-rh force-pushed the aesteve/always-create-devices-file branch from 659ee17 to 487051a Compare September 30, 2022 10:01
@aesteve-rh
Copy link
Member Author

aesteve-rh commented Sep 30, 2022 •
edited
Loading

Verified with latest version (that adds a comment to the devicesfile):

$ cat /etc/lvm/devices/system.devices
# Created by Vdsm pid 7237 at Fri Sep 30 12:00:47 2022

Original LVM header (for reference):

$ cat /etc/lvm/devices/system.devices
# LVM uses devices listed in this file.
# Created by LVM command vgimportdevices pid 48030 at Wed Apr  6 18:20:03 2022

@aesteve-rh
Copy link
Member Author

/ost

@aesteve-rh aesteve-rh force-pushed the aesteve/always-create-devices-file branch from 487051a to 6963951 Compare September 30, 2022 12:58
@aesteve-rh
Copy link
Member Author

/ost

@aesteve-rh @mz-pdm
lvmdevices: remove try...except block
5dbe526 
Remove try...except block around _create_system_devices.
The cmdutils.Error exception is only catched to
log a warning and raise it again. The tool will
not print the log as of now, but even if it did,
logging a warning and raising is a wrong pattern
which adds no valuable information.

Errors that happen during _create_system_devices
should be just propagated to the caller.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
@aesteve-rh @mz-pdm
lvmdevices: always create devices file
aa96827 
Create empty devices file even if no lvm
volumes are found with
`vdsm-tool config-lvm-filter`.

Otherwise configuring LVM to use a devices
file without the file allows lvm commands
to see all volumes, with the associated risk.

Bug-Url: https://bugzilla.redhat.com/2125290
Signed-off-by: Albert Esteve <aesteve@redhat.com>
@mz-pdm mz-pdm force-pushed the aesteve/always-create-devices-file branch from 6963951 to aa96827 Compare October 4, 2022 13:41
@mz-pdm mz-pdm merged commit b8692e9 into oVirt:master Oct 4, 2022
@aesteve-rh aesteve-rh deleted the aesteve/always-create-devices-file branch October 4, 2022 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nirs nirs nirs approved these changes

@barpavel barpavel Awaiting requested review from barpavel

@mkemel mkemel Awaiting requested review from mkemel

Assignees

@aesteve-rh aesteve-rh

Labels
bug Issue is a bug or fix for a bug storage verified Change was tested; please describe how it was tested in the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aesteve-rh @nirs @mz-pdm