Re: [Qemu-devel] [PATCH v3 1/4] net/announce: Allow optional list of interfaces

[Dr. David Alan Gilbert]

* Eric Blake (address@hidden) wrote:
> On 6/10/19 1:43 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > Allow the caller to restrict the set of interfaces that announces are
> > sent on.  The default is still to send on all interfaces.
> > 
> > e.g.
> > 
> >   { "execute": "announce-self", "arguments": { "initial": 50, "max": 550, 
> > "rounds": 5, "step": 50, "interfaces": ["vn2","vn1"] } }
> > 
> > This doesn't affect the behaviour of migraiton announcments.
> > 
> > Note: There's still only one timer for the qmp command, so that
> > performing an 'announce-self' on one list of interfaces followed
> > by another 'announce-self' on another list will stop the announces
> > on the existing set.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> 
> > +++ b/qapi/net.json
> > @@ -699,6 +699,9 @@
> >  #
> >  # @step: Delay increase (in ms) after each self-announcement attempt
> >  #
> > +# @interfaces: An optional list of interface names, which restrict the
> 
> restricts

Done

> > +#        announcment to the listed interfaces. (Since 4.1)
> 
> announcement

Done

> > +#
> >  # Since: 4.0
> >  ##
> >  
> > @@ -706,7 +709,8 @@
> >    'data': { 'initial': 'int',
> >              'max': 'int',
> >              'rounds': 'int',
> > -            'step': 'int' } }
> > +            'step': 'int',
> > +            '*interfaces': ['str'] } }
> >  
> >  ##
> >  # @announce-self:
> > @@ -718,9 +722,10 @@
> >  #
> >  # Example:
> >  #
> > -# -> { "execute": "announce-self"
> > +# -> { "execute": "announce-self",
> 
> Embarrassing that we didn't notice that one earlier.

The way to avoid it I guess would be to parse the example code.

> >  #      "arguments": {
> > -#          "initial": 50, "max": 550, "rounds": 10, "step": 50 } }
> > +#          "initial": 50, "max": 550, "rounds": 10, "step": 50,
> > +#          "interfaces": ["vn2","vn3"] } }
> 
> Worth a space after the comma? Not required, but I think it looks nicer.

Added

> As I only focused on doc issues, I'll leave the full review to others.

Thanks,

Dave

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



--
Dr. David Alan Gilbert / address@hidden / Manchester, UK