[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] monitor: remove QAPI_EVENT_VSERPORT_CHANGE thro
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH] monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle |
Date: |
Tue, 11 Aug 2015 18:20:38 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Tue, Aug 11, 2015 at 07:04:07PM +0200, address@hidden wrote:
> From: Marc-André Lureau <address@hidden>
>
> QAPI_EVENT_VSERPORT_CHANGE reports changes of a virtio serial port
> state. However, the events may be for different ports, but the throttle
> mechanism may replace the event for a different port, since it only
> checks the event type.
>
> libvirt relies on a correct state to be reported for all channels: the
> qemu-ga commands may no longer work if the state is reported
> disconnected. This can be triggered easily by having more than 1
> virtio-serial (qemu-ga + spice agent for example), and restarting
> quickly daemons or more realistically going quickly in and out of
> suspend.
>
> In a future patch, we may want to throttle events based on their
> arguments, but this will likely require dynamic allocations and more
> complicated code to insert/lookup pending events based on various
> arguments ("id" in QAPI_EVENT_VSERPORT_CHANGE case).
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1244064
>
> Signed-off-by: Marc-André Lureau <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
> diff --git a/monitor.c b/monitor.c
> index aeea2b5..e4d56f7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -558,7 +558,6 @@ static void monitor_qapi_event_init(void)
> monitor_qapi_event_throttle(QAPI_EVENT_BALLOON_CHANGE, 1000);
> monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000);
> monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000);
> - monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000);
>
> qmp_event_set_func_emit(monitor_qapi_event_queue);
> }
I wonder if we should add a big warnng comment here saying only
to add events if they don't have context sensitive args.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|