6:47 p.m.
In a system that uses dynamic_ownership=0 and NFS disks, the _only_
path in the virDomainDestroy path where libvirt itself performs a
syscall on the NFS file system was on attempting to relabel a disk
back to its default label. If an NFS server hosting a domain's
disk image hangs, then this renders libvirtd stuck in an lstat()
call, with the driver lock held, effectively blocking out _all_
other attempts to connect to libvirtd for any remaining domains not
tied to the hung NFS server. (Verification that the SELinux relabel
was the only point where libvirtd, rather than not qemu, could hang,
was done by Red Hat VDSM folks in https://bugzilla.redhat.com/746666;
with a hack to avoid the relabel, the destroy no longer hangs.)
But without bleeding edge NFS servers, you can't put a SELinux
label on the file in the first place; libvirt was happily ignoring
the failure to change the label on domain start (thanks to the
selinux virt_use_nfs bool), only to hang on the stuck server on
domain destroy that it would have again ignored had the server not
been stuck. If we allow the users to modify the XML to mark that
a file does not need to be labeled in the first place, then we can
avoid both attempts to label at startup and relabel at destroy, thus
preventing libvirtd from getting stuck on a virDomainDestroy()
called on a guest whose NFS server went down.
This patch series is based on an earlier attempt (the hack which
got tested in the above-mentioned bz 746666), incorporating feedback
from Dan Berrange suggesting that the ability for the user to
override labeling on a per-disk basis is smarter than marking
ignored failure to label in internal XML used only by libvirtd:
v1: https://www.redhat.com/archives/libvir-list/2011-December/msg00246.html
I'm still working on patch 6/6 (actually hooking up the security
manager to honor the relabel='no' request), and hope to post that
tomorrow as part of this thread; that's really the only patch that
resembles anything from v1 (the first 5 patches are basically brand
new to this series), but I'm posting the first 5 patches now to get
review started and make sure the proposed XML makes sense.
I envision that this can be further expanded (sticking <seclabel>
elements on more XML items that get labeled in the host), but for
now I focused just on disk devices, as those are the most likely
to reside over NFS.
Ultimately, this patch series is only a bandaid; the underlying
problem (making a syscall that can block indefinitely on a hung
NFS server while holding the driver lock) is still present, and
triggered if you have dynamic_ownership=1 (where we also attempt
a chown of the disk image, regardless of the SELinux labels).
Later down the road, I also plan to work on Dan's proposal to
break the driver lock into smaller, more manageable sections, so
we can get back to our documented goal of never blocking while
holding lock, but instead using the job condition variables to
detect when a single domain is stuck on a blocked call:
https://www.redhat.com/archives/libvir-list/2011-November/msg00267.html
Eric Blake (5):
schema: rewrite seclabel rng to match code
seclabel: refactor existing domain_conf usage
seclabel: move seclabel stuff earlier
seclabel: extend XML to allow per-disk label overrides
seclabel: allow a seclabel override on a disk src
docs/formatdomain.html.in | 29 ++-
docs/schemas/domaincommon.rng | 115 +++++--
src/conf/domain_conf.c | 414 ++++++++++++--------
src/conf/domain_conf.h | 39 +-
.../qemuxml2argv-seclabel-dynamic-baselabel.args | 4 +
.../qemuxml2argv-seclabel-dynamic-baselabel.xml | 28 ++
.../qemuxml2argv-seclabel-dynamic-override.args | 5 +
.../qemuxml2argv-seclabel-dynamic-override.xml | 40 ++
.../qemuxml2argv-seclabel-dynamic.args | 4 +
.../qemuxml2argv-seclabel-dynamic.xml | 26 ++
.../qemuxml2argv-seclabel-static-relabel.args | 4 +
.../qemuxml2argv-seclabel-static-relabel.xml | 29 ++
.../qemuxml2argv-seclabel-static.args | 4 +
.../qemuxml2argv-seclabel-static.xml | 28 ++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c | 4 +
16 files changed, 575 insertions(+), 204 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-baselabel.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-relabel.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static-relabel.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-static.xml
--
1.7.7.4