From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52028) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPDEH-0000Mr-4p for qemu-devel@nongnu.org; Tue, 11 Aug 2015 13:20:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPDEB-00066x-CU for qemu-devel@nongnu.org; Tue, 11 Aug 2015 13:20:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33286) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPDEB-00066e-4D for qemu-devel@nongnu.org; Tue, 11 Aug 2015 13:20:43 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id B21482589F for ; Tue, 11 Aug 2015 17:20:42 +0000 (UTC) Date: Tue, 11 Aug 2015 18:20:38 +0100 From: "Daniel P. Berrange" Message-ID: <20150811172038.GM19953@redhat.com> References: <1439312647-22245-1-git-send-email-marcandre.lureau@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1439312647-22245-1-git-send-email-marcandre.lureau@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com Cc: amit.shah@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org On Tue, Aug 11, 2015 at 07:04:07PM +0200, marcandre.lureau@redhat.com wro= te: > From: Marc-Andr=C3=A9 Lureau >=20 > 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. >=20 > 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. >=20 > 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). >=20 > Fixes: > https://bugzilla.redhat.com/show_bug.cgi?id=3D1244064 >=20 > Signed-off-by: Marc-Andr=C3=A9 Lureau Reviewed-by: Daniel P. Berrange > 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); > =20 > 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 --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|