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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list