[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] virtio-serial: report frontend connection s
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] virtio-serial: report frontend connection state via monitor |
Date: |
Thu, 29 May 2014 14:05:54 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 |
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 <address@hidden>
> ---
> 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
[Qemu-devel] [PATCH 2/2] char: report frontend open/closed state in 'query-chardev', Laszlo Ersek, 2014/05/29
Re: [Qemu-devel] [PATCH 0/2] help libvirt know what's up with qga, Eric Blake, 2014/05/29
Re: [Qemu-devel] [PATCH 0/2] help libvirt know what's up with qga, Eric Blake, 2014/05/29