On 05/29/2014 01:36 PM, Laszlo Ersek wrote: > Libvirt wants to know about the guest-side connection state of some > virtio-serial ports (in particular the one(s) assigned to guest agent(s)). > Introduce a new property that allows libvirt to request connection state > reporting, and report the state via new monitor events. > > RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1080376 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > include/monitor/monitor.h | 2 ++ > hw/char/virtio-console.c | 20 +++++++++++++++++--- > monitor.c | 2 ++ > docs/qmp/qmp-events.txt | 34 ++++++++++++++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 3 deletions(-) > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > index 1c1f56f..4fcb5b4 100644 > --- a/include/monitor/monitor.h > +++ b/include/monitor/monitor.h > @@ -51,6 +51,8 @@ typedef enum MonitorEvent { > QEVENT_BLOCK_IMAGE_CORRUPTED, > QEVENT_QUORUM_FAILURE, > QEVENT_QUORUM_REPORT_BAD, > + QEVENT_VSERPORT_CONNECTED, > + QEVENT_VSERPORT_DISCONNECTED,
Yay - it's discoverable. Libvirt already queries the set of events that
qemu announces it can send, so the existence of these events is a
witness that the new connection property exists and should be enabled.
Would it hurt anything to unconditionally emit the events, rather than
requiring a configuration option to turn them on?
Also, since these events are triggered in relation to guest activity, I
think they need to be rate-limited (a guest that repeatedly opens and
closes the device as fast as possible should not cause an event storm).
> + if (vcon->report_connstate && dev->id) {
> + QObject *data;
> +
> + data = qobject_from_jsonf("{ 'id': %s }", dev->id);
> + monitor_protocol_event(guest_connected ? QEVENT_VSERPORT_CONNECTED :
> +
> QEVENT_VSERPORT_DISCONNECTED,
> + data);
I wish Wenchao's series on converting events into full-blown QAPI
citizens would hurry up - one of these series will have to rebase on top
of the other.
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg00000.html
> +++ b/docs/qmp/qmp-events.txt
> @@ -509,6 +509,40 @@ Example:
> "host": "127.0.0.1", "sasl_username": "luiz" } },
> "timestamp": { "seconds": 1263475302, "microseconds": 150772 } }
>
> +VSERPORT_CONNECTED
> +------------------
Do we _really_ need two separate events? Why not just one event
VSERPORT_CHANGE, along with an additional data member 'open':'bool' that
is true when the guest opened the port, and false when the guest closed
the port?
1 vs. 2 events is bike-shedding, so I can live with your proposal. But
missing rate-limiting is worth either a v2 or a followup patch. And
unless there is a compelling reason to require a configuration variable
to turn the event on or off, I think it would be simpler to just make
the event unconditional.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
