* Jason Wang (jasow...@redhat.com) wrote:
>
> On 2019/6/13 下午5:59, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >
> > 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 <dgilb...@redhat.com>
> > ---
> > hw/net/virtio-net.c | 4 ++--
> > include/net/announce.h | 8 ++++++--
> > net/announce.c | 46 +++++++++++++++++++++++++++++++++++-------
> > net/trace-events | 3 ++-
> > qapi/net.json | 9 +++++++--
> > 5 files changed, 56 insertions(+), 14 deletions(-)
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index c3f5fccfd1..b9e1cd71cf 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -2360,7 +2360,7 @@ static int virtio_net_post_load_device(void *opaque,
> > int version_id)
> > timer_mod(n->announce_timer.tm,
> > qemu_clock_get_ms(n->announce_timer.type));
> > } else {
> > - qemu_announce_timer_del(&n->announce_timer);
> > + qemu_announce_timer_del(&n->announce_timer, false);
> > }
> > }
> > @@ -2784,7 +2784,7 @@ static void virtio_net_device_unrealize(DeviceState
> > *dev, Error **errp)
> > virtio_net_del_queue(n, i);
> > }
> > - qemu_announce_timer_del(&n->announce_timer);
> > + qemu_announce_timer_del(&n->announce_timer, false);
> > g_free(n->vqs);
> > qemu_del_nic(n->nic);
> > virtio_net_rsc_cleanup(n);
> > diff --git a/include/net/announce.h b/include/net/announce.h
> > index 773470428b..3d90c83c23 100644
> > --- a/include/net/announce.h
> > +++ b/include/net/announce.h
> > @@ -22,8 +22,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 data
> > + * 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);
> > /*
> > * Under BQL/main thread
> > diff --git a/net/announce.c b/net/announce.c
> > index 1ce42b571d..4d4e2c22a1 100644
> > --- a/net/announce.c
> > +++ b/net/announce.c
> > @@ -15,6 +15,8 @@
> > #include "qapi/qapi-commands-net.h"
> > #include "trace.h"
> > +static GData *named_timers;
> > +
> > int64_t qemu_announce_timer_step(AnnounceTimer *timer)
> > {
> > int64_t step;
> > @@ -31,8 +33,13 @@ int64_t qemu_announce_timer_step(AnnounceTimer *timer)
> > return step;
> > }
> > -void qemu_announce_timer_del(AnnounceTimer *timer)
> > +/*
> > + * If 'free_named' is true, then remove the timer from the list
> > + * and free the timer itself.
> > + */
> > +void qemu_announce_timer_del(AnnounceTimer *timer, bool free_named)
> > {
> > + bool free_timer = false;
> > if (timer->tm) {
> > timer_del(timer->tm);
> > timer_free(timer->tm);
> > @@ -40,6 +47,18 @@ void qemu_announce_timer_del(AnnounceTimer *timer)
> > }
> > qapi_free_strList(timer->params.interfaces);
> > timer->params.interfaces = NULL;
> > + if (free_named && timer->params.has_id) {
> > + free_timer = timer ==
> > + g_datalist_get_data(&named_timers, timer->params.id);
>
>
> Any chance that the timer get from datalist is different from the one that
> caller passed to us?
No, I've replaced this with:
+ if (free_named && timer->params.has_id) {
+ AnnounceTimer *listTimer;
+ /*
+ * Sanity check: There should only be one timer on the list with
+ * the id.
+ */
+ list_timer = g_datalist_get_data(&named_timers, timer->params.id);
+ assert(timer == list_timer);
+ free_timer = true;
+ g_datalist_remove_data(&named_timers, timer->params.id);
+ }
> Thanks
>
>
> > + g_datalist_remove_data(&named_timers, timer->params.id);
> > + }
> > + trace_qemu_announce_timer_del(free_named, free_timer,
> > timer->params.id);
> > + g_free(timer->params.id);
> > + timer->params.id = NULL;
> > +
> > + if (free_timer) {
> > + g_free(timer);
> > + }
> > }
> > /*
> > @@ -56,7 +75,7 @@ void qemu_announce_timer_reset(AnnounceTimer *timer,
> > * We're under the BQL, so the current timer can't
> > * be firing, so we should be able to delete it.
> > */
> > - qemu_announce_timer_del(timer);
> > + qemu_announce_timer_del(timer, false);
> > QAPI_CLONE_MEMBERS(AnnounceParameters, &timer->params, params);
> > timer->round = params->rounds;
> > @@ -120,7 +139,8 @@ static void qemu_announce_self_iter(NICState *nic, void
> > *opaque)
> > skip = false;
> > }
> > - trace_qemu_announce_self_iter(nic->ncs->name,
> > + trace_qemu_announce_self_iter(timer->params.has_id ? timer->params.id
> > : "_",
> > + nic->ncs->name,
> > qemu_ether_ntoa(&nic->conf->macaddr),
> > skip);
> > if (!skip) {
> > @@ -143,7 +163,7 @@ static void qemu_announce_self_once(void *opaque)
> > if (--timer->round) {
> > qemu_announce_timer_step(timer);
> > } else {
> > - qemu_announce_timer_del(timer);
> > + qemu_announce_timer_del(timer, true);
> > }
> > }
> > @@ -154,12 +174,24 @@ void qemu_announce_self(AnnounceTimer *timer,
> > AnnounceParameters *params)
> > if (params->rounds) {
> > qemu_announce_self_once(timer);
> > } else {
> > - qemu_announce_timer_del(timer);
> > + qemu_announce_timer_del(timer, true);
> > }
> > }
> > void qmp_announce_self(AnnounceParameters *params, Error **errp)
> > {
> > - static AnnounceTimer announce_timer;
> > - qemu_announce_self(&announce_timer, params);
> > + AnnounceTimer *named_timer;
> > + if (!params->has_id) {
> > + params->id = g_strdup("");
> > + params->has_id = true;
> > + }
> > +
> > + named_timer = g_datalist_get_data(&named_timers, params->id);
> > +
> > + if (!named_timer) {
> > + named_timer = g_new0(AnnounceTimer, 1);
> > + g_datalist_set_data(&named_timers, params->id, named_timer);
> > + }
> > +
> > + qemu_announce_self(named_timer, params);
> > }
> > diff --git a/net/trace-events b/net/trace-events
> > index 875ef2a0f3..ac57056497 100644
> > --- a/net/trace-events
> > +++ b/net/trace-events
> > @@ -1,7 +1,8 @@
> > # See docs/devel/tracing.txt for syntax documentation.
> > # announce.c
> > -qemu_announce_self_iter(const char *name, const char *mac, int skip)
> > "%s:%s skip: %d"
> > +qemu_announce_self_iter(const char *id, const char *name, const char *mac,
> > int skip) "%s:%s:%s skip: %d"
> > +qemu_announce_timer_del(bool free_named, bool free_timer, char *id) "free
> > named: %d free timer: %d id: %s"
> > # vhost-user.c
> > vhost_user_event(const char *chr, int event) "chr: %s got event: %d"
> > diff --git a/qapi/net.json b/qapi/net.json
> > index 6f2cd4f530..728990f4fb 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -702,6 +702,10 @@
> > # @interfaces: An optional list of interface names, which restricts the
> > # announcement 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 parameters. (Since 4.1)
> > +#
> > # 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": {} }
> > #
> > # Since: 4.0
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
