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

Re: [libvirt] [PATCH 3/3] Do proper escaping of cgroup resource partitions



On 04/26/2013 04:45 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> If a user cgroup name begins with "cgroup.", "_" or with any of
> the controllers from /proc/cgroups followed by a dot, then they
> need to be prefixed with a single underscore. eg if there is
> an object "cpu.service", then this would end up as "_cpu.service"
> in the cgroup filesystem tree, however, "waldo.service" would
> stay "waldo.service", at least as long as nobody comes up with
> a cgroup controller called "waldo".

Do we also need to consider cgroup names beginning with leading '.'?

> 
> Since we require a '.XXXX' suffix on all partitions, there is
> no scope for clashing with the kernel 'tasks' and 'release_agent'
> files.
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/util/vircgroup.c  | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/vircgroupmock.c | 27 +++++++++++++---
>  tests/vircgrouptest.c | 54 ++++++++++++++++++++++++++++++++
>  3 files changed, 163 insertions(+), 4 deletions(-)

>  #if defined HAVE_MNTENT_H && defined HAVE_GETMNTENT_R
> +static int virCgroupNeedEscape(const char *path)

NeedsEscape sounds a bit better than NeedEscape, if you want to make the
tweak.

> +static int virCgroupEscape(char **path)
> +{
> +    size_t len = strlen(*path);
> +    int rc;
> +
> +    if ((rc = virCgroupNeedEscape(*path)) <= 0)
> +        return rc;
> +

From here...

> +    if (VIR_REALLOC_N(*path, len + 2) < 0)
> +        return -ENOMEM;
> +
> +    memmove((*path) + 1,
> +            *path,
> +            len + 1);
> +    (*path)[0] = '_';

...to here could be simplified to:

char escape = '_';
VIR_INSERT_ELEMENT(*path, 0, len, escape);

> +++ b/tests/vircgroupmock.c
> @@ -66,7 +66,7 @@ static char *fakesysfsdir;
>   * Co-mounting cpu & cpuacct controllers
>   * An anonymous controller for systemd
>   */
> -const char *mounts =
> +const char *procmounts =

I didn't closely inspect the test; but at least the fact that it is
there gives us reassurance that the change is right.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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