Re: [PATCH v2 08/13] xenstore: modify init-xenstore-domain parameter syntax

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2016-01-07 at 07:34 +0100, Juergen Gross wrote:
> On 06/01/16 17:21, Ian Campbell wrote:
> > On Fri, 2015-12-18 at 14:14 +0100, Juergen Gross wrote:
> > > init-xenstore-domain takes only positional parameters today. Change
> > > this to a more flexible parameter syntax allowing to specify
> > > additional
> > > options or to omit some.
> > > 
> > > Today the supported usage is:
> > > 
> > > init-xenstore-domain <xenstore-kernel> <memory_mb> <flask-label>
> > >                      [<ramdisk-file>]
> > > 
> > > Modify this to:
> > > 
> > > init-xenstore-domain --kernel <xenstore-kernel>
> > >                      --memory <memory_mb>
> > >                      [--flask <flask-label>]
> > >                      [--ramdisk <ramdisk-file>]
> > > 
> > > The flask label is made optional in order to support xenstore domains
> > > without the need of a flask enabled hypervisor.
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > 
> > I think Daniel and I both acked this patch in v1 (as #5/9). Is this
> > version
> > different? If so please include an intra-version changelog after the "-
> > --"
> > to help us out.
> 
> There was an error in parameter parsing (omitting kernel or memory
> wasn't detected). It was mentioned in the changelog in the cover letter.

In general we prefer these sorts of details to be mentioned in the patch
themselves, after a "---" marker such that they don't get included in the
actual commit. The cover letter should be more of a summary.

> I'll add a note next time when I drop an Ack.

Thanks.

WRT that change and:

> +    if (optind != argc || !kernel || !memory) {
> +        usage();
>          return 2;
>      }

Under what circumstances can optind != argc? AFAICT it should never happen
because the switch cover all valid options and returns otherwise. If it can
never happen it should be an assert.

Oh, is it to catch things like "--foo bar baz" i.e. additional non-option
arguments (of which there should be none)?

If that is the case: Acked-by: Ian Campbell <ian.campbell@citrix.com>

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel