qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [RFC 0/6] Throttle event VSERPORT_CHANGE separately by "id"


From: Markus Armbruster
Subject: [Qemu-devel] [RFC 0/6] Throttle event VSERPORT_CHANGE separately by "id"
Date: Fri, 25 Sep 2015 16:00:41 +0200

VSERPORT_CHANGE is emitted when the guest opens or closes a
virtio-serial port.  The event's member "id" identifies the port.

When several events arrive quickly, throttling drops all but the last
of them.  Because of that, a QMP client must assume that *any* port
may have changed state when it receives a VSERPORT_CHANGE event and
throttling may have happened.

Make the event more useful by throttling it for each port separately.

Marc-André recently posted 'monitor: throttle VSERPORT_CHANGED by
"id"' to do the same thing.  Why am I redoing his work?  Let me
explain.

In master, event throttling works per QAPIEvent.  Throttling state is
kept in MonitorQAPIEventState monitor_qapi_event_state[].  Going from
event to throttling state is a simple array subscript.

Marc-André's series makes the code governing throttling pluggable per
event.  MonitorQAPIEventState gets two new members, the governor
function and an opaque.  It loses the actual throttling state.  That
one's now in new type MonitorQAPIEventPending.

The standard governor has the opaque point to a single
MonitorQAPIEventPending.

The governor for VSERPORT_CHANGED has the opaque point to a hash table
mapping "id" to MonitorQAPIEventPending.

In my review, I challenged the complexity of this solution, but
Marc-André thinks it's justified.  That leaves me two choices: concede
the point, or come up with something that's actually simpler.

My solution doesn't make anything pluggable.  Instead, I simply
generalize the map from event to throttling state.  Instead of using
just the QAPIEvent as key for looking up state, I permit additional
use of event-specific data.  For VSERPORT_CHANGED, I additionally use
"id".  monitor_qapi_event_state[] becomes a hash table.

No callbacks, no type-punning, less code, and fewer code paths to
test.

RFC because it's compile-tested only.  Compile-tested only because I
want to give my feedback to Marc-André's work as soon as I can
(although "as soon as I can" still means "frustratingly late", I'm
afraid).

PATCH 1+2 clean up the code some before I start.  PATCH 2 may fix
bugs, but to be sure I'd have to think some more about the old code.

PATCH 3-5 do the actual work.

PATCH 6 fixes docs to mention throttling.

Diffstat for monitor.c
    Marc-André's series: 201 insertions(+), 55 deletions(-)
    mine:                109 insertions(+), 77 deletions(-)

Markus Armbruster (6):
  monitor: Reduce casting of QAPI event QDict
  monitor: Simplify event throttling
  monitor: Split MonitorQAPIEventConf off MonitorQAPIEventState
  monitor: Turn monitor_qapi_event_state[] into a hash table
  monitor: Throttle event VSERPORT_CHANGE separately by "id"
  docs: Document QMP event rate limiting

 docs/qmp/qmp-events.txt |  12 ++++
 docs/qmp/qmp-spec.txt   |   5 ++
 monitor.c               | 186 ++++++++++++++++++++++++++++--------------------
 3 files changed, 126 insertions(+), 77 deletions(-)

-- 
2.4.3




reply via email to

[Prev in Thread] Current Thread [Next in Thread]