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

Re: [libvirt] [PATCH 3/8] Make cgroups a little more efficient



On Wed, Jul 22, 2009 at 04:23:42PM +0100, Daniel P. Berrange wrote:
> * src/cgroup.c: Detect the mount location of every controller at
>   time a virCgroupPtr is created. Detect current process' placement
>   within group to avoid assuming it is in the root. Pass controller
>   ID into SetValueStr/GetValueStr to enable much duplicated code to
>   be eliminated
[...]
> +static int virCgroupDetectMounts(virCgroupPtr group)
[...]
> +                if (typelen == len && STREQLEN(typestr, tmp, len) &&
> +                    !(group->controllers[i].mountPoint = strdup(entry.mnt_dir)))
> +                    goto no_memory;

  Humpf that works but it's a bit hard to be sure we are doing the right
  thing in all cases.

[...]
> +    if (mapping == NULL) {
> +        VIR_ERROR0("Unable to open /proc/self/cgroup");

  relatively low level but shouldn't that be localized ?

[...]
> +static int virCgroupDetect(virCgroupPtr group)
>  {
[...]
> +    rc = virCgroupDetectMounts(group);
> +    if (rc < 0) {
> +        VIR_ERROR("Failed to detect mounts for %s", group->path);

  same here

[...]
> +static int virCgroupCpuSetInherit(virCgroupPtr parent, virCgroupPtr group)
>  {
[...]
> +            VIR_ERROR("Failed to get %s %d", inherit_values[i], rc);
> +            break;
[...]
> +        if (rc != 0) {
> +            VIR_ERROR("Failed to set %s %d", inherit_values[i], rc);
>              break;

  here too, any reason we should not localize ?

[...]
> +static int virCgroupNew(const char *path,
> +                        virCgroupPtr *group)
>  {
>      int rc = 0;
>      char *typpath = NULL;

  it's internal but maybe check for group == NULL since we write there


  Clearly I'm not really following what the patches changes, but code
  looks fine, ACK

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]