Re: [Qemu-devel] [PATCH 2/3] monitor: throttle QAPI_EVENT_VSERPORT_CHANGE by "id"

[Marc-André Lureau]

Hi

On Wed, Sep 16, 2015 at 6:40 PM, Markus Armbruster <address@hidden> wrote:
>
> I got a few more questions.
>
> Do we expect more events in need of separate throttling by some ID-like
> member of their data object?
> Do we expect *different* callbacks?  Callbacks that throttle by some
> member of their data object won't count as different.
>
> Do we really need the callback and the type punning?  As long as all we
> have is the current two throttles, I'd try to do without, as follows.
>

That's not a question you can answer easily, but I can easily imagine
that we may throttle events based on different conditions. So I prefer
a modular code if it's easy to do and hopefully easy to understand.

> monitor_qapi_event_throttle() takes the name of a data member as
> additional argument.  If null, throttle like monitor_qapi_event_delay().
> Else, throttle like monitor_qapi_event_id_delay().  Except do it in a
> single function, with a single MonitorQAPIEventState.  The natural
> representation of "either a single MonitorQAPIEventPending or a hash
> table of them" is a union.

You need a dispatch in a single function then, and it's very much not
modular code that way. I'd prefer we keep the modularity this patch
brings: different functions and structure for different throttle
implementations.

-- 
Marc-André Lureau