Change in vdsm[ovirt-3.5]: lvm: Do not use udev to set permissions on vdsm images
nsoffer at redhat.com
nsoffer at redhat.com
Wed Oct 8 13:01:15 UTC 2014
Hello Federico Simoncelli,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/33931
to review the following change.
Change subject: lvm: Do not use udev to set permissions on vdsm images
......................................................................
lvm: Do not use udev to set permissions on vdsm images
udev has changed the rules recently, so setting USER, GROUP, or MODE
will change also the selinux label behind your back. This issue caused
vms to pause after extend, rendering thin provisioning disk useless.
We had a temporary fix, applying a static selinux label on vdsm images,
but this breaks libvirt security. This patch avoids this issue by not
using udev to set any permissions, thus preserving the secure selinux
labels set by libvirt.
This patch replaces the following commits with a simpler and hopefully
longer lasting fix which does not need any platform specific code.
d8d6c17 gitignore: Ingore vdsm-lvm.rule.tpl
00fbc83 lvm: Modify lv selinux label only if not labablled as libvirt image
b2268e4 spec: Enable lvm selinux fix for Fedora
75fc495 lvm: Set libvirt image selinux label on block devices backing vdsm images
Change-Id: I57d9987bf0be19e6e233baaeea10877918eb849b
Bug-Url: https://bugzilla.redhat.com/1149883
Relates-To: https://bugzilla.redhat.com/1147910
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/33875
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M .gitignore
M configure.ac
M vdsm.spec.in
M vdsm/storage/Makefile.am
D vdsm/storage/vdsm-chcon.in
R vdsm/storage/vdsm-lvm.rules.in
6 files changed, 9 insertions(+), 78 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/31/33931/1
diff --git a/.gitignore b/.gitignore
index da260eb..efb99aa 100644
--- a/.gitignore
+++ b/.gitignore
@@ -57,9 +57,7 @@
vdsm/sos/vdsm.py
vdsm/storage/protect/safelease
vdsm/storage/lvm.env
-vdsm/storage/vdsm-chcon
vdsm/storage/vdsm-lvm.rules
-vdsm/storage/vdsm-lvm.rules.tpl
vdsm/sudoers.vdsm
vdsm/svdsm.logger.conf
vdsm/vdscli.py
diff --git a/configure.ac b/configure.ac
index 9938ba1..afe6d32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -56,17 +56,6 @@
AM_CONDITIONAL([HOOKS], [test "${enable_hooks}" = "yes"])
AC_ARG_ENABLE(
- [chcon_hack],
- [AS_HELP_STRING(
- [--enable-chcon-hack],
- [enable chcon hack for block devices @<:@default=no@:>@]
- )],
- ,
- [enable_chcon_hack="no"]
-)
-AM_CONDITIONAL([CHCON_HACK], [test "${enable_chcon_hack}" = "yes"])
-
-AC_ARG_ENABLE(
[libvirt-sanlock],
[AS_HELP_STRING(
[--disable-libvirt-sanlock],
@@ -121,11 +110,6 @@
[with_libvirt_service_default="${sysconfdir}/sysconfig/libvirtd"]
)
AC_SUBST([LIBVIRT_SERVICE_DEFAULT], ["${with_libvirt_service_default}"])
-
-
-# Selinux image label
-AC_SUBST([SVIRT_IMAGE_LABEL], ['svirt_image_t'])
-AC_SUBST([SVIRT_CONTENT_LABEL], ['svirt_content_t'])
# Users and groups
@@ -208,7 +192,6 @@
AC_PATH_PROG([BLKID_PATH], [blkid], [/sbin/blkid])
AC_PATH_PROG([BRCTL_PATH], [brctl], [/usr/sbin/brctl])
AC_PATH_PROG([CAT_PATH], [cat], [/bin/cat])
-AC_PATH_PROG([CHCON_PATH], [chcon], [/bin/chcon])
AC_PATH_PROG([CHKCONFIG_PATH], [chkconfig], [/sbin/chkconfig])
AC_PATH_PROG([CHMOD_PATH], [chmod], [/bin/chmod])
AC_PATH_PROG([CHOWN_PATH], [chown], [/bin/chown])
@@ -231,7 +214,6 @@
AC_PATH_PROG([ISCSIADM_PATH], [iscsiadm], [/sbin/iscsiadm])
AC_PATH_PROG([KILL_PATH], [kill], [/bin/kill])
AC_PATH_PROG([LSBLK_PATH], [lsblk], [/bin/lsblk])
-AC_PATH_PROG([LS_PATH], [ls], [/bin/ls])
AC_PATH_PROG([LVM_PATH], [lvm], [/sbin/lvm])
AC_PATH_PROG([MKFS_MSDOS_PATH], [mkfs.msdos], [/sbin/mkfs.msdos])
AC_PATH_PROG([MKFS_PATH], [mkfs], [/sbin/mkfs])
@@ -300,8 +282,7 @@
vdsm/storage/Makefile
vdsm/storage/imageRepository/Makefile
vdsm/storage/protect/Makefile
- vdsm/storage/vdsm-chcon
- vdsm/storage/vdsm-lvm.rules.tpl
+ vdsm/storage/vdsm-lvm.rules
vdsm/virt/Makefile
vdsm_hooks/Makefile
vdsm_hooks/checkimages/Makefile
diff --git a/vdsm.spec.in b/vdsm.spec.in
index 3e42f37..fd8c709 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -35,10 +35,6 @@
%global with_vhostmd 1
%endif
-%if 0%{?fedora} >= 19 || 0%{?rhel} >= 7
-%global with_chcon_hack 1
-%endif
-
%if 0%{?fedora} >= 15 || 0%{?rhel} >= 7
%global with_systemd 1
%endif
@@ -643,7 +639,7 @@
%if 0%{?enable_autotools}
autoreconf -if
%endif
-%configure %{?with_hooks:--enable-hooks} %{?with_chcon_hack:--enable-chcon-hack}
+%configure %{?with_hooks:--enable-hooks}
make
# Setting software_version and software_revision in dsaversion.py
baserelease=`echo "%{release}" | sed 's/\([0-9]\+\(\.[0-9]\+\)\?\).*/\1/'`
@@ -669,11 +665,6 @@
# Install the lvm rules
install -Dm 0644 vdsm/storage/vdsm-lvm.rules \
%{buildroot}%{_udevrulesdir}/12-vdsm-lvm.rules
-
-%if 0%{?with_chcon_hack}
-install -Dm 0755 vdsm/storage/vdsm-chcon \
- %{buildroot}%{_udevexecdir}/vdsm-chcon
-%endif
install -Dm 0644 vdsm/limits.conf \
%{buildroot}/etc/security/limits.d/99-vdsm.conf
@@ -1150,9 +1141,6 @@
%endif
%{python_sitelib}/sos/plugins/vdsm.py*
%{_udevrulesdir}/12-vdsm-lvm.rules
-%if 0%{?with_chcon_hack}
-%{_udevexecdir}/vdsm-chcon
-%endif
/etc/security/limits.d/99-vdsm.conf
%{_mandir}/man8/vdsmd.8*
%if 0%{?rhel}
diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am
index 89fa1e5..99b1460 100644
--- a/vdsm/storage/Makefile.am
+++ b/vdsm/storage/Makefile.am
@@ -81,22 +81,3 @@
EXTRA_DIST = \
lvm.env.in \
$(NULL)
-
-all-local: vdsm-lvm.rules
-
-vdsm-lvm.rules: vdsm-lvm.rules.tpl
-if CHCON_HACK
- python -c '\
- import sys, re; \
- s = open(sys.argv[1]).read(); \
- pat = re.compile(r"{{.+?}}\n?", re.S); \
- s = pat.sub("", s); \
- sys.stdout.write(s)' $< > $@;
-else
- python -c '\
- import sys, re; \
- s = open(sys.argv[1]).read(); \
- pat = re.compile(r"{{if chcon_hack}}\n?.+?{{endif}}\n?", re.S); \
- s = pat.sub("", s); \
- sys.stdout.write(s)' $< > $@;
-endif
diff --git a/vdsm/storage/vdsm-chcon.in b/vdsm/storage/vdsm-chcon.in
deleted file mode 100644
index 6f1eb6e..0000000
--- a/vdsm/storage/vdsm-chcon.in
+++ /dev/null
@@ -1,14 +0,0 @@
-#!/bin/sh
-
-# This script must be called from a udev rule and assumes the udev environment
-# variables.
-
-# Do not touch the device if it is already labelled is libvirt image. It will
-# probably be a fixed_disk_t or it may have no selinux label.
-if @LS_PATH@ -Z "$DEVNAME" | \
- @GREP_PATH@ -q -E ":@SVIRT_CONTENT_LABEL@:|:@SVIRT_IMAGE_LABEL@:"; then
- exit 0
-fi
-
-echo "Changing selinux type to @SVIRT_IMAGE_LABEL@ on $DEVNAME" >&2
- at CHCON_PATH@ -t @SVIRT_IMAGE_LABEL@ "$DEVNAME"
diff --git a/vdsm/storage/vdsm-lvm.rules.tpl.in b/vdsm/storage/vdsm-lvm.rules.in
similarity index 89%
rename from vdsm/storage/vdsm-lvm.rules.tpl.in
rename to vdsm/storage/vdsm-lvm.rules.in
index fb6c87a..7a1a1b2 100644
--- a/vdsm/storage/vdsm-lvm.rules.tpl.in
+++ b/vdsm/storage/vdsm-lvm.rules.in
@@ -16,20 +16,17 @@
# DM_LV_NAME - logical volume name
# DM_VG_NAME - volume group name
# DM_LV_LAYER - logical volume layer (blank if not set)
-{{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
-# ensures that the label exist after device changes. See
-# https://bugzilla.redhat.com/1147910
-{{endif}}
# "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"
+# Volumes used as vdsm images
+# WARNING: we cannot use USER, GROUP and MODE since using any of them will
+# change the selinux label to the default, causing vms to pause after extending
+# disks. https://bugzilla.redhat.com/1147910
+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]", RUN+="@CHOWN_PATH@ @VDSMUSER@:@QEMUGROUP@ $env{DEVNAME}", GOTO="lvm_end"
+
+# Other volumes used by vdsm
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"
--
To view, visit http://gerrit.ovirt.org/33931
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I57d9987bf0be19e6e233baaeea10877918eb849b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
More information about the vdsm-patches
mailing list