Nir Soffer has uploaded a new change for review.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
lvm: Cleanup lvm udev rules
Make the vdsm lvm rules suck less:
- Check the vg name once, instead repating the horrible uuid pattern on every rule - Group rules with same settings (e.g. sanlock rules) - Document each group of rules - Remove irelevant documentation, copied from dm-lvm.rules
Change-Id: Ib185a7d9a5da54036b8b0163a83ff6376b511533 Releates-To: https://bugzilla.redhat.com/1149883 Signed-off-by: Nir Soffer nsoffer@redhat.com --- M vdsm/storage/vdsm-lvm.rules.tpl.in 1 file changed, 17 insertions(+), 20 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/33874/1
diff --git a/vdsm/storage/vdsm-lvm.rules.tpl.in b/vdsm/storage/vdsm-lvm.rules.tpl.in index fb6c87a..bdb33f1 100644 --- a/vdsm/storage/vdsm-lvm.rules.tpl.in +++ b/vdsm/storage/vdsm-lvm.rules.tpl.in @@ -1,5 +1,5 @@ # -# Copyright 2010 Red Hat, Inc. and/or its affiliates. +# Copyright 2010-2014 Red Hat, Inc. and/or its affiliates. # # Licensed to you under the GNU General Public License as published by # the Free Software Foundation; either version 2 of the License, or @@ -7,17 +7,8 @@ # LICENSE_GPL_v2 which accompany this distribution. #
-# Udev rules for LVM. -# -# These rules create symlinks for LVM logical volumes in -# /dev/VG directory (VG is an actual VG name). Some udev -# environment variables are set (they can be used in later -# rules as well): -# DM_LV_NAME - logical volume name -# DM_VG_NAME - volume group name -# DM_LV_LAYER - logical volume layer (blank if not set) +# Vdsm rules for lvm logical volumes. {{if chcon_hack}} -# # The libvirt image label is required to allow qemu to access volumes. Libvirt # sets this label on volumes when starting a vm, but on EL7 and Fedora the # label is lost after refreshing a logical volume, and vm get paused. This rule @@ -28,15 +19,21 @@ # "add" event is processed on coldplug only, so we need "change", too. ACTION!="add|change", GOTO="lvm_end"
-# Fix ownership for RHEV volumes -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@"{{if chcon_hack}}, RUN+="vdsm-chcon"{{endif}}, GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]_MERGE", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="_remove_me_[a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9]_[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="metadata", MODE:="0600", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="ids", MODE:="0660", OWNER:="@VDSMUSER@", GROUP:="@SNLKGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="inbox", MODE:="0600", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="outbox", MODE:="0600", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" -ENV{DM_VG_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", ENV{DM_LV_NAME}=="leases", MODE:="0660", OWNER:="@VDSMUSER@", GROUP:="@SNLKGROUP@", GOTO="lvm_end" +# Filter out vgs which do not look like a vdsm vg +ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end" + +# Volumes used as vdsm images +ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@"{{if chcon_hack}}, RUN+="vdsm-chcon"{{endif}}, GOTO="lvm_end" + +# Temprory volumes - not accessed by libvirt/qemu +ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]_MERGE", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" +ENV{DM_LV_NAME}=="_remove_me_[a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9]_[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" + +# Special storage domain volumes used by vdsm +ENV{DM_LV_NAME}=="metadata|inbox|outbox", MODE:="0600", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" + +# Special storage domain volumes used by sanlock +ENV{DM_LV_NAME}=="ids|leases", MODE:="0660", OWNER:="@VDSMUSER@", GROUP:="@SNLKGROUP@", GOTO="lvm_end"
# FIXME: make special lvs vdsm-only readable, MODE doesn't work on rhel6
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12800/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11852/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12643/ : SUCCESS
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 1: Verified+1
(3 comments)
Verified on rhel6.6 and 7 that rules cleanup did not cause any regressions in devices ownership and permissions.
http://gerrit.ovirt.org/#/c/33874/1/vdsm/storage/vdsm-lvm.rules.tpl.in File vdsm/storage/vdsm-lvm.rules.tpl.in:
Line 6 Line 7 Line 8 Line 9 Line 10 This was also copied from dm-lvm.rules
Line 14 Line 15 Line 16 Line 17 Line 18 This was copied from dm-lvm.rules - this rule (vdsm-lvm) does not set these variables or create symlinks.
Line 27 Line 28 Line 29 Line 30 Line 31 Another RHEV leftover
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 1:
(1 comment)
http://gerrit.ovirt.org/#/c/33874/1/vdsm/storage/vdsm-lvm.rules.tpl.in File vdsm/storage/vdsm-lvm.rules.tpl.in:
Line 19: # "add" event is processed on coldplug only, so we need "change", too. Line 20: ACTION!="add|change", GOTO="lvm_end" Line 21: Line 22: # Filter out vgs which do not look like a vdsm vg Line 23: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end" This could be much nicer if we could replace this horrible uuid pattern with @UUID_PATTERN@, but autoconf removes "[" and "]" characters from variables values.
We can use udev environment variable as a temporary variables in the udev rules, but this seems too hacky. We can also use separate string replacement (not using autoconf) for this, but it seems too much trouble. Line 24: Line 25: # Volumes used as vdsm images Line 26: ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@"{{if chcon_hack}}, RUN+="vdsm-chcon"{{endif}}, GOTO="lvm_end" Line 27:
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 2:
Version 2 rebase over latest changes in master.
Need to verify again to make sure the merge did not introduce any regression.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 2:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12866/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12708/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11917/ : FAILURE
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 3:
Rebase after significant changes in master.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 3:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/12864/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/13816/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13653/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 4:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13916/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13127/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14080/ : FAILURE
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5:
Build Failed
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/13917/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/13128/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/14081/ : FAILURE
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5:
(2 comments)
http://gerrit.ovirt.org/#/c/33874/5/vdsm/storage/vdsm_lvm_rules.template.in File vdsm/storage/vdsm_lvm_rules.template.in:
Line 29: # "add" event is processed on coldplug only, so we need "change", too. Line 30: ACTION!="add|change", GOTO="lvm_end" Line 31: Line 32: # Filter out vgs which do not look like a vdsm vg Line 33: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end" If we are to keep this as a python template, how about using a local CONSTANT for the uuid pattern? And use [0-9a-f]*8 etc? (Can wait for another patch) Line 34: Line 35: # Volumes used as vdsm images Line 36: """) Line 37:
Line 48: """) Line 49: Line 50: write("""\ Line 51: Line 52: # Temprory volumes - not accessed by libvirt/qemu Spelling: Temporary Line 53: ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]_MERGE", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" Line 54: ENV{DM_LV_NAME}=="_remove_me_[a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9]_[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" Line 55: Line 56: # Special volumes used by vdsm
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5:
(2 comments)
http://gerrit.ovirt.org/#/c/33874/5/vdsm/storage/vdsm_lvm_rules.template.in File vdsm/storage/vdsm_lvm_rules.template.in:
Line 29: # "add" event is processed on coldplug only, so we need "change", too. Line 30: ACTION!="add|change", GOTO="lvm_end" Line 31: Line 32: # Filter out vgs which do not look like a vdsm vg Line 33: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end"
If we are to keep this as a python template, how about using a local CONSTA
Maybe a constant for UUID_PATTERN:
ENV{DM_VG_NAME}!="%s", ... % UUID_PATTERN
ENV{DM_LV_NAME}=="%s", ... % UUID_PATTERN
I tried to do this in the past using autoconf, but it broke all the shell special characters. Line 34: Line 35: # Volumes used as vdsm images Line 36: """) Line 37:
Line 48: """) Line 49: Line 50: write("""\ Line 51: Line 52: # Temprory volumes - not accessed by libvirt/qemu
Spelling: Temporary
ok Line 53: ENV{DM_LV_NAME}=="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]_MERGE", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" Line 54: ENV{DM_LV_NAME}=="_remove_me_[a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9][a-zA-Z0-9]_[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@", GOTO="lvm_end" Line 55: Line 56: # Special volumes used by vdsm
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/33874/5/vdsm/storage/vdsm_lvm_rules.template.in File vdsm/storage/vdsm_lvm_rules.template.in:
Line 29: # "add" event is processed on coldplug only, so we need "change", too. Line 30: ACTION!="add|change", GOTO="lvm_end" Line 31: Line 32: # Filter out vgs which do not look like a vdsm vg Line 33: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end"
Maybe a constant for UUID_PATTERN:
autoconf quoting is a bit too much for me to advise about. If you can do it - great.
But I care less about the readability of the generated rule than about the readability of the vdsm_lvm_rules.template python script.
To improve the latter, you don't need autoconf black magic,
HEX='[0-9a-f]' UUID_PATTERN = HEX*8 + '-' + (HEX*4 + '-')*3 + HEX * 12
should be enough. Line 34: Line 35: # Volumes used as vdsm images Line 36: """) Line 37:
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5:
(1 comment)
http://gerrit.ovirt.org/#/c/33874/5/vdsm/storage/vdsm_lvm_rules.template.in File vdsm/storage/vdsm_lvm_rules.template.in:
Line 29: # "add" event is processed on coldplug only, so we need "change", too. Line 30: ACTION!="add|change", GOTO="lvm_end" Line 31: Line 32: # Filter out vgs which do not look like a vdsm vg Line 33: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end"
autoconf quoting is a bit too much for me to advise about. If you can do it
I failed to do it with autoconf, but now when we generate this in Python there is no need for black magic.
I will add another patch on top to make the rules more readable. Line 34: Line 35: # Volumes used as vdsm images Line 36: """) Line 37:
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.ovirt.org/#/c/33874/5/vdsm/storage/vdsm_lvm_rules.template.in File vdsm/storage/vdsm_lvm_rules.template.in:
Line 29: # "add" event is processed on coldplug only, so we need "change", too. Line 30: ACTION!="add|change", GOTO="lvm_end" Line 31: Line 32: # Filter out vgs which do not look like a vdsm vg Line 33: ENV{DM_VG_NAME}!="[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9]-[a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9][a-f0-9]", GOTO="lvm_end"
I failed to do it with autoconf, but now when we generate this in Python th
OK. Line 34: Line 35: # Volumes used as vdsm images Line 36: """) Line 37:
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 6:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 6:
Build Started (1/2) -> http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17448/
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 6:
Build Started (2/2) -> http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17274/
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 6:
This version fix a typo in the udev rule comment and refine the commit message.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 6:
Build Successful
http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/17274/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/17448/ : SUCCESS
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 7:
* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])
Nir Soffer has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 7: Verified+1
Dan Kenigsberg has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 7: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
lvm: Cleanup lvm udev rules
Make the vdsm lvm rules suck less:
- Check the vg name once, instead repeating the uuid pattern - Group rules with same settings (e.g, sanlock rules) - Document each group of rules - Remove irrelevant documentation, copied from dm-lvm.rules
Change-Id: Ib185a7d9a5da54036b8b0163a83ff6376b511533 Releates-To: https://bugzilla.redhat.com/1149883 Signed-off-by: Nir Soffer nsoffer@redhat.com Reviewed-on: https://gerrit.ovirt.org/33874 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/storage/vdsm_lvm_rules.template.in 1 file changed, 15 insertions(+), 19 deletions(-)
Approvals: Nir Soffer: Verified Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: lvm: Cleanup lvm udev rules ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org