Re: [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID

[Dr. David Alan Gilbert]

* Eric Blake (address@hidden) wrote:
> On 6/10/19 1:44 PM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> > 
> > Previously there was a single instance of the timer used by
> > monitor triggered announces, that's OK, but when combined with the
> > previous change that lets you have announces for subsets of interfaces
> > it's a bit restrictive if you want to do different things to different
> > interfaces.
> > 
> > Add an 'id' field to the announce, and maintain a list of the
> > timers based on id.
> > 
> > This allows you to for example:
> >     a) Start an announce going on interface eth0 for a long time
> >     b) Start an announce going on interface eth1 for a long time
> >     c) Kill the announce on eth0 while leaving eth1 going.
> > 
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > ---
> 
> > +++ b/include/net/announce.h
> > @@ -23,8 +23,12 @@ struct AnnounceTimer {
> >  /* Returns: update the timer to the next time point */
> >  int64_t qemu_announce_timer_step(AnnounceTimer *timer);
> >  
> > -/* Delete the underlying timer and other data */
> > -void qemu_announce_timer_del(AnnounceTimer *timer);
> > +/*
> > + * Delete the underlying timer and other datas
> 
> 'data' is already plural, 'datas' is not a word.

oops yes, fixed.

> > + * If 'free_named' true and the timer is a named timer, then remove
> > + * it from the list of named timers and free the AnnounceTimer itself.
> > + */
> > +void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named);
> >  
> 
> > +++ b/qapi/net.json
> > @@ -702,6 +702,10 @@
> >  # @interfaces: An optional list of interface names, which restrict the
> >  #        announcment to the listed interfaces. (Since 4.1)
> >  #
> > +# @id: A name to be used to identify an instance of announce-timers
> > +#        and to allow it to modified later.  Not for use as
> > +#        part of the migration paramters. (Since 4.1)
> 
> parameters

Fixed.

> > +#
> >  # Since: 4.0
> >  ##
> >  
> > @@ -710,7 +714,8 @@
> >              'max': 'int',
> >              'rounds': 'int',
> >              'step': 'int',
> > -            '*interfaces': ['str'] } }
> > +            '*interfaces': ['str'],
> > +            '*id' : 'str' } }
> >  
> >  ##
> >  # @announce-self:
> > @@ -725,7 +730,7 @@
> >  # -> { "execute": "announce-self",
> >  #      "arguments": {
> >  #          "initial": 50, "max": 550, "rounds": 10, "step": 50,
> > -#          "interfaces": ["vn2","vn3"] } }
> > +#          "interfaces": ["vn2","vn3"], "id": "bob" } }
> >  # <- { "return": {} }
> >  #
> 
> Worth an example of deleting a timer by id?
> 

The syntax is actually the same - the only thing you need to do
to cancel is set 'rounds' to 0 for the named id.

> >  # Since: 4.0
> > diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> > index 163126cf07..7184e2bff4 100644
> > --- a/tests/virtio-net-test.c
> > +++ b/tests/virtio-net-test.c
> > @@ -186,7 +186,7 @@ static void announce_self(void *obj, void *data, 
> > QGuestAllocator *t_alloc)
> >      rsp = qmp("{ 'execute' : 'announce-self', "
> >                    " 'arguments': {"
> >                        " 'initial': 50, 'max': 550,"
> > -                      " 'rounds': 10, 'step': 50 } }");
> > +                      " 'rounds': 10, 'step': 50, 'id': 'bob' } }");
> 
> And here, is it worth testing that you can delete by id, rather than
> just create with an id?

OK, I'll have a look at how painful that is.

Dave

> >      assert(!qdict_haskey(rsp, "error"));
> >      qobject_unref(rsp);
> >  
> > 
> 
> -- 
> 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