Re: [PATCH v3 2/3] qemu_cgroup: Handle device mapper targets properly

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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.

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux