Re: [Qemu-devel] monitor: enable OOB by default

2018-07-19 Thread Peter Xu
On Wed, Jul 18, 2018 at 05:08:21PM +0200, Markus Armbruster wrote: > Marc-André, one question for you inline. Search for your name. > > Peter Xu writes: > > > On Thu, Jun 28, 2018 at 03:20:41PM +0200, Markus Armbruster wrote: > >> Peter Xu writes: > >> > >> > On Wed, Jun 27, 2018 at 03:13:57P

Re: [Qemu-devel] monitor: enable OOB by default

2018-07-18 Thread Markus Armbruster
Marc-André, one question for you inline. Search for your name. Peter Xu writes: > On Thu, Jun 28, 2018 at 03:20:41PM +0200, Markus Armbruster wrote: >> Peter Xu writes: >> >> > On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: >> >> Monitor behavior changes even when the clie

Re: [Qemu-devel] monitor: enable OOB by default

2018-07-04 Thread Markus Armbruster
Peter Xu writes: > On Mon, Jul 02, 2018 at 07:43:06AM +0200, Markus Armbruster wrote: >> More lose ends: >> >> * scripts/qmp/ doesn't support OOB, yet. qmp-shell.py in particular >> >> * test-qmp-cmds neglects to cover the OOB additions to qmp-dispatch.c > > Would you mind I put these aside fo

Re: [Qemu-devel] monitor: enable OOB by default

2018-07-03 Thread Peter Xu
On Mon, Jul 02, 2018 at 07:43:06AM +0200, Markus Armbruster wrote: > More lose ends: > > * scripts/qmp/ doesn't support OOB, yet. qmp-shell.py in particular > > * test-qmp-cmds neglects to cover the OOB additions to qmp-dispatch.c Would you mind I put these aside for now? I'm afraid things gro

Re: [Qemu-devel] monitor: enable OOB by default

