[lxc-devel] [PATCH] lxc-ls: Update lxc.group handling
Serge Hallyn
serge.hallyn at ubuntu.com
Fri May 2 16:06:22 UTC 2014
Quoting Stéphane Graber (stgraber at ubuntu.com):
> On Fri, May 02, 2014 at 01:18:32PM +0000, Serge Hallyn wrote:
> > Quoting Stéphane Graber (stgraber at ubuntu.com):
> > > This introduces a new -g/--group argument to filter containers based on
> > > their groups.
> > >
> > > This supports the rather obvious: --group blah
> > > Which will only list containers that are in group blah.
> > >
> > > It may also be passed multiple times: --group blah --group bleh
> > > Which will list containers that are in either (or both) blah or bleh.
> > >
> > > And it also takes: --group blah,bleh --group doh
> > > Which will list containers that are either in BOTH blah and bleh or in doh.
> > >
> > > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> >
> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> >
> > just one comment below,
> >
> > > ---
> > > doc/lxc-ls.sgml.in | 13 +++++++++++++
> > > src/lxc/lxc-ls.in | 39 +++++++++++++++++++++++++++++++++------
> > > 2 files changed, 46 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/doc/lxc-ls.sgml.in b/doc/lxc-ls.sgml.in
> > > index 1c40d33..20e31b2 100644
> > > --- a/doc/lxc-ls.sgml.in
> > > +++ b/doc/lxc-ls.sgml.in
> > > @@ -56,6 +56,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > <arg choice="opt">--stopped</arg>
> > > <arg choice="opt">-f</arg>
> > > <arg choice="opt">-F <replaceable>format</replaceable></arg>
> > > + <arg choice="opt">-g <replaceable>groups</replaceable></arg>
> > > <arg choice="opt">--nesting</arg>
> > > <arg choice="opt">filter</arg>
> > > </cmdsynopsis>
> > > @@ -152,6 +153,18 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > >
> > > <varlistentry>
> > > <term>
> > > + <option><optional>-g, --groups <replaceable>groups</replaceable></optional></option>
> > > + </term>
> > > + <listitem>
> > > + <para>
> > > + Comma separated list of groups the container must have to be displayed.
> > > + The parameter may be passed multiple times.
> > > + </para>
> > > + </listitem>
> > > + </varlistentry>
> > > +
> > > + <varlistentry>
> > > + <term>
> > > <option><optional>--nesting</optional></option>
> > > </term>
> > > <listitem>
> > > diff --git a/src/lxc/lxc-ls.in b/src/lxc/lxc-ls.in
> > > index fa387b3..e3df039 100755
> > > --- a/src/lxc/lxc-ls.in
> > > +++ b/src/lxc/lxc-ls.in
> > > @@ -103,8 +103,8 @@ def get_root_path(path):
> > >
> > > # Constants
> > > FIELDS = ("name", "state", "ipv4", "ipv6", "autostart", "pid",
> > > - "memory", "ram", "swap")
> > > -DEFAULT_FIELDS = ("name", "state", "ipv4", "ipv6", "autostart")
> > > + "memory", "ram", "swap", "groups")
> > > +DEFAULT_FIELDS = ("name", "state", "ipv4", "ipv6", "groups", "autostart")
> > >
> > > # Begin parsing the command line
> > > parser = argparse.ArgumentParser(description=_("LXC: List containers"),
> > > @@ -141,6 +141,11 @@ parser.add_argument("-F", "--fancy-format", type=str,
> > > default=",".join(DEFAULT_FIELDS),
> > > help=_("comma separated list of fields to show"))
> > >
> > > +parser.add_argument("-g", "--groups", type=str, action="append",
> > > + metavar="GROUPS",
> > > + help=_("groups (comma separated) the container must "
> > > + "be a member of"))
> > > +
> > > parser.add_argument("--nesting", dest="nesting", action="store_true",
> > > help=_("show nested containers"))
> > >
> > > @@ -189,6 +194,10 @@ if args.nesting:
> > > parser.error(_("Showing nested containers requires setns to the "
> > > "PID namespace which your kernel doesn't support."))
> > >
> > > +## Check that -g is passed alongside -f
> > > +if args.groups and not args.fancy:
> > > + parser.error(_("Group filtering requires fancy formatting."))
> > > +
> > > # Set the actual lxcpath value
> > > if not args.lxcpath:
> > > args.lxcpath = lxc.default_config_path
> > > @@ -229,6 +238,19 @@ def get_containers(fd=None, base="/", root=False):
> > > except:
> > > continue
> > >
> > > + if args.groups:
> > > + try:
> > > + set_has = set(container.get_config_item("lxc.group"))
> > > + except KeyError:
> > > + set_has = set()
> > > +
> > > + for group in args.groups:
> > > + set_must = set(group.split(","))
> > > + if not set_must - set_has:
> > > + break
> > > + else:
> > > + continue
> > > +
> > > if container.controllable:
> > > state = container.state
> > > else:
> > > @@ -254,15 +276,20 @@ def get_containers(fd=None, base="/", root=False):
> > > elif container.init_pid != -1:
> > > entry['pid'] = str(container.init_pid)
> > >
> > > + if 'groups' in args.fancy_format:
> > > + entry['groups'] = "-"
> > > + try:
> > > + groups = container.get_config_item("lxc.group")
> > > + if len(groups) > 0:
> > > + entry['groups'] = ", ".join(groups)
> > > + except KeyError:
> > > + pass
> > > +
> > > if 'autostart' in args.fancy_format:
> > > entry['autostart'] = "NO"
> > > try:
> > > if container.get_config_item("lxc.start.auto") == "1":
> > > entry['autostart'] = "YES"
> > > -
> > > - groups = container.get_config_item("lxc.group")
> > > - if len(groups) > 0:
> > > - entry['autostart'] = "YES (%s)" % ", ".join(groups)
> >
> > If a container is in group blah and autostart, you'll now just
> > say 'autostart = yes', which *could* be misleading. Since this
> > behavior is rather subtle (you've had to explain it many times)
> > I think this might be worth keeping.
>
> This would be pretty redundant as the column right on the left of
> AUTOSTART is GROUP which happens to show exactly what used to be right
> next to the YES in the AUTOSTART column.
>
> So the new output is:
> NAME STATE IPV4 IPV6 GROUPS AUTOSTART
> -----------------------------------------------------
> network STOPPED - - b, c YES
> nsec-internet STOPPED - - a, d YES
> precise-gui STOPPED - - a, b YES
>
> What you suggest would show:
> NAME STATE IPV4 IPV6 GROUPS AUTOSTART
> -----------------------------------------------------
> network STOPPED - - b, c YES (b, c)
> nsec-internet STOPPED - - a, d YES (a, d)
> precise-gui STOPPED - - a, b YES (a, b)
>
>
> So just adding duplication without any real value that I can see.
Agreed, so how about showing autostart='GROUP' in those
cases, and autostart='YES' only if no groups are specified?
More information about the lxc-devel
mailing list