----- Original Message -----
> Hello Dave,
>
> I have been working on a new command named lscgroup to
> list cgroups' information and finished it recently.
>
> This command displays all the cgroups' information or the
> appointed cgroups' information relatively according to user's
> input. The following shows the two types of output.
>
> 1.Display all the cgroups' information of the system.
> crash> lscgroup
> blkio:/
> blkio:/libvirt
> blkio:/libvirt/lxc
> blkio:/libvirt/qemu
> net_cls:/
> ...
> ...
> cpuset:/
> cpuset:/libvirt
> cpuset:/libvirt/lxc
> cpuset:/libvirt/qemu
>
>
> 2.Display the appointed cgroups' information.
> crash> lscgroup memory:/ cpuset:/libvirt
> memory:/
> memory:/libvirt
> memory:/libvirt/lxc
> memory:/libvirt/qemu
> cpuset:/libvirt
> cpuset:/libvirt/lxc
> cpuset:/libvirt/qemu
>
> To see more details, please refer to the help information and the patch.
>
> To build the module from the top-level crash-<version> directory,
> enter:
> $ cp <path-to>/lscgroup.c extensions
> $ make extensions
Thanks for diving into this kernel subsystem. Given the emergence
of cgroups, this command could ultimately be considered as a new
built-in command.
However, that being said, it does need some additional work.
First, for older kernels that don't have cgroups, there should be a
protection mechanism to prevent this from happening:
crash> sys | grep RELEASE
RELEASE: 2.6.18-146.el5
crash> lscgroup
lscgroup: invalid structure member offset: cgroupfs_root_root_list
FILE: lscgroup.c LINE: 503 FUNCTION: cgroup_list_cgroups()
[/usr/bin/crash] error trace: 4611e1 => 7f3b8f3a19d7 => 7f3b8f3a14e6 => 50b701
50b701: OFFSET_verify+161
4611e1: exec_command+801
lscgroup: invalid structure member offset: cgroupfs_root_root_list
FILE: lscgroup.c LINE: 503 FUNCTION: cgroup_list_cgroups()
crash>
You should check for the existence of a kernel variable or structure definition,
and if they don't exist, call command_not_supported().
Secondly, if I am not mistaken, your command only shows "enabled" cgroups?
So for example, on this RHEL6 system, the command quietly does nothing:
crash> sys | grep RELEASE
RELEASE: 2.6.32-15.el6.x86_64
crash> lscgroup
crash>
Is that the case? I wonder if you should also show cgroups that are
not enabled, i.e., like /proc/cgroups does? Or at a minimum, it should
show display something like "(no cgroups enabled)" instead of just
showing nothing at all.
Thirdly, your help page shows a "-h" option, which is unnecessary:
crash> help lscgroup
NAME
lscgroup - list all cgroups
SYNOPSIS
lscgroup lscgroup [<controllers>:<path>] [...][-h]
...
because it doesn't do anything other than show the SYNOPSIS line
from the "help lscgroup" output:
crash> lscgroup -h
Usage:
lscgroup lscgroup [<controllers>:<path>] [...][-h]
Enter "help lscgroup" for details.
crash>
And you should change the third line in help_lscgroup[] from:
char *help_lscgroup[] = {
"lscgroup",
"list all cgroups",
"lscgroup [<controllers>:<path>] [...][-h]",
to:
"[<controllers>:<path>] [...][-h]",
so that "lscgroup" is not printed twice in the "help lscgroup" and
the "Usage:" output.
And lastly, and most importantly, is what the command
does *not* show.
Consider that most crash commands that delve into kernel
subsystems typically show information intermingled with
key data structure addresses. So if there is a bug associated
with a kernel subsystem, the data structure addresses displayed
by a crash command can be used as a starting point for debugging
purposes.
So for example, if there was a bug associated with cgroups, your
command's output only shows the name strings, but there's nothing
else to "work with" for debugging. Wouldn't it be more helpful to
at least display the cgroup and/or the cgroupfs_root addresses?
Then there would be useful data structure addresses to start with.
Thanks,
Dave
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
