[libvirt] [PATCH] Switch to a more extensible annotation system for RPC protocols

[Daniel P. Berrange]

On Wed, Apr 17, 2013 at 10:16:08AM -0600, Eric Blake wrote:
> On 04/17/2013 09:43 AM, Daniel P. Berrange wrote:
> > From: "Daniel P. Berrange" <berrange at redhat.com>
> > 
> > Currently the RPC protocol files can contain annotations after
> > the protocol enum eg
> > 
> >    REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247, /* autogen autogen priority:high */
> > 
> > This is not very extensible as the number of annotations grows.
> 
> Hmm - wonder if that means you plan on adding annotations soon :)

You're not wrong !  I'll be adding annotations to describe the
access control rules for each API call, so that we can auto-generate
methods for doing access control checks in each driver.

> > Change it to use
> > 
> >     /**
> >      * @generate: both
> >      * @priority: high
> >      */
> >    REMOTE_PROC_DOMAIN_SNAPSHOT_LIST_CHILDREN_NAMES = 247,
> 
> At which point grouping by 10 no longer matters.

I guess, I was in two minds as to whether to remove that grouping
or not.


> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, /* skipgen skipgen */
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, /* skipgen skipgen */
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, /* skipgen skipgen */
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, /* skipgen skipgen */
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, /* skipgen skipgen */
> > +    /**
> > +     * @generate: none
> > +     */
> > +    VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1,
> > +    /**
> 
> I think it would be nicer to put a blank line before the /** of each
> listing, so that it is even more visually obvious that a comment applies
> to the enum below it.

Yeah, a good idea.

> 
> > +     * @generate: none
> > +     */
> > +    VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2,
> > +    /**
> > +     * @generate: none
> > +     */
> > +    VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3,
> > +    /**
> > +     * @generate: none
> > +     */
> > +    VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4,
> > +    /**
> > +     * @generate: none
> > +     */
> > +    VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5,
> >  
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6, /* skipgen skipgen */
> > -    VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7, /* skipgen skipgen */
> > +    /**
> 
> Hmm, your conversion preserved existing blank lines, but if you take my
> advice of adding a blank before every /**, I don't see any point in
> having double blanks before multiples of 5 or 10.

Ok.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|