Re: [Qemu-devel] [PATCH 3/3] monitor: remove old entries from event hash table

[Markus Armbruster]

address@hidden writes:

> From: Marc-André Lureau <address@hidden>
>
> Do not let the hash table grow without limit, schedule a cleanup for
> outdated event.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  monitor.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/monitor.c b/monitor.c
> index 7f5c80f..8d0c61b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -540,6 +540,45 @@ monitor_qapi_event_pending_new(QAPIEvent event)
>      return p;
>  }
>  
> +static void monitor_qapi_event_id_remove(void *opaque)
> +{
> +    MonitorQAPIEventPending *p = opaque;
> +    MonitorQAPIEventState *s = &monitor_qapi_event_state[p->event];
> +    GHashTable *ht = s->data;
> +    GHashTableIter iter;
> +    gpointer value;
> +
> +    g_hash_table_iter_init(&iter, ht);
> +    while (g_hash_table_iter_next(&iter, NULL, &value)) {
> +        if (value == p) {
> +            g_hash_table_iter_remove(&iter);
> +            return;
> +        }
> +    }
> +}
> +
> +/*
> + * do not let the hash table grow, if no later pending event
> + * scheduled, remove the old entry after rate timeout.
> + */
> +static void monitor_qapi_event_id_schedule_remove(MonitorQAPIEventPending *p)
> +{
> +    MonitorQAPIEventState *s = &monitor_qapi_event_state[p->event];
> +    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
> +    int64_t then = now + s->rate;
> +
> +    p->timer->cb = monitor_qapi_event_id_remove;

I doubt you're supposed to poke into p->timer like that.

> +    timer_mod_ns(p->timer, then);
> +}
> +
> +static void monitor_qapi_event_id_handler(void *opaque)
> +{
> +    MonitorQAPIEventPending *p = opaque;
> +
> +    monitor_qapi_event_handler(p);
> +    monitor_qapi_event_id_schedule_remove(p);
> +}
> +
>  static bool
>  monitor_qapi_event_id_delay(MonitorQAPIEventState *evstate, QDict *data)
>  {
> @@ -554,7 +593,13 @@ monitor_qapi_event_id_delay(MonitorQAPIEventState 
> *evstate, QDict *data)
>          g_hash_table_insert(ht, g_strdup(id), p);
>      }
>  
> -    return monitor_qapi_event_pending_update(evstate, p, data);
> +    if (monitor_qapi_event_pending_update(evstate, p, data)) {
> +        p->timer->cb = monitor_qapi_event_id_handler;
> +        return true;
> +    } else {
> +        monitor_qapi_event_id_schedule_remove(p);
> +        return false;
> +    }
>  }
>  
>  /*

Possibly cleaner than messing with timer->cb: have a single callback
that does the right thing, i.e. when the timer goes off, check whether
we have an event.  If yes, send it and rearm the timer.  If no, remove
the hash entry.