Nir Soffer has posted comments on this change.
Change subject: ssl: ssl_accept blocks after reboot
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.ovirt.org/#/c/33643/2/lib/vdsm/sslutils.py
File lib/vdsm/sslutils.py:
Line 25: DEFAULT_ACCEPT_TIMEOUT = 5
Line 26:
Line 27: # M2Crypto.threading needs initialization.
Line 28: # See https://bugzilla.redhat.com/482420
Line 29: threading.init()
Why this is new code? I move this code here few moth ago, after you deleted in one of your patches.
Please separate this to a new patch and explain this change.
Line 30:
Line 31:
Line 32: class SSLSocket(object):
Line 33: def __init__(self, connection):
Line 117:
Line 118: try:
Line 119: client.setup_ssl()
Line 120: client.set_accept_state()
Line 121: client.settimeout(self.accept_timeout)
accept does not work in non-blocking mode?
This is really bad for a non blocking server.
Line 122: client.accept_ssl()
Line 123: client.settimeout(None)
Line 124: except SSL.SSLError as e:
Line 125: raise SSL.SSLError("%s, client %s" % (e, address[0]))
--
To view, visit http://gerrit.ovirt.org/33643
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-HasComments: Yes
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 <nsoffer(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/33492
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce(a)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 <nsoffer(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Dan Kenigsberg has submitted this change and it was merged.
Change subject: ssl: ssl_accept blocks after reboot
......................................................................
ssl: ssl_accept blocks after reboot
When rebooting host most of the times ssl_accept blocks when there is
new connection established just after socket is opened. Setting timeout
on socket seems to help but when testing I noticed sometimes there is
still connection blocked on ssl_accept.
Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Signed-off-by: pkliczewski <piotr.kliczewski(a)gmail.com>
Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/33611
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/sslutils.py
M tests/sslTests.py
2 files changed, 29 insertions(+), 2 deletions(-)
Approvals:
Piotr Kliczewski: Verified
Dan Kenigsberg: Looks good to me, approved
--
To view, visit http://gerrit.ovirt.org/33611
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: ssl_accept blocks after reboot
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.ovirt.org/33611
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
Dan Kenigsberg has posted comments on this change.
Change subject: ssl: ssl_accept blocks after reboot
......................................................................
Patch Set 4: Code-Review-1
(1 comment)
http://gerrit.ovirt.org/#/c/33611/4/tests/sslTests.py
File tests/sslTests.py:
Line 114: server.server.socket.accept_timeout = 1
Line 115: timeout = server.server.socket.accept_timeout + 1
Line 116: server.start()
Line 117: try:
Line 118: client_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
nit picking again: please use
with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as client_socket
and drop the close() on finally. client_socket may not be defined there.
Line 119: client_socket.settimeout(timeout)
Line 120: client_socket.connect((HOST, server.port))
Line 121: # Wait for data that will never arrive.
Line 122: # This will return successfuly if the other side closes the
--
To view, visit http://gerrit.ovirt.org/33611
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I759436b5bfb6c2334d253d12806258cbe1c3720f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali(a)redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes