Re: [PATCH v2 3/6] qemu-img: Add bitmap sub-command

[Eric Blake]

On 5/4/20 5:01 AM, Max Reitz wrote:

> > > > +.. option:: bitmap {--add [-g GRANULARITY] [--disabled] | --remove |
> > > > --clear | --enable | --disable | --merge SOURCE_BITMAP [-b
> > > > SOURCE_FILE [-F SOURCE_FMT]]} [--object OBJECTDEF] [--image-opts] [-f
> > > > FMT] FILENAME BITMAP
> > >
> > > So I can do multiple operations in one roll, but they all use the same
> > > BITMAP?  Sounds a bit weird.  It actually took me a while to understands
> > > this, because I thought for sure that each command would take a bitmap
> > > name.  (And was ready to complain that it looked like they don’t, but,
> > > well, that’s because they don’t.)
> >
> > All of the operations take one bitmap name (the final BITMAP).
> > Additionally, the --merge operation takes a second bitmap name
> > (SOURCE_BITMAP).  None of the other operations need a second bitmap
> > name, so only --merge requires an option argument.  As written, the { a
> > | b | c } implies that operations are mutually exclusive: you can only
> > request one operation per qemu-img invocation.
>
> Well, as I found out later it’s supposed to imply that.  I always expect
> {} to mean repetition.


> > In command line syntax, I'm most used to seeing repetition as '...',
> > optional as [], and mutually-exclusive choice as {|}.  Yes, that's
> > different than EBNF.
>
> It’s confusing is what it is, and unnecessarily so.  The | already
> signifies exclusion: Say there are -a and -b, if they’re not mutually
> exclusive, then the doc describe them as “[-a] [-b]”; if they are, it’d
> be “-a | -b”.  Maybe “(-a | -b)”.

Is s/{/(/ is all the more that would be needed to take this patch as-is? 
 Or should I really try and re-write it to take a list of operations, 
and iterate through the list until the first failure?

(At any rate, I'll probably end up trying that anyways, just for 
comparison in the lines of code...)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org