[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3 3/4] net/announce: Add optional ID |
Date: |
Wed, 12 Jun 2019 16:32:40 +0100 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
* 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