https://bugzilla.redhat.com/show_bug.cgi?id=1557769
Problem with device mapper targets is that there can be several
other devices 'hidden' behind them. For instance, /dev/dm-1 can
consist of /dev/sda, /dev/sdb and /dev/sdc. Therefore, when
setting up devices CGroup and namespaces we have to take this
into account.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
libvirt.spec.in | 2 ++
src/qemu/qemu_cgroup.c | 69 +++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 62 insertions(+), 9 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index b55a947ec9..ebfac10866 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -796,6 +796,8 @@ Requires: gzip
Requires: bzip2
Requires: lzop
Requires: xz
+# For mpath devices
+Requires: device-mapper
%if 0%{?fedora} || 0%{?rhel} > 7
Requires: systemd-container
%endif
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index b604edb31c..42502e1b03 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -37,6 +37,7 @@
#include "virtypedparam.h"
#include "virnuma.h"
#include "virsystemd.h"
+#include "virdevmapper.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -60,7 +61,10 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
int perms = VIR_CGROUP_DEVICE_READ;
- int ret;
+ char **targetPaths = NULL;
+ size_t i;
+ int rv;
+ int ret = -1;
if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
return 0;
@@ -71,12 +75,35 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
VIR_DEBUG("Allow path %s, perms: %s",
path, virCgroupGetDevicePermsString(perms));
- ret = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
+ rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
virCgroupGetDevicePermsString(perms),
- ret);
+ rv);
+ if (rv < 0)
+ goto cleanup;
+ if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
+ errno != ENOSYS && errno != EBADF) {
+ virReportSystemError(errno,
+ _("Unable to get devmapper targets for %s"),
+ path);
+ goto cleanup;
+ }
+
+ for (i = 0; targetPaths && targetPaths[i]; i++) {
+ rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, false);
+
+ virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
+ virCgroupGetDevicePermsString(perms),
+ rv);
+ if (rv < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(targetPaths);
return ret;
}
@@ -109,10 +136,11 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
virStorageSourcePtr src)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- int perms = VIR_CGROUP_DEVICE_READ |
- VIR_CGROUP_DEVICE_WRITE |
- VIR_CGROUP_DEVICE_MKNOD;
- int ret;
+ int perms = VIR_CGROUP_DEVICE_RWM;
+ char **targetPaths = NULL;
+ size_t i;
+ int rv;
+ int ret = -1;
if (!virCgroupHasController(priv->cgroup,
VIR_CGROUP_CONTROLLER_DEVICES))
@@ -126,11 +154,34 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
VIR_DEBUG("Deny path %s", src->path);
- ret = virCgroupDenyDevicePath(priv->cgroup, src->path, perms, true);
+ rv = virCgroupDenyDevicePath(priv->cgroup, src->path, perms, true);
virDomainAuditCgroupPath(vm, priv->cgroup, "deny", src->path,
- virCgroupGetDevicePermsString(perms), ret);
+ virCgroupGetDevicePermsString(perms), rv);
+ if (rv < 0)
+ goto cleanup;
+ if (virDevMapperGetTargets(src->path, &targetPaths) < 0 &&
+ errno != ENOSYS && errno != EBADF) {
+ virReportSystemError(errno,
+ _("Unable to get devmapper targets for %s"),
+ src->path);
+ goto cleanup;
+ }
+
+ for (i = 0; targetPaths && targetPaths[i]; i++) {
+ rv = virCgroupDenyDevicePath(priv->cgroup, targetPaths[i], perms, false);
+
+ virDomainAuditCgroupPath(vm, priv->cgroup, "deny", targetPaths[i],
+ virCgroupGetDevicePermsString(perms),
+ rv);
+ if (rv < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ virStringListFree(targetPaths);
return ret;
}
--
2.16.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 11:03:26 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1557769
This BZ is private. Please don't use links which can't be viewed by the
public.
>
> Problem with device mapper targets is that there can be several
> other devices 'hidden' behind them. For instance, /dev/dm-1 can
> consist of /dev/sda, /dev/sdb and /dev/sdc. Therefore, when
> setting up devices CGroup and namespaces we have to take this
> into account.
Also I'd appreciate if you could justify this here. Mention that the
kernel was fixed and this was supposed to be happening from the
beggining.
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> libvirt.spec.in | 2 ++
> src/qemu/qemu_cgroup.c | 69 +++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 62 insertions(+), 9 deletions(-)
[...]
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index b604edb31c..42502e1b03 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
[...]
> @@ -71,12 +75,35 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
> VIR_DEBUG("Allow path %s, perms: %s",
> path, virCgroupGetDevicePermsString(perms));
>
> - ret = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
> + rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
>
> virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
> virCgroupGetDevicePermsString(perms),
> - ret);
> + rv);
> + if (rv < 0)
> + goto cleanup;
>
> + if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
> + errno != ENOSYS && errno != EBADF) {
> + virReportSystemError(errno,
> + _("Unable to get devmapper targets for %s"),
> + path);
> + goto cleanup;
> + }
> +
> + for (i = 0; targetPaths && targetPaths[i]; i++) {
> + rv = virCgroupAllowDevicePath(priv->cgroup, targetPaths[i], perms, false);
> +
> + virDomainAuditCgroupPath(vm, priv->cgroup, "allow", targetPaths[i],
> + virCgroupGetDevicePermsString(perms),
> + rv);
> + if (rv < 0)
> + goto cleanup;
> + }
> +
> + ret = 0;
> + cleanup:
> + virStringListFree(targetPaths);
> return ret;
> }
This looks okay. I did not check that the 'errnos' returned by
libdevmapper are sane given your checks though.
[...]
> @@ -126,11 +154,34 @@ qemuTeardownImageCgroup(virDomainObjPtr vm,
>
> VIR_DEBUG("Deny path %s", src->path);
>
> - ret = virCgroupDenyDevicePath(priv->cgroup, src->path, perms, true);
> + rv = virCgroupDenyDevicePath(priv->cgroup, src->path, perms, true);
>
> virDomainAuditCgroupPath(vm, priv->cgroup, "deny", src->path,
> - virCgroupGetDevicePermsString(perms), ret);
> + virCgroupGetDevicePermsString(perms), rv);
> + if (rv < 0)
> + goto cleanup;
>
> + if (virDevMapperGetTargets(src->path, &targetPaths) < 0 &&
> + errno != ENOSYS && errno != EBADF) {
> + virReportSystemError(errno,
> + _("Unable to get devmapper targets for %s"),
> + src->path);
> + goto cleanup;
> + }
> +
> + for (i = 0; targetPaths && targetPaths[i]; i++) {
> + rv = virCgroupDenyDevicePath(priv->cgroup, targetPaths[i], perms, false);
This isn't a good idea. If a backing device of a device mapper volume is
shared between two disks, this would rip it out of cgroups even if it
would be still used. I think a simple reproducer for this should be if
you use two LVs in the same VG for backing two disks and hot-unplug one
of them.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Apr 03, 2018 at 11:03:26 +0200, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1557769
>
> Problem with device mapper targets is that there can be several
> other devices 'hidden' behind them. For instance, /dev/dm-1 can
> consist of /dev/sda, /dev/sdb and /dev/sdc. Therefore, when
> setting up devices CGroup and namespaces we have to take this
> into account.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> libvirt.spec.in | 2 ++
> src/qemu/qemu_cgroup.c | 69 +++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 62 insertions(+), 9 deletions(-)
[...]
> @@ -71,12 +75,35 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
> VIR_DEBUG("Allow path %s, perms: %s",
> path, virCgroupGetDevicePermsString(perms));
>
> - ret = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
> + rv = virCgroupAllowDevicePath(priv->cgroup, path, perms, true);
So this returns 1 if 'path' is not a char or block device ...
>
> virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path,
> virCgroupGetDevicePermsString(perms),
> - ret);
> + rv);
> + if (rv < 0)
> + goto cleanup;
>
> + if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
> + errno != ENOSYS && errno != EBADF) {
> + virReportSystemError(errno,
So in that case this is definitely not necessary and should be skipped.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.