[sheepdog] [PATCH v2] dog: permit two or more depth of subcommands

[Hitoshi Mitake]

At Wed, 13 Nov 2013 14:15:19 +0800,
Liu Yuan wrote:
> 
> On Wed, Nov 13, 2013 at 02:53:27PM +0900, Hitoshi Mitake wrote:
> > At Wed, 13 Nov 2013 10:53:15 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Wed, Nov 13, 2013 at 12:22:31AM +0900, Hitoshi Mitake wrote:
> > > > At Tue, 12 Nov 2013 15:38:17 +0800,
> > > > Liu Yuan wrote:
> > > > > 
> > > > > On Tue, Nov 12, 2013 at 04:33:49PM +0900, Hitoshi Mitake wrote:
> > > > > > At Mon, 11 Nov 2013 21:24:47 +0800,
> > > > > > Liu Yuan wrote:
> > > > > > > 
> > > > > > > On Mon, Nov 11, 2013 at 10:16:57PM +0900, Hitoshi Mitake wrote:
> > > > > > > > From: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > > > > > > 
> > > > > > > > Current dog command dies when user invokes "dog node log level set"
> > > > > > > > without any arguments because of its subcommand design. This patch
> > > > > > > > removes this limitation.
> > > > > > > > 
> > > > > > > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > v2: define do_generic_subcommand() as a macro for a common case (depth == 0)
> > > > > > > > 
> > > > > > > >  dog/common.c | 5 +++--
> > > > > > > >  dog/dog.h    | 5 ++++-
> > > > > > > >  dog/node.c   | 3 ++-
> > > > > > > >  3 files changed, 9 insertions(+), 4 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/dog/common.c b/dog/common.c
> > > > > > > > index 4480b99..90b43ad 100644
> > > > > > > > --- a/dog/common.c
> > > > > > > > +++ b/dog/common.c
> > > > > > > > @@ -224,7 +224,8 @@ int send_light_req(const struct node_id *nid, struct sd_req *hdr)
> > > > > > > >  	return 0;
> > > > > > > >  }
> > > > > > > >  
> > > > > > > > -int do_generic_subcommand(struct subcommand *sub, int argc, char **argv)
> > > > > > > > +int do_generic_subcommand(struct subcommand *sub, int depth,
> > > > > > > > +			  int argc, char **argv)
> > > > > > > >  {
> > > > > > > 
> > > > > > > Why we need 'int depth'? I don't think we need depth since we don't know where
> > > > > > > is the horizon. What I meant in my last reply is just make 5 as a bigger one, like
> > > > > > > 
> > > > > > > #define DOG_MAX_PARAMETERS 10
> > > > > > 
> > > > > > What does the word "horizon" mean in this context? I think we need the
> > > > > > depth variable for detecting the beginning of arguments for
> > > > > > subcommands.
> > > > > >
> > > > > 
> > > > > I meant 'beginning of arguements for subcommands'. But boundary of subcommands
> > > > > depends on context. E.g, 'dog vdi log level list', vdi is subcommand of dog,
> > > > > log is subcommand of vdi and so on. So I think we only need a marco to define
> > > > > the max allowed subcommdans. An extra parameter isn't necessary to me.
> > > > 
> > > > Do you mean we should replace the line:
> > > > 	if (flags & CMD_NEED_ARG && argc < 5)
> > > > with a line like this:
> > > > 	if (flags & CMD_NEED_ARG && argc < DOG_MAX_PARAMETERS)
> > > > ?
> > > 
> > > Yes
> > > 
> > > > If we do so, ordinal subcommands cannot work well.
> > > 
> > > So what is the problem?
> > 
> > If we changes do_generic_subcommand() in the above way, existing
> > subcommands like "dog vdi cache flush <vdi>" cannot work well.
> 
> What exactly do you mean by 'not work well'?

Produces an error message like below even if the argument <vdiname> is passed:

Usage: dog vdi cache {flush|delete|info|purge} [-s snapshot] [-a address] [-p port] [-h] <vdiname>
Available subcommands:
  flush                   flush the cache of the vdi specified.
  delete                  delete the cache of the vdi specified in all nodes.
  info                    show usage of the cache
  purge                   purge the cache of all vdi (no flush)
Options:
  -s, --snapshot          specify a snapshot id or tag name
  -a, --address           specify the daemon address (default: localhost)
  -p, --port              specify the daemon port
  -h, --help              display this help and exit

Thanks,
Hitoshi