Enable ZFS storage driver (bz #1471912)

Don't use cgroup mount points from /proc/mounts that are hidden (bz #1470593)
disk driver name=... should be optional (bz #1473091)
This commit is contained in:
Cole Robinson 2017-08-04 16:40:24 -04:00
parent ea04e2dddf
commit 90207a429e
5 changed files with 339 additions and 2 deletions

View File

@ -33,7 +33,7 @@ Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c
index 03410a1..8f1e0b7 100644
index 03410a1a4..8f1e0b7b7 100644
--- a/src/util/vircrypto.c
+++ b/src/util/vircrypto.c
@@ -152,8 +152,14 @@ virCryptoEncryptDataAESgnutls(gnutls_cipher_algorithm_t gnutls_enc_alg,

View File

@ -0,0 +1,90 @@
From: Neal Gompa <ngompa13@gmail.com>
Date: Mon, 17 Jul 2017 11:32:46 -0400
Subject: [PATCH] spec: Add support for building the zfs storage driver
Where it can be supported in Fedora, the driver is built and made
available as a subpackage.
Signed-off-by: Neal Gompa <ngompa13@gmail.com>
(cherry picked from commit 9af764e86aef7dfb0191a9561bf1d1abf941da05)
---
libvirt.spec.in | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 8eb67fa2e..f9a705e7c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -70,6 +70,13 @@
%define with_storage_gluster 0%{!?_without_storage_gluster:1}
%define with_numactl 0%{!?_without_numactl:1}
+# F25+ has zfs-fuse
+%if 0%{?fedora} >= 25
+ %define with_storage_zfs 0%{!?_without_storage_zfs:1}
+%else
+ %define with_storage_zfs 0
+%endif
+
# A few optional bits off by default, we enable later
%define with_fuse 0%{!?_without_fuse:0}
%define with_cgconfig 0%{!?_without_cgconfig:0}
@@ -113,6 +120,12 @@
%endif
%endif
+# zfs-fuse is not available on some architectures
+%ifarch s390 s390x aarch64
+ %define with_storage_zfs 0
+%endif
+
+
# RHEL doesn't ship OpenVZ, VBox, UML, PowerHypervisor,
# VMware, libxenserver (xenapi), libxenlight (Xen 4.1 and newer),
# or HyperV.
@@ -364,6 +377,12 @@ BuildRequires: glusterfs-devel >= 3.4.1
%if %{with_storage_sheepdog}
BuildRequires: sheepdog
%endif
+%if %{with_storage_zfs}
+# Support any conforming implementation of zfs. On stock Fedora
+# this is zfs-fuse, but could be zfsonlinux upstream RPMs
+BuildRequires: /sbin/zfs
+BuildRequires: /sbin/zpool
+%endif
%if %{with_numactl}
# For QEMU/LXC numa info
BuildRequires: numactl-devel
@@ -597,6 +616,11 @@ Requires: device-mapper
# For Sheepdog support
Requires: sheepdog
%endif
+%if %{with_storage_zfs}
+# Support any conforming implementation of zfs
+Requires: /sbin/zfs
+Requires: /sbin/zpool
+%endif
%if %{with_qemu}
# From QEMU RPMs
Requires: /usr/bin/qemu-img
@@ -1063,6 +1087,12 @@ rm -rf .git
%define arg_storage_gluster --without-storage-gluster
%endif
+%if %{with_storage_zfs}
+ %define arg_storage_zfs --with-storage-zfs
+%else
+ %define arg_storage_zfs --without-storage-zfs
+%endif
+
%if %{with_numactl}
%define arg_numactl --with-numactl
%else
@@ -1170,6 +1200,7 @@ rm -f po/stamp-po
%{?arg_storage_rbd} \
%{?arg_storage_sheepdog} \
%{?arg_storage_gluster} \
+ %{?arg_storage_zfs} \
%{?arg_numactl} \
%{?arg_numad} \
--with-capng \

View File

@ -0,0 +1,150 @@
From: Juan Hernandez <jhernand@redhat.com>
Date: Thu, 6 Jul 2017 17:03:31 +0200
Subject: [PATCH] Avoid hidden cgroup mount points
Currently the scan of the /proc/mounts file used to find cgroup mount
points doesn't take into account that mount points may hidden by other
mount points. For, example in certain Kubernetes environments the
/proc/mounts contains the following lines:
cgroup /sys/fs/cgroup/net_prio,net_cls cgroup ...
tmpfs /sys/fs/cgroup tmpfs ...
cgroup /sys/fs/cgroup/net_cls,net_prio cgroup ...
In this particular environment the first mount point is hidden by the
second one. The correct mount point is the third one, but libvirt will
never process it because it only checks the first mount point for each
controller (net_cls in this case). So libvirt will try to use the first
mount point, which doesn't actually exist, and the complete detection
process will fail.
To avoid that issue this patch changes the virCgroupDetectMountsFromFile
function so that when there are duplicates it takes the information from
the last line in /proc/mounts. This requires removing the previous
explicit condition to skip duplicates, and adding code to free the
memory used by the processing of duplicated lines.
Related-To: https://bugzilla.redhat.com/1468214
Related-To: https://github.com/kubevirt/libvirt/issues/4
Signed-off-by: Juan Hernandez <jhernand@redhat.com>
(cherry picked from commit dacd160d7479e0ec2d8a63f102145fd30636a1c8)
---
src/util/vircgroup.c | 23 ++++++++++++++---------
tests/vircgroupdata/kubevirt.mounts | 25 +++++++++++++++++++++++++
tests/vircgroupdata/kubevirt.parsed | 10 ++++++++++
tests/vircgrouptest.c | 1 +
4 files changed, 50 insertions(+), 9 deletions(-)
create mode 100644 tests/vircgroupdata/kubevirt.mounts
create mode 100644 tests/vircgroupdata/kubevirt.parsed
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index f2477d5e9..322f7fb54 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -396,6 +396,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
const char *typestr = virCgroupControllerTypeToString(i);
int typelen = strlen(typestr);
char *tmp = entry.mnt_opts;
+ struct virCgroupController *controller = &group->controllers[i];
while (tmp) {
char *next = strchr(tmp, ',');
int len;
@@ -405,18 +406,22 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
} else {
len = strlen(tmp);
}
- /* NB, the same controller can appear >1 time in mount list
- * due to bind mounts from one location to another. Pick the
- * first entry only
- */
- if (typelen == len && STREQLEN(typestr, tmp, len) &&
- !group->controllers[i].mountPoint) {
+
+ if (typelen == len && STREQLEN(typestr, tmp, len)) {
char *linksrc;
struct stat sb;
char *tmp2;
- if (VIR_STRDUP(group->controllers[i].mountPoint,
- entry.mnt_dir) < 0)
+ /* Note that the lines in /proc/mounts have the same
+ * order than the mount operations, and that there may
+ * be duplicates due to bind mounts. This means
+ * that the same mount point may be processed more than
+ * once. We need to save the results of the last one,
+ * and we need to be careful to release the memory used
+ * by previous processing. */
+ VIR_FREE(controller->mountPoint);
+ VIR_FREE(controller->linkPoint);
+ if (VIR_STRDUP(controller->mountPoint, entry.mnt_dir) < 0)
goto error;
tmp2 = strrchr(entry.mnt_dir, '/');
@@ -452,7 +457,7 @@ virCgroupDetectMountsFromFile(virCgroupPtr group,
VIR_WARN("Expecting a symlink at %s for controller %s",
linksrc, typestr);
} else {
- group->controllers[i].linkPoint = linksrc;
+ controller->linkPoint = linksrc;
}
}
}
diff --git a/tests/vircgroupdata/kubevirt.mounts b/tests/vircgroupdata/kubevirt.mounts
new file mode 100644
index 000000000..ca036196b
--- /dev/null
+++ b/tests/vircgroupdata/kubevirt.mounts
@@ -0,0 +1,25 @@
+rootfs / rootfs rw 0 0
+proc /proc proc rw,nosuid,nodev,noexec,relatime 0 0
+udev /dev devtmpfs rw,nosuid,relatime,size=10240k,nr_inodes=1006404,mode=755 0 0
+devpts /dev/pts devpts rw,relatime,gid=5,mode=620,ptmxmode=000 0 0
+sysfs /sys sysfs rw,nosuid,nodev,noexec,relatime 0 0
+/dev/sda1 / ext4 rw,noatime,data=ordered 0 0
+tmpfs /run tmpfs rw,nodev,relatime,size=812296k,mode=755 0 0
+mqueue /dev/mqueue mqueue rw,nosuid,nodev,noexec,relatime 0 0
+shm /dev/shm tmpfs rw,nosuid,nodev,noexec,relatime 0 0
+debugfs /sys/kernel/debug debugfs rw,nosuid,nodev,noexec,relatime 0 0
+cgroup_root /sys/fs/cgroup tmpfs rw,nosuid,nodev,noexec,relatime,size=10240k,mode=755 0 0
+openrc /sys/fs/cgroup/openrc cgroup rw,nosuid,nodev,noexec,relatime,release_agent=/lib64/rc/sh/cgroup-release-agent.sh,name=openrc 0 0
+cpuset /some/random/location/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0
+cpuset /sys/fs/cgroup/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuset 0 0
+cpu /sys/fs/cgroup/cpu cgroup rw,nosuid,nodev,noexec,relatime,cpu 0 0
+cpuacct /some/random/location/cpuset cgroup rw,nosuid,nodev,noexec,relatime,cpuacct 0 0
+cpuacct /sys/fs/cgroup/cpuacct cgroup rw,nosuid,nodev,noexec,relatime,cpuacct 0 0
+memory /sys/fs/cgroup/memory cgroup rw,nosuid,nodev,noexec,relatime,memory 0 0
+devices /sys/fs/cgroup/devices cgroup rw,nosuid,nodev,noexec,relatime,devices 0 0
+freezer /sys/fs/cgroup/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0
+blkio /sys/fs/cgroup/blkio cgroup rw,nosuid,nodev,noexec,relatime,blkio 0 0
+perf_event /sys/fs/cgroup/perf_event cgroup rw,nosuid,nodev,noexec,relatime,perf_event 0 0
+hugetlb /sys/fs/cgroup/hugetlb cgroup rw,nosuid,nodev,noexec,relatime,hugetlb 0 0
+binfmt_misc /proc/sys/fs/binfmt_misc binfmt_misc rw,nosuid,nodev,noexec,relatime 0 0
+freezer /some/random/location/freezer cgroup rw,nosuid,nodev,noexec,relatime,freezer 0 0
diff --git a/tests/vircgroupdata/kubevirt.parsed b/tests/vircgroupdata/kubevirt.parsed
new file mode 100644
index 000000000..694870723
--- /dev/null
+++ b/tests/vircgroupdata/kubevirt.parsed
@@ -0,0 +1,10 @@
+cpu /sys/fs/cgroup/cpu
+cpuacct /sys/fs/cgroup/cpuacct
+cpuset /sys/fs/cgroup/cpuset
+memory /sys/fs/cgroup/memory
+devices /sys/fs/cgroup/devices
+freezer /some/random/location/freezer
+blkio /sys/fs/cgroup/blkio
+net_cls <null>
+perf_event /sys/fs/cgroup/perf_event
+name=systemd <null>
diff --git a/tests/vircgrouptest.c b/tests/vircgrouptest.c
index f55ef74a1..cf0315f16 100644
--- a/tests/vircgrouptest.c
+++ b/tests/vircgrouptest.c
@@ -885,6 +885,7 @@ mymain(void)
DETECT_MOUNTS("cgroups3");
DETECT_MOUNTS("all-in-one");
DETECT_MOUNTS("no-cgroups");
+ DETECT_MOUNTS("kubevirt");
if (virTestRun("New cgroup for self", testCgroupNewForSelf, NULL) < 0)
ret = -1;

View File

@ -0,0 +1,53 @@
From: Jim Fehlig <jfehlig@suse.com>
Date: Tue, 18 Jul 2017 10:20:35 -0600
Subject: [PATCH] docs: schema: make disk driver name attribute optional
/domain/devices/disk/driver/@name is not a required or mandatory
attribute according to formatdomain, and indeed it was agreed on
IRC that the attribute is "optional for input, recommended (but
not required) for output". Currently the schema requires the
attribute, causing virt-xml-validate to fail on disk config where
the driver name is not explicitly specified. E.g.
# cat test.xml | grep -A 5 cdrom
<disk type='file' device='cdrom'>
<driver type='raw'/>
<target dev='hdb' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='1'/>
</disk>
# virt-xml-validate test.xml
Relax-NG validity error : Extra element devices in interleave
test.xml:21: element devices: Relax-NG validity error : Element domain failed to validate content
test.xml fails to validate
Relaxing the name attribute to be optional fixes the validation
# virt-xml-validate test.xml
test.xml validates
(cherry picked from commit b494e09d058f09b48d0fd8855edd557101294671)
---
docs/schemas/domaincommon.rng | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 9a7d03ed9..38dda780e 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1670,9 +1670,11 @@
</element>
</define>
<define name="driverFormat">
- <attribute name="name">
- <ref name="genericName"/>
- </attribute>
+ <optional>
+ <attribute name="name">
+ <ref name="genericName"/>
+ </attribute>
+ </optional>
<optional>
<attribute name='type'>
<choice>

View File

@ -70,6 +70,13 @@
%define with_storage_gluster 0%{!?_without_storage_gluster:1}
%define with_numactl 0%{!?_without_numactl:1}
# F25+ has zfs-fuse
%if 0%{?fedora} >= 25
%define with_storage_zfs 0%{!?_without_storage_zfs:1}
%else
%define with_storage_zfs 0
%endif
# A few optional bits off by default, we enable later
%define with_fuse 0%{!?_without_fuse:0}
%define with_cgconfig 0%{!?_without_cgconfig:0}
@ -113,6 +120,12 @@
%endif
%endif
# zfs-fuse is not available on some architectures
%ifarch s390 s390x aarch64
%define with_storage_zfs 0
%endif
# RHEL doesn't ship OpenVZ, VBox, UML, PowerHypervisor,
# VMware, libxenserver (xenapi), libxenlight (Xen 4.1 and newer),
# or HyperV.
@ -220,7 +233,7 @@
Summary: Library providing a simple virtualization API
Name: libvirt
Version: 2.2.1
Release: 2%{?dist}%{?extra_release}
Release: 3%{?dist}%{?extra_release}
License: LGPLv2+
Group: Development/Libraries
BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root
@ -233,6 +246,13 @@ Source: http://libvirt.org/sources/%{?mainturl}libvirt-%{version}.tar.xz
# Fix padding of encrypted data (bz #1452622)
Patch0001: 0001-Fix-padding-of-encrypted-data.patch
# Enable ZFS storage driver (bz #1471912)
Patch0002: 0002-spec-Add-support-for-building-the-zfs-storage-driver.patch
# Don't use cgroup mount points from /proc/mounts that are hidden (bz
# #1470593)
Patch0003: 0003-Avoid-hidden-cgroup-mount-points.patch
# disk driver name=... should be optional (bz #1473091)
Patch0004: 0004-docs-schema-make-disk-driver-name-attribute-optional.patch
Requires: libvirt-daemon = %{version}-%{release}
Requires: libvirt-daemon-config-network = %{version}-%{release}
@ -367,6 +387,12 @@ BuildRequires: glusterfs-devel >= 3.4.1
%if %{with_storage_sheepdog}
BuildRequires: sheepdog
%endif
%if %{with_storage_zfs}
# Support any conforming implementation of zfs. On stock Fedora
# this is zfs-fuse, but could be zfsonlinux upstream RPMs
BuildRequires: /sbin/zfs
BuildRequires: /sbin/zpool
%endif
%if %{with_numactl}
# For QEMU/LXC numa info
BuildRequires: numactl-devel
@ -600,6 +626,11 @@ Requires: device-mapper
# For Sheepdog support
Requires: sheepdog
%endif
%if %{with_storage_zfs}
# Support any conforming implementation of zfs
Requires: /sbin/zfs
Requires: /sbin/zpool
%endif
%if %{with_qemu}
# From QEMU RPMs
Requires: /usr/bin/qemu-img
@ -1066,6 +1097,12 @@ rm -rf .git
%define arg_storage_gluster --without-storage-gluster
%endif
%if %{with_storage_zfs}
%define arg_storage_zfs --with-storage-zfs
%else
%define arg_storage_zfs --without-storage-zfs
%endif
%if %{with_numactl}
%define arg_numactl --with-numactl
%else
@ -1173,6 +1210,7 @@ rm -f po/stamp-po
%{?arg_storage_rbd} \
%{?arg_storage_sheepdog} \
%{?arg_storage_gluster} \
%{?arg_storage_zfs} \
%{?arg_numactl} \
%{?arg_numad} \
--with-capng \
@ -1914,6 +1952,12 @@ exit 0
%changelog
* Fri Aug 04 2017 Cole Robinson <crobinso@redhat.com> - 2.2.1-3
- Enable ZFS storage driver (bz #1471912)
- Don't use cgroup mount points from /proc/mounts that are hidden (bz
#1470593)
- disk driver name=... should be optional (bz #1473091)
* Tue May 30 2017 Cole Robinson <crobinso@redhat.com> - 2.2.1-2
- Fix padding of encrypted data (bz #1452622)