On Mon, Jul 26, 2010 at 1:51 PM, Ivana Hutarova Varekova
<[email protected]> wrote:
> This patch fixes lssubsys output - if there was subsystem attached to
> not-mounted hierarchy then
> lssubsys -a
> does not show this subsystems
> ---
> EXAMPLE:
> BROKEN:
> $ ./lssubsys -am
> cpuset
> cpuacct /cgroup/memoryd
> memory /cgroup/memory
> freezer
> net_cls /cgroup/net_cls
>
> FIXED:
> $ ./lssubsys -am
> cpuset
> ns <- controller is in
> hierarchy but it is not mounted
> cpu,devices <- controller is in hierarchy
> but it is not mounted
> cpuacct /cgroup/memoryd
> memory /cgroup/memory
> freezer
> net_cls /cgroup/net_cls
>
>
>
> Signed-off-by: Ivana Hutarova Varekova <[email protected]>
Can you split up the different issues?
> ---
>
> src/tools/lssubsys.c | 146
> +++++++++++++++++++++++++++++++++++++++++++++-----
> 1 files changed, 130 insertions(+), 16 deletions(-)
>
> diff --git a/src/tools/lssubsys.c b/src/tools/lssubsys.c
> index 3e3a3dd..9ad2fb8 100644
> --- a/src/tools/lssubsys.c
> +++ b/src/tools/lssubsys.c
> @@ -36,11 +36,15 @@ static void usage(int status, const char *program_name)
> } else {
> fprintf(stdout, "Usage: %s [-m] [controller] [...]\n",
> program_name);
> - fprintf(stdout, "list all sybsystems of given controller\n");
> + fprintf(stdout, "Usage: %s -a [-m] \n",
> + program_name);
> + fprintf(stdout, "list all subsystems of given controller\n");
> + fprintf(stdout, "if no controller is set,");
> + fprintf(stdout, " list all mounted subsystems\n");
> fprintf(stdout, " -h, --help Display this help\n");
> fprintf(stdout, " -m, --mount-points Display mount
> points\n");
> fprintf(stdout, " -a, --all ");
> - fprintf(stdout, "Display all not mounted subsystems\n");
> + fprintf(stdout, "Display all controllers \n");
> }
> }
>
> @@ -55,6 +59,11 @@ static int print_controller(cont_name_t cont_name, int
> flags)
> int output = 0;
>
> ret = cgroup_get_controller_begin(&handle, &controller);
> + if (ret != 0) {
> + fprintf(stderr, "cannot read controller data: %s\n",
> + cgroup_strerror(ret));
> + return ret;
> + }
>
This fix in a separate patch please?
> path[0] = '\0';
> name[0] = '\0';
> @@ -163,28 +172,134 @@ static int cgroup_list_controllers(const char *tname,
> return final_ret;
> }
>
> -static int cgroup_list_all_controllers(const char *tname)
> +/* display all controllers attached to the given hierarchy */
> +static int print_all_controllers_in_hierarchy(const char *tname,
> + int hierarchy, int flags)
> {
> int ret = 0;
> void *handle;
> struct controller_data info;
> + int first = 1;
> + cont_name_t cont_name;
> + int init = 1;
> +
> + /* initialize libcgroup */
> + ret = cgroup_init();
> + if (ret)
> + /*
> + * if the group is not initialised we assume
> + * no mount points are available
> + */
> + init = 0;
>
nit, have the braces since the comment makes multiple lines. Easier to read.
> ret = cgroup_get_all_controller_begin(&handle, &info);
> + if (ret != 0) {
> + fprintf(stderr, "cannot read controller data: %s\n",
> + cgroup_strerror(ret));
> + return ret;
> + }
>
Again, this should be in a separate patch. Also what happens if you
get ECGEOF here. Might make sense to add a comment about that.
> while (ret != ECGEOF) {
> - if (info.hierarchy == 0)
> - printf("%s\n", info.name);
> + /* controller is in the hierrachy */
> + if (info.hierarchy == hierarchy) {
right, so in order to reduce nesting depth, can you make it
if (info.hierarchy != hierarchy)
goto some_label;
(sorry, this web interface is completely broken). But, yeah, something
similar to this?
> +
> + /* the first controller in the hierrachy*/
> + if (first) {
> + /*
> + * if mounted flag is set then
> + * test whether it is mounted
> + */
> + if ((flags & FL_MOUNT) && (init == 1)) {
> + strncpy(cont_name, info.name,
> + FILENAME_MAX);
FILENAME_MAX - 1 ?
> + cont_name[FILENAME_MAX-1] = '\0';
> +
it might be a better option to memset cont_name to '\0' and only copy
at most FILENAME_MAX - 1 bytes
> + ret = print_controller(cont_name,
> + flags + FL_LIST);
> + /*
> + * mount point was found,
> + * output is done
> + */
> + if (ret == 0) {
> + cgroup_get_all_controller_end(
> + &handle);
> + return 0;
> + }
> + }
> + printf("%s", info.name);
> + first = 0;
> + } else
> + printf(",%s", info.name);
> + }
> +
> ret = cgroup_get_all_controller_next(&handle, &info);
> if (ret && ret != ECGEOF) {
> fprintf(stderr,
> "%s: cgroup_get_controller_next failed (%s)\n",
> tname, cgroup_strerror(ret));
> + cgroup_get_all_controller_end(&handle);
> + return ret;
> + }
> + }
> +
> + cgroup_get_all_controller_end(&handle);
> + printf("\n");
> +
> + if (ret == ECGEOF)
> + ret = 0;
> +
> + return ret;
> +}
> +
> +/* go through the list of all controllers gather them based on hierarchy
> number
> + and print them */
> +static int cgroup_list_all_controllers(const char *tname, int flags)
> +{
> + int ret = 0;
> + void *handle;
> + struct controller_data info;
> +
> + int h_list[CG_CONTROLLER_MAX]; /* list of hierarchies */
> + int counter = 0;
> + int j;
> +
> + ret = cgroup_get_all_controller_begin(&handle, &info);
> +
> + while (ret != ECGEOF) {
> + if (info.hierarchy == 0) {
> + /* the controller is not attached to any hierrachy */
> + printf("%s\n", info.name);
> + } else {
> + /* the controller is attached to some hierarchy */
> + h_list[counter] = info.hierarchy;
> + counter++;
> + for (j = 0; j < counter-1; j++) {
> + /*
> + * the hierarchy already was on the list
> + * so remove the new record
> + */
> + if (h_list[j] == info.hierarchy) {
> + counter--;
> + break;
> + }
> + }
> + }
> +
> + ret = cgroup_get_all_controller_next(&handle, &info);
> + if (ret && ret != ECGEOF) {
> + fprintf(stderr,
> + "cgroup_get_controller_next failed (%s)\n",
> + cgroup_strerror(ret));
> return ret;
> }
> }
>
> ret = cgroup_get_all_controller_end(&handle);
>
> + for (j = 0; j < counter; j++)
> + ret = print_all_controllers_in_hierarchy(tname,
> + h_list[j], flags);
> +
> return ret;
>
> }
> @@ -231,28 +346,27 @@ int main(int argc, char *argv[])
>
> /* read the list of controllers */
> while (optind < argc) {
> -
> + if (flags & FL_ALL) {
> + fprintf(stderr, "Warning: too many parameters\n");
> + break;
> + }
> flags |= FL_LIST;
> strncpy(cont_name[c_number], argv[optind], FILENAME_MAX);
> cont_name[c_number][FILENAME_MAX-1] = '\0';
> c_number++;
> optind++;
> if (optind == CG_CONTROLLER_MAX) {
> - fprintf(stderr, "too much parameters\n");
> + fprintf(stderr, "Warning: too many parameters\n");
> break;
> }
> }
>
> - /*
> - * print the information
> - * based on list of input controllers and flags
> - */
> - ret = cgroup_list_controllers(argv[0], cont_name, flags);
> - if (ret)
> - return ret;
> -
> if (flags & FL_ALL)
> - ret = cgroup_list_all_controllers(argv[0]);
> + /* print the information about all controllers */
> + ret = cgroup_list_all_controllers(argv[0], flags);
> + else
> + /* print information about mounted controllers */
> + ret = cgroup_list_controllers(argv[0], cont_name, flags);
>
> return ret;
> }
looks fine otherwise, will try to give it a run sometime later on in the day!
Thanks,
Dhaval
------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://ad.doubleclick.net/clk;226879339;13503038;l?
http://clk.atdmt.com/CRS/go/247765532/direct/01/
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
