Re: [libvirt] [PATCH 5/6] qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event
On 09/24/2014 05:50 AM, Laine Stump wrote:
NIC_RX_FILTER_CHANGED is sent by qemu any time a NIC driver in the
guest modified the NIC's RX Filter (for example, if the MAC address of
the NIC is changed by the guest).
This patch doesn't do anything useful with that event; it just sets up
all the plumbing to get news of the event into a worker thread with
all proper locking/reference counting, and provide an easy place to
add in desired functionality.
For easy reference the next time a handler for a qemu event is
written, here is the sequence:
The handler in qemu_json_monitor.c calls
qemu_json_monitor.c:qemuMonitorJSONHandleNicRxFilterChanged()
which calls
qemu_monitor.c:qemuMonitorEmitNicRxFilterChanged()
which uses QEMU_MONITOR_CALLBACK() to call
mon->cb->domainNicRxFilterChanged(), ie:
qemuProcessHandleNicRxFilterChanged()
which will queue an event QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED for
a worker thread to handle.
When the worker thread gets the event, it calls
qemuProcessEventHandler()
which calls
processNicRxFilterChangedEvent()
and *that* is where the actual work will be done (and any
event-specific memory allocated during qemuProcessHandleXXX() will be
freed) - it is the middle of this function where functionality behind
the event will be added in the next patch; for now there is just a
VIR_DEBUG() to log the event.
This is the kind of stuff while great in a commit message - is perhaps
even better in the comments of the code somewhere. Not sure of which is
the "best place", but perhaps before the typedef enum {}
qemuProcessEventType; At least someone will get a head start on the
various places they have to change. Anyone adding an event has to start
there.
---
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor.c | 13 +++++++++++
src/qemu/qemu_monitor.h | 7 ++++++
src/qemu/qemu_monitor_json.c | 17 ++++++++++++++
src/qemu/qemu_process.c | 42 +++++++++++++++++++++++++++++++++
6 files changed, 135 insertions(+)
Although not my area of expertise, things look good. Looks like all the
changes were very similar to other events added. Not the definitive ACK
you probably want, but hopefully someone else who's made changes in this
space recently can take a quick look to make sure you've covered everything.
My one question would be - is there any sort of issue if when
registering the event that the underlying qemu doesn't support/have the
NIC_RX_FILTER_CHANGED event defined? Or does that just not matter and
all this does is take events sent to libvirt after registering at some
higher level to receive "any" event. It seems to be the latter through
qemuMonitorIO, but I figured I'd ask...
John