2018-07-01 Thread Markus Armbruster
More lose ends: * scripts/qmp/ doesn't support OOB, yet. qmp-shell.py in particular * test-qmp-cmds neglects to cover the OOB additions to qmp-dispatch.c

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-29 Thread Peter Xu
On Thu, Jun 28, 2018 at 11:29:30AM +0200, Markus Armbruster wrote: > Peter Xu writes: > > > On Wed, Jun 27, 2018 at 10:35:15AM +0200, Markus Armbruster wrote: > >> Markus Armbruster writes: > >> > >> > Another lose end: event COMMAND_DROPPED seems to lack test coverage. > >> > >> Hmm, dropping

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-29 Thread Peter Xu
On Thu, Jun 28, 2018 at 03:20:41PM +0200, Markus Armbruster wrote: > Peter Xu writes: > > > On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: > >> Monitor behavior changes even when the client rejects capability "oob". > >> > >> Traditionally, the monitor reads, executes and res

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-29 Thread Peter Xu
On Thu, Jun 28, 2018 at 08:55:48AM +0200, Markus Armbruster wrote: > Peter Xu writes: > > > On Wed, Jun 27, 2018 at 01:23:07PM +0200, Markus Armbruster wrote: > >> Daniel P. Berrangé writes: > >> > >> > On Wed, Jun 27, 2018 at 10:41:38AM +0200, Markus Armbruster wrote: > >> >> Markus Armbruster

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-29 Thread Peter Xu
On Thu, Jun 28, 2018 at 09:04:19AM +0200, Markus Armbruster wrote: > Eric Blake writes: > > > On 06/27/2018 07:07 AM, Peter Xu wrote: > > > Worse than that - broadcasting to all monitors is categorically broken. > Different monitors make use the same "id" formatting scheme, so if you >

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-28 Thread Markus Armbruster
Peter Xu writes: > On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: >> Monitor behavior changes even when the client rejects capability "oob". >> >> Traditionally, the monitor reads, executes and responds to one command >> after the other. If the client sends in-band commands

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-28 Thread Markus Armbruster
Daniel P. Berrangé writes: > On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: >> Monitor behavior changes even when the client rejects capability "oob". >> >> Traditionally, the monitor reads, executes and responds to one command >> after the other. If the client sends in-band

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-28 Thread Eric Blake
On 06/28/2018 01:55 AM, Markus Armbruster wrote: Changing an existing event from broadcast to unicast is an observable change in existing behavior. Compatibility break unless we can show nobody can use / uses the observation. And no one could have been relying on the broadcast COMMAND_DROPPED

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-28 Thread Markus Armbruster
Peter Xu writes: > On Wed, Jun 27, 2018 at 10:35:15AM +0200, Markus Armbruster wrote: >> Markus Armbruster writes: >> >> > Another lose end: event COMMAND_DROPPED seems to lack test coverage. >> >> Hmm, dropping commands serves to limit the request queue. What limits >> the response queue? >

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-28 Thread Markus Armbruster
Eric Blake writes: > On 06/27/2018 07:07 AM, Peter Xu wrote: > Worse than that - broadcasting to all monitors is categorically broken. Different monitors make use the same "id" formatting scheme, so if you broadcast COMMAND_DROPPED to a different monitor you might have clashing >>>

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Peter Xu writes: > On Wed, Jun 27, 2018 at 01:23:07PM +0200, Markus Armbruster wrote: >> Daniel P. Berrangé writes: >> >> > On Wed, Jun 27, 2018 at 10:41:38AM +0200, Markus Armbruster wrote: >> >> Markus Armbruster writes: >> >> >> >> > Markus Armbruster writes: >> >> > >> >> >> I fooled aro

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Peter Xu
On Wed, Jun 27, 2018 at 09:40:40AM +0200, Markus Armbruster wrote: > Another lose end: event COMMAND_DROPPED seems to lack test coverage. This one I can do. Since I'll try to prepare some patches to fix the event issue, I can try to add the test alongside. Thanks for mentioning! -- Peter Xu

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Peter Xu
On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: > Monitor behavior changes even when the client rejects capability "oob". > > Traditionally, the monitor reads, executes and responds to one command > after the other. If the client sends in-band commands faster than the > server

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Daniel P . Berrangé
On Wed, Jun 27, 2018 at 03:13:57PM +0200, Markus Armbruster wrote: > Monitor behavior changes even when the client rejects capability "oob". > > Traditionally, the monitor reads, executes and responds to one command > after the other. If the client sends in-band commands faster than the > server

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Monitor behavior changes even when the client rejects capability "oob". Traditionally, the monitor reads, executes and responds to one command after the other. If the client sends in-band commands faster than the server can execute them, the kernel will eventually refuse to buffer more, and sendi

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Eric Blake
On 06/27/2018 07:07 AM, Peter Xu wrote: Worse than that - broadcasting to all monitors is categorically broken. Different monitors make use the same "id" formatting scheme, so if you broadcast COMMAND_DROPPED to a different monitor you might have clashing "id" and thus incorrectly tell a client

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Peter Xu
On Wed, Jun 27, 2018 at 10:35:15AM +0200, Markus Armbruster wrote: > Markus Armbruster writes: > > > Another lose end: event COMMAND_DROPPED seems to lack test coverage. > > Hmm, dropping commands serves to limit the request queue. What limits > the response queue? As long as we have a request

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Peter Xu
On Wed, Jun 27, 2018 at 01:23:07PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé writes: > > > On Wed, Jun 27, 2018 at 10:41:38AM +0200, Markus Armbruster wrote: > >> Markus Armbruster writes: > >> > >> > Markus Armbruster writes: > >> > > >> >> I fooled around a bit, and I think there

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Daniel P. Berrangé writes: > On Wed, Jun 27, 2018 at 10:41:38AM +0200, Markus Armbruster wrote: >> Markus Armbruster writes: >> >> > Markus Armbruster writes: >> > >> >> I fooled around a bit, and I think there are a few lose ends. >> > [...] >> >> Talking to a QMP monitor that supports OOB: >

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Daniel P . Berrangé
On Wed, Jun 27, 2018 at 10:41:38AM +0200, Markus Armbruster wrote: > Markus Armbruster writes: > > > Markus Armbruster writes: > > > >> I fooled around a bit, and I think there are a few lose ends. > > [...] > >> Talking to a QMP monitor that supports OOB: > >> > >> $ socat UNIX:test-qmp REA

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Markus Armbruster writes: > Markus Armbruster writes: > >> I fooled around a bit, and I think there are a few lose ends. > [...] >> Talking to a QMP monitor that supports OOB: >> >> $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> ' >> {"QMP": {"version": {"qemu": {

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Markus Armbruster writes: > Another lose end: event COMMAND_DROPPED seems to lack test coverage. Hmm, dropping commands serves to limit the request queue. What limits the response queue? Before OOB, the monitor read at most one command, and wrote its response with monitor_puts(). For input, t

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Markus Armbruster writes: > I fooled around a bit, and I think there are a few lose ends. [...] > Talking to a QMP monitor that supports OOB: > > $ socat UNIX:test-qmp READLINE,history=$HOME/.qmp_history,prompt='QMP> ' > {"QMP": {"version": {"qemu": {"micro": 50, "minor": 12, "major": 2},

Re: [Qemu-devel] monitor: enable OOB by default

2018-06-27 Thread Markus Armbruster
Another lose end: event COMMAND_DROPPED seems to lack test coverage.