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/