Hello Federico Simoncelli, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/33627

to review the following change.

Change subject: lvm: Set libvirt image selinux label on block devices backing 
vdsm images
......................................................................

lvm: Set libvirt image selinux label on block devices backing vdsm images

The SELinux sVirt protection for QEMU virtual machines is setup in such
a way that a domain can only access files or devices which are labelled
svirt_image_t label. Libvirt sets this label on block devices backing
images when it starts a vm.

On Fedora 19, 20 and EL 7, the selinux label on the block device is lost
after refreshing a logical volume.  The root cause of this issue is
systemd-udevd, trying to "preserve" the selinux label upon device change
event.

Loosing the selinux label causes the vm to pause. The only way to use
the vm is to restart the vm.  Practically, this breaks thin provisioning
on block storage, since after each automatic extend, a logical volume
must be refreshed.

This patch adds a temporary hack, by updating vdsm lvm rules to set the
libvirt image selinux label on vdsm images. This change should be
reverted when a fix is available in systemd-udevd.

This hack is enabled by default only for EL7, since we hope to get a
fix for systemd-udevd soon for Fedora.

To enable this hack on other platforms:

    ./configure --enable-chcon-hack

Change-Id: I95f85c7b548b2c058693b20b1fa177714a6e1a10
Bug-Url: https://bugzilla.redhat.com/1127460
Releates-To: https://bugzilla.redhat.com/1147910
Signed-off-by: Nir Soffer <nsof...@redhat.com>
Reviewed-on: http://gerrit.ovirt.org/33492
Reviewed-by: Dan Kenigsberg <dan...@redhat.com>
Reviewed-by: Federico Simoncelli <fsimo...@redhat.com>
---
M configure.ac
M vdsm.spec.in
M vdsm/storage/Makefile.am
R vdsm/storage/vdsm-lvm.rules.tpl.in
4 files changed, 52 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/27/33627/1

diff --git a/configure.ac b/configure.ac
index c35289b..4261216 100644
--- a/configure.ac
+++ b/configure.ac
@@ -56,6 +56,17 @@
 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],
@@ -110,6 +121,9 @@
     [with_libvirt_service_default="${sysconfdir}/sysconfig/libvirtd"]
 )
 AC_SUBST([LIBVIRT_SERVICE_DEFAULT], ["${with_libvirt_service_default}"])
+
+AC_SUBST([LIBVIRT_IMAGE_LABEL], ['svirt_image_t'])
+
 
 # Users and groups
 AC_SUBST([VDSMUSER], [vdsm])
@@ -191,6 +205,7 @@
 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])
@@ -281,7 +296,7 @@
        vdsm/storage/Makefile
        vdsm/storage/imageRepository/Makefile
        vdsm/storage/protect/Makefile
-       vdsm/storage/vdsm-lvm.rules
+       vdsm/storage/vdsm-lvm.rules.tpl
        vdsm/virt/Makefile
        vdsm_hooks/Makefile
        vdsm_hooks/checkimages/Makefile
diff --git a/vdsm.spec.in b/vdsm.spec.in
index ca2e86f..205715a 100644
--- a/vdsm.spec.in
+++ b/vdsm.spec.in
@@ -35,6 +35,10 @@
 %global with_vhostmd 1
 %endif
 
+%if 0%{?rhel} >= 7
+%global with_chcon_hack 1
+%endif
+
 %if 0%{?fedora} >= 15 || 0%{?rhel} >= 7
 %global with_systemd 1
 %endif
@@ -637,7 +641,7 @@
 %if 0%{?enable_autotools}
 autoreconf -if
 %endif
-%configure %{?with_hooks:--enable-hooks}
+%configure %{?with_hooks:--enable-hooks} 
%{?with_chcon_hack:--enable-chcon-hack}
 make
 # Setting software_version and software_revision in dsaversion.py
 baserelease=`echo "%{release}" | sed 's/\([0-9]\+\(\.[0-9]\+\)\?\).*/\1/'`
diff --git a/vdsm/storage/Makefile.am b/vdsm/storage/Makefile.am
index 99b1460..89fa1e5 100644
--- a/vdsm/storage/Makefile.am
+++ b/vdsm/storage/Makefile.am
@@ -81,3 +81,22 @@
 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-lvm.rules.in b/vdsm/storage/vdsm-lvm.rules.tpl.in
similarity index 88%
rename from vdsm/storage/vdsm-lvm.rules.in
rename to vdsm/storage/vdsm-lvm.rules.tpl.in
index 341278d9..0869cdf 100644
--- a/vdsm/storage/vdsm-lvm.rules.in
+++ b/vdsm/storage/vdsm-lvm.rules.tpl.in
@@ -16,12 +16,23 @@
 #   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
+#
+# TODO: use SECLABEL{selinux}="@LIBVIRT_IMAGE_LABEL@" when this syntax is
+# supported. See https://bugzilla.redhat.com/1015300
+{{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@", 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]",
 OWNER:="@VDSMUSER@", GROUP:="@QEMUGROUP@"{{if chcon_hack}}, RUN+="@CHCON_PATH@ 
-t @LIBVIRT_IMAGE_LABEL@ $env{DEVNAME}"{{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"


-- 
To view, visit http://gerrit.ovirt.org/33627
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I95f85c7b548b2c058693b20b1fa177714a6e1a10
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to