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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/