[Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Amit Shah]

The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:

  s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)

are available in the git repository at:

  https://git.kernel.org/pub/scm/qemu/amit/virtio-serial vser-for-2.5

for you to fetch changes up to 1717c47170ec8df29d1c314761d9caf736c8b274:

  monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle (2015-09-01 17:16:24 +0530)

----------------------------------------------------------------
Disable event throttling for virtio-serial ports - events were throttled
per-device instead of per-port, leading to missed events by higher
layers.

----------------------------------------------------------------
Marc-André Lureau (1):
  monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle

 monitor.c | 1 -
 1 file changed, 1 deletion(-)

-- 
2.4.3

[Qemu-devel] [PULL 1/1] monitor: remove QAPI_EVENT_VSERPORT_CHANGE throttle

[Amit Shah]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

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 <marcandre.lureau@redhat.com>
Message-Id: <1439312647-22245-1-git-send-email-marcandre.lureau@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 monitor.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index fc32f12..16aa92c 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);
 }
-- 
2.4.3

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Peter Maydell]

On 1 September 2015 at 13:04, Amit Shah <amit.shah@redhat.com> wrote:
> The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:
>
>   s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)
>
> are available in the git repository at:
>
>   https://git.kernel.org/pub/scm/qemu/amit/virtio-serial vser-for-2.5

fatal: repository
'https://git.kernel.org/pub/scm/qemu/amit/virtio-serial/' not found

Typo?

-- PMM

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Amit Shah]

On (Tue) 01 Sep 2015 [13:13:56], Peter Maydell wrote:
> On 1 September 2015 at 13:04, Amit Shah <amit.shah@redhat.com> wrote:
> > The following changes since commit 090d0bfd948343d522cd20bc634105b5cfe2483b:
> >
> >   s390: fix softmmu compilation (2015-08-28 16:05:24 +0100)
> >
> > are available in the git repository at:
> >
> >   https://git.kernel.org/pub/scm/qemu/amit/virtio-serial vser-for-2.5
> 
> fatal: repository
> 'https://git.kernel.org/pub/scm/qemu/amit/virtio-serial/' not found
> 
> Typo?

Huh; yeah.  I wonder why request-pull nevertheless gave me the pull
blob.


		Amit

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Marc-André Lureau]

Hi Amit

Out of curiosity, did you check the series "monitor: throttle
VSERPORT_CHANGED by id"? That would fix the throttling instead of just
removing it. This can be applied later, but reviews are always welcome
:)

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Amit Shah]

On (Tue) 01 Sep 2015 [14:27:48], Marc-André Lureau wrote:
> Hi Amit
> 
> Out of curiosity, did you check the series "monitor: throttle
> VSERPORT_CHANGED by id"? That would fix the throttling instead of just
> removing it. This can be applied later, but reviews are always welcome
> :)

So do you want to wait for that series to be ready instead of pulling
this in now?  I'm fine with either approach.

Also: it helps to CC the people you want to review the series - I had
missed that one.


		Amit

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Marc-André Lureau]

Hi

On Tue, Sep 1, 2015 at 2:47 PM, Amit Shah <amit.shah@redhat.com> wrote:
> So do you want to wait for that series to be ready instead of pulling
> this in now?  I'm fine with either approach.

I am fine with both too, I just wanted to point out that there is a
already a follow-up.

> Also: it helps to CC the people you want to review the series - I had
> missed that one.

oops, sorry I missed the CC


-- 
Marc-André Lureau

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Peter Maydell]

On 1 September 2015 at 13:47, Amit Shah <amit.shah@redhat.com> wrote:
> On (Tue) 01 Sep 2015 [14:27:48], Marc-André Lureau wrote:
>> Hi Amit
>>
>> Out of curiosity, did you check the series "monitor: throttle
>> VSERPORT_CHANGED by id"? That would fix the throttling instead of just
>> removing it. This can be applied later, but reviews are always welcome
>> :)
>
> So do you want to wait for that series to be ready instead of pulling
> this in now?  I'm fine with either approach.

I'm going to assume you'll send me a fixed pullreq mail
when you've figured out what you want in it; for now I'm
dropping this from my to-process queue.

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/1] virtio-serial: disable event throttling

[Amit Shah]

On (Tue) 01 Sep 2015 [17:32:07], Peter Maydell wrote:
> On 1 September 2015 at 13:47, Amit Shah <amit.shah@redhat.com> wrote:
> > On (Tue) 01 Sep 2015 [14:27:48], Marc-André Lureau wrote:
> >> Hi Amit
> >>
> >> Out of curiosity, did you check the series "monitor: throttle
> >> VSERPORT_CHANGED by id"? That would fix the throttling instead of just
> >> removing it. This can be applied later, but reviews are always welcome
> >> :)
> >
> > So do you want to wait for that series to be ready instead of pulling
> > this in now?  I'm fine with either approach.
> 
> I'm going to assume you'll send me a fixed pullreq mail
> when you've figured out what you want in it; for now I'm
> dropping this from my to-process queue.

Right - sorry I should've made that clear.

Thanks,

		Amit