[libvirt] [PATCH 5/6] qemu: setup infrastructure to handle NIC_RX_FILTER_CHANGED event

[John Ferlan]

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