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

Re: [libvirt] [PATCH 6/7] Use cgroups for block device whitelisting in QEMU guests



On Fri, Jul 17, 2009 at 09:04:28AM -0400, Daniel P. Berrange wrote:
> * src/qemu_driver.c: Set a restrictive block device whitelist for
>   all QEMU guests. Update whitelist when hotplugging disks.
> * src/cgroup.h, src/cgroup.c: Add some more convenience methods
>   for dealing with block device whitelists.

> +}

Worth adding a description for this function too, especially the return
value which is not trivial from reading the code.

> +
> +int virCgroupDenyDevicePath(virCgroupPtr group,
> +                             const char *path)
> +{
> +    struct stat sb;
> +
> +    if (stat(path, &sb) < 0)
> +        return -errno;
> +
> +    if (!S_ISCHR(sb.st_mode) && !S_ISBLK(sb.st_mode))
> +        return -EINVAL;
> +
> +    return virCgroupDenyDevice(group,
> +                               S_ISCHR(sb.st_mode) ? 'c' : 'b',
> +                               major(sb.st_rdev),
> +                               minor(sb.st_rdev));
> +}
[...]
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
> index f6d3f52..33e9cfa 100644
> --- a/src/qemu_driver.c
> +++ b/src/qemu_driver.c
> @@ -1378,12 +1378,19 @@ error:
>      return -1;
>  }
>  
> +static const char *const devs[] = {
> +    "/dev/null", "/dev/full", "/dev/zero",
> +    "/dev/random", "/dev/urandom",
> +    "/dev/ptmx", "/dev/kvm", "/dev/kqemu",
> +};

  Hum, that list sounds a bit arbitrary, this could break for random
reasons  maybe this should be extended through the configuration, I
assume a mismatch may result in domain failing to start or operate
properly, right ?

[...]
> +
> +    rc = virCgroupAllowDeviceMajor(cgroup, 'c', 136);

  Hum, that's a magic number, can we get a meaningful #define


   The idea sounds good but I'm a bit afraid of the inflexibility,
this has the potential of making qemu/kvm far more fragile without
a way to fix this by patching and recompiling.
   Again I'm not a cgroup expert but I feel a bit uneasy, can we get
at least an option to disable it at runtime in the configuration (sorry
if I missed that !) ?

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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