Re: [libvirt] [PATCH 02/13] Introduce the function virCgroupMoveTask
hi~
> +int virCgroupMoveTask(virCgroupPtr src_group, virCgroupPtr
dest_group)
> +{
> + int rc = 0;
> + int i;
> + char *content, *value, *next;
> +
> + for (i = 0 ; i < VIR_CGROUP_CONTROLLER_LAST ; i++) {
> + /* Skip over controllers not mounted */
> + if (!src_group->controllers[i].mountPoint ||
> + !dest_group->controllers[i].mountPoint)
> + continue;
Should we insist that src_group and dest_group have the same set of
mounted controllers? I'm worried that if we call this function, but the
set of mounted controllers differs between the two sets, then we end up
moving processes between some controllers and stranding them in the
source for the remaining controllers.
True. So I change it to move tasks under one controller, and leave all the
other controller unmodified. You can see it in my new patch. :)
As you know, different cgroups are independent to each other. So I think
operate on only one controller will make sense.
> + *next = '\0';
> + if ((rc = virCgrouAddTaskStr(dest_group, value) < 0))
> + goto cleanup;
> + value = next + 1;
> + }
> + if (*value != '\0') {
> + if ((rc = virCgrouAddTaskStr(dest_group, value) < 0))
Does it make sense to parse all the string into integers, just to format
the integers back into strings? Or would it be simpler to just cat the
contents of the 'tasks' file from the source into the destination
without bothering to interpret the date in transit?
I have tried this. But it seems that tasks file includes "\n", so it
won't work.
> + goto cleanup;
> + }
> +
> + VIR_FREE(content);
> + }
> +
> + return 0;
> +
> +cleanup:
> + virCgroupMoveTask(dest_group, src_group);
Is this cleanup always correct, or is it only correct if 'dest_group'
started life empty? Should we at least log a warning message if a move
was partially attempted and then reverted, particularly if the reversion
attempt failed?
The cleanup way is also changed, please refer to my new patches.
Thanks. :)
--
Best Regards,
Tang chen