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