This is RFC v3 of Monitor Out-Of-Band series.  It's getting longer and
changes happens between versions, so I decided to re-write the cover
letter.

This series was born from this one:

  https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg04310.html

The idea comes from Markus Armbruster and the discussion we had in the
thread.  It's not a super ideal solution (I believe Markus had been
thinking hard to keep everything in order meanwhile trying to satisfy
the migration requirement), but AFAIU it's currently the best.

What is OOB?
============

It's the shortcut of Out-Of-Band execution, its name is given by
Markus.  It's a way to quickly execute a QMP request.  Say, originally
QMP is going throw these steps:

      JSON Parser --> QMP Dispatcher --> Respond
          /|\    (2)                (3)     |
       (1) |                               \|/ (4)
           +---------  main thread  --------+

The requests are executed by the so-called QMP-dispatcher after the
JSON is parsed.  If OOB is on, we run the command directly in the
parser and quickly returns.

This series changed the QMP handling logic by moving the command
parsing and responding phases into IOThreads, so to be more accurate,
after the series the above graph would change into this:

               queue/kick              queue/kick
     JSON Parser ======> QMP Dispatcher =====> Responder
         /|\ |     (3)       /|\  |      (4)      | /|\
      (1) |  | (2)            |   |               |  |
          |  |                |  \|/           (6)|  |(5)
          |  |            main thread             |  |
          |  |                                    |  |
          |  +--------> monitor IO thread <-------+  |
          +-----------/                   \----------+

New Interfaces
==============

QMP Introspection Changes
-------------------------

When do query-qmp-schema, we were getting something like:

  {"name": "migrate-incoming", "ret-type": "17",
   "meta-type": "command", "arg-type": "89"}

Now we will get a new key named "allow-oob":

  {"name": "migrate-incoming", "ret-type": "17", "allow-oob": true,
   "meta-type": "command", "arg-type": "89"}

Which shows whether a command supports OOB.

QMP Negociation Changes
-----------------------

We were running "qmp_capabilities" always without parameters like:

  {"execute": "qmp_capabilities"}

Now we can enable capabilities (we don't have any capability before
this series) like OOB using:

  {"execute": "qmp_capabilities", "arguments": {"enable": ["oob"]}}

Only after we explicitly enable OOB capability can we send requests in
OOB manner.  Otherwise we'll have exactly the same QMP session as
before, just like when OOB is not there.

When OOB is enabled, it's possible that OOB reply reaches faster than
previous command, so clients should be prepared.

Trigger OOB execution
---------------------

Let's take migrate-incoming as example.  The old command looks like:

  {"execute": "migrate-incoming", "arguments" : {"uri": "xxxxxx"}}

To execute a command with OOB execution, we need to specify it in the
QMP request in the extra "control" key:

  {"execute": "migrate-incoming", "arguments" : {"uri": "xxxxxx"},
   "control" { "run-oob": true } }

Then the command will be run in parser, and it can preempt other
commands.

Others
=================

The last patch of OOB test may need some attention.  I used
dump-guest-memory as a time-consuming command to test whether OOB is
working, and the only command I can test now is "migrate-incoming".  I
hope that is a "okay" solution for unit tests.  Any other suggestions
on that would be welcomed.

v3 changelog:
- inline monitor_io_thread_destroy(). [Stefan]
- rename usage of "*io_thread*" into "*iothread*" [Stefan]
- fixed quite a few English errors [Stefan]
- add the missing "return" (oops!) in handle_qmp_command(). [Stefan]
- rename QMP_ASYNC_QUEUE_LEN_MAX to QMP_REQ_QUEUE_LEN_MAX. [Stefan]
- add OOB cap negociationg phase, conditionally enable OOB.  Make sure
  that old QMP client requests will never be dropped by using
  monitor_suspend() when OOB not enabled. [Stefan]
- add new lock Monitor.qmp.qmp_queue_lock, then take it when queuing
  for requests/responses. [Stefan]
- move Monitor fields "qmp_requests" & "qmp_responses" into MonitorQMP
  struct for cleaness.
- write more tests for oob, and some smoke test with libvirt, hoping
  that it won't break too many things

v2 changelog:
- use 10-char hash for git commit ID [Eric]
- instead of changing qstring_get_str(), add new
  qstring_get_try_str(), and rename qobject_get_str() to
  qobject_get_try_str() to follow the naming rule [Eric]
- add one more patch to let object_property_get_str() leverage the new
  qobject_get_try_str(). [Eric]
- introduce JSONMessageEmitFunc in the JSON parser patch [Eric]
- use per-monitor request queue, rather than a global queue
- add queue flow control, when full (current queue depth: 8), send
  event to notify client (two new patches added for this)
- stop monitor thread first before destroying monitor objects [Dan]
- document that client should drop responses with unknown IDs.
- move all the documents into a single standalone patch.
- use iothread for the monitor io thread.
- respond in IO thread, rather than the dispatcher
- new patch: in QMP greeting message, add 'oob' field in
  'capabilities', so that client can explicitly know whether server
  supports oob

Please review.  Thanks.

Peter Xu (27):
  char-io: fix possible race on IOWatchPoll
  qobject: introduce qstring_get_try_str()
  qobject: introduce qobject_get_try_str()
  qobject: let object_property_get_str() use new API
  monitor: move skip_flush into monitor_data_init
  qjson: add "opaque" field to JSONMessageParser
  monitor: move the cur_mon hack deeper for QMP
  monitor: unify global init
  monitor: let mon_list be tail queue
  monitor: create monitor dedicate iothread
  monitor: allow to use IO thread for parsing
  qmp: introduce QMPCapability
  qmp: negociate QMP capabilities
  qmp: introduce some capability helpers
  monitor: introduce monitor_qmp_respond()
  monitor: let monitor_{suspend|resume} thread safe
  monitor: separate QMP parser and dispatcher
  qmp: add new event "request-dropped"
  monitor: send event when request queue full
  qapi: introduce new cmd option "allow-oob"
  qmp: support out-of-band (oob) execution
  qmp: let migrate-incoming allow out-of-band
  qmp: isolate responses into io thread
  monitor: enable IO thread for (qmp & !mux) typed
  docs: update QMP documents for OOB commands
  tests: qmp-test: verify command batching
  tests: qmp-test: add oob test

 chardev/char-io.c                |  16 +-
 docs/devel/qapi-code-gen.txt     |  51 +++-
 docs/interop/qmp-spec.txt        |  32 ++-
 include/monitor/monitor.h        |   2 +-
 include/qapi/qmp/dispatch.h      |   2 +
 include/qapi/qmp/json-streamer.h |  10 +-
 include/qapi/qmp/qstring.h       |   2 +
 monitor.c                        | 546 +++++++++++++++++++++++++++++++++------
 qapi-schema.json                 |  63 ++++-
 qapi/introspect.json             |   6 +-
 qapi/migration.json              |   3 +-
 qapi/qmp-dispatch.c              |  39 +++
 qga/main.c                       |   5 +-
 qobject/json-streamer.c          |   6 +-
 qobject/qjson.c                  |   5 +-
 qobject/qstring.c                |  21 ++
 qom/object.c                     |   9 +-
 scripts/qapi-commands.py         |  19 +-
 scripts/qapi-introspect.py       |  10 +-
 scripts/qapi.py                  |  15 +-
 scripts/qapi2texi.py             |   2 +-
 tests/libqtest.c                 |   5 +-
 tests/qapi-schema/test-qapi.py   |   2 +-
 tests/qmp-test.c                 | 122 ++++++++-
 trace-events                     |   2 +
 vl.c                             |   3 +-
 26 files changed, 876 insertions(+), 122 deletions(-)

-- 
2.13.5

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [RFC v3 00/27] QMP: out-of-band (OOB) execution support
Type: series
Message-id: 20171106094643.14881-1-peterx@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20171106094643.14881-1-peterx@redhat.com -> patchew/20171106094643.14881-1-peterx@redhat.com
Switched to a new branch 'test'
5525c4e791 tests: qmp-test: add oob test
ccc9e4c399 tests: qmp-test: verify command batching
7f45b4c6c0 docs: update QMP documents for OOB commands
58cfe877d2 monitor: enable IO thread for (qmp & !mux) typed
5e1d56ce74 qmp: isolate responses into io thread
aef4275536 qmp: let migrate-incoming allow out-of-band
5e68beacf3 qmp: support out-of-band (oob) execution
43c7215a30 qapi: introduce new cmd option "allow-oob"
e11127ba4b monitor: send event when request queue full
45cef4f7e4 qmp: add new event "request-dropped"
485da28be1 monitor: separate QMP parser and dispatcher
4892fe9ca2 monitor: let monitor_{suspend|resume} thread safe
1b86166d9c monitor: introduce monitor_qmp_respond()
0f48093add qmp: introduce some capability helpers
8d3f33043d qmp: negociate QMP capabilities
023b386d0e qmp: introduce QMPCapability
2bde5ca8ce monitor: allow to use IO thread for parsing
f4cc112f80 monitor: create monitor dedicate iothread
3590fdc1d4 monitor: let mon_list be tail queue
11c818a9ac monitor: unify global init
36d3efb87d monitor: move the cur_mon hack deeper for QMP
bf3e493a86 qjson: add "opaque" field to JSONMessageParser
17367fe7a1 monitor: move skip_flush into monitor_data_init
0c98d4baa4 qobject: let object_property_get_str() use new API
aa4b973dd5 qobject: introduce qobject_get_try_str()
981ccebc1e qobject: introduce qstring_get_try_str()
d40ba38085 char-io: fix possible race on IOWatchPoll

=== OUTPUT BEGIN ===
Checking PATCH 1/27: char-io: fix possible race on IOWatchPoll...
Checking PATCH 2/27: qobject: introduce qstring_get_try_str()...
Checking PATCH 3/27: qobject: introduce qobject_get_try_str()...
Checking PATCH 4/27: qobject: let object_property_get_str() use new API...
Checking PATCH 5/27: monitor: move skip_flush into monitor_data_init...
Checking PATCH 6/27: qjson: add "opaque" field to JSONMessageParser...
Checking PATCH 7/27: monitor: move the cur_mon hack deeper for QMP...
Checking PATCH 8/27: monitor: unify global init...
Checking PATCH 9/27: monitor: let mon_list be tail queue...
Checking PATCH 10/27: monitor: create monitor dedicate iothread...
Checking PATCH 11/27: monitor: allow to use IO thread for parsing...
Checking PATCH 12/27: qmp: introduce QMPCapability...
Checking PATCH 13/27: qmp: negociate QMP capabilities...
Checking PATCH 14/27: qmp: introduce some capability helpers...
Checking PATCH 15/27: monitor: introduce monitor_qmp_respond()...
Checking PATCH 16/27: monitor: let monitor_{suspend|resume} thread safe...
ERROR: braces {} are necessary for all arms of this statement
#28: FILE: monitor.c:4014:
+    if (atomic_dec_fetch(&mon->suspend_cnt) == 0)
[...]

ERROR: Missing Signed-off-by: line(s)

total: 2 errors, 0 warnings, 16 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 17/27: monitor: separate QMP parser and dispatcher...
Checking PATCH 18/27: qmp: add new event "request-dropped"...
Checking PATCH 19/27: monitor: send event when request queue full...
Checking PATCH 20/27: qapi: introduce new cmd option "allow-oob"...
Checking PATCH 21/27: qmp: support out-of-band (oob) execution...
Checking PATCH 22/27: qmp: let migrate-incoming allow out-of-band...
Checking PATCH 23/27: qmp: isolate responses into io thread...
Checking PATCH 24/27: monitor: enable IO thread for (qmp & !mux) typed...
Checking PATCH 25/27: docs: update QMP documents for OOB commands...
Checking PATCH 26/27: tests: qmp-test: verify command batching...
Checking PATCH 27/27: tests: qmp-test: add oob test...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

On Mon, Nov 06, 2017 at 02:12:17AM -0800, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Subject: [Qemu-devel] [RFC v3 00/27] QMP: out-of-band (OOB) execution support
> Type: series
> Message-id: 20171106094643.14881-1-peterx@redhat.com
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> git config --local diff.renamelimit 0
> git config --local diff.renames True
> 
> commits="$(git log --format=%H --reverse $BASE..)"
> for c in $commits; do
>     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
>     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
>         failed=1
>         echo
>     fi
>     n=$((n+1))
> done
> 
> exit $failed
> === TEST SCRIPT END ===
> 
> Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> From https://github.com/patchew-project/qemu
>  * [new tag]               patchew/20171106094643.14881-1-peterx@redhat.com -> patchew/20171106094643.14881-1-peterx@redhat.com
> Switched to a new branch 'test'
> 5525c4e791 tests: qmp-test: add oob test
> ccc9e4c399 tests: qmp-test: verify command batching
> 7f45b4c6c0 docs: update QMP documents for OOB commands
> 58cfe877d2 monitor: enable IO thread for (qmp & !mux) typed
> 5e1d56ce74 qmp: isolate responses into io thread
> aef4275536 qmp: let migrate-incoming allow out-of-band
> 5e68beacf3 qmp: support out-of-band (oob) execution
> 43c7215a30 qapi: introduce new cmd option "allow-oob"
> e11127ba4b monitor: send event when request queue full
> 45cef4f7e4 qmp: add new event "request-dropped"
> 485da28be1 monitor: separate QMP parser and dispatcher
> 4892fe9ca2 monitor: let monitor_{suspend|resume} thread safe
> 1b86166d9c monitor: introduce monitor_qmp_respond()
> 0f48093add qmp: introduce some capability helpers
> 8d3f33043d qmp: negociate QMP capabilities
> 023b386d0e qmp: introduce QMPCapability
> 2bde5ca8ce monitor: allow to use IO thread for parsing
> f4cc112f80 monitor: create monitor dedicate iothread
> 3590fdc1d4 monitor: let mon_list be tail queue
> 11c818a9ac monitor: unify global init
> 36d3efb87d monitor: move the cur_mon hack deeper for QMP
> bf3e493a86 qjson: add "opaque" field to JSONMessageParser
> 17367fe7a1 monitor: move skip_flush into monitor_data_init
> 0c98d4baa4 qobject: let object_property_get_str() use new API
> aa4b973dd5 qobject: introduce qobject_get_try_str()
> 981ccebc1e qobject: introduce qstring_get_try_str()
> d40ba38085 char-io: fix possible race on IOWatchPoll
> 
> === OUTPUT BEGIN ===
> Checking PATCH 1/27: char-io: fix possible race on IOWatchPoll...
> Checking PATCH 2/27: qobject: introduce qstring_get_try_str()...
> Checking PATCH 3/27: qobject: introduce qobject_get_try_str()...
> Checking PATCH 4/27: qobject: let object_property_get_str() use new API...
> Checking PATCH 5/27: monitor: move skip_flush into monitor_data_init...
> Checking PATCH 6/27: qjson: add "opaque" field to JSONMessageParser...
> Checking PATCH 7/27: monitor: move the cur_mon hack deeper for QMP...
> Checking PATCH 8/27: monitor: unify global init...
> Checking PATCH 9/27: monitor: let mon_list be tail queue...
> Checking PATCH 10/27: monitor: create monitor dedicate iothread...
> Checking PATCH 11/27: monitor: allow to use IO thread for parsing...
> Checking PATCH 12/27: qmp: introduce QMPCapability...
> Checking PATCH 13/27: qmp: negociate QMP capabilities...
> Checking PATCH 14/27: qmp: introduce some capability helpers...
> Checking PATCH 15/27: monitor: introduce monitor_qmp_respond()...
> Checking PATCH 16/27: monitor: let monitor_{suspend|resume} thread safe...
> ERROR: braces {} are necessary for all arms of this statement
> #28: FILE: monitor.c:4014:
> +    if (atomic_dec_fetch(&mon->suspend_cnt) == 0)
> [...]
> 
> ERROR: Missing Signed-off-by: line(s)

Will add it in next post.  I still haven't found a good way to let
magit add this line for me every time automatically.

Please review the rest of things, thanks.

-- 
Peter Xu

On Mon, Nov 06, 2017 at 09:08:00PM +0800, Peter Xu wrote:
> On Mon, Nov 06, 2017 at 02:12:17AM -0800, no-reply@patchew.org wrote:
> > Hi,
> > 
> > This series seems to have some coding style problems. See output below for
> > more information:
> > 
> > Subject: [Qemu-devel] [RFC v3 00/27] QMP: out-of-band (OOB) execution support
> > Type: series
> > Message-id: 20171106094643.14881-1-peterx@redhat.com
> > 
> > === TEST SCRIPT BEGIN ===
> > #!/bin/bash
> > 
> > BASE=base
> > n=1
> > total=$(git log --oneline $BASE.. | wc -l)
> > failed=0
> > 
> > git config --local diff.renamelimit 0
> > git config --local diff.renames True
> > 
> > commits="$(git log --format=%H --reverse $BASE..)"
> > for c in $commits; do
> >     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> >     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> >         failed=1
> >         echo
> >     fi
> >     n=$((n+1))
> > done
> > 
> > exit $failed
> > === TEST SCRIPT END ===
> > 
> > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > From https://github.com/patchew-project/qemu
> >  * [new tag]               patchew/20171106094643.14881-1-peterx@redhat.com -> patchew/20171106094643.14881-1-peterx@redhat.com
> > Switched to a new branch 'test'
> > 5525c4e791 tests: qmp-test: add oob test
> > ccc9e4c399 tests: qmp-test: verify command batching
> > 7f45b4c6c0 docs: update QMP documents for OOB commands
> > 58cfe877d2 monitor: enable IO thread for (qmp & !mux) typed
> > 5e1d56ce74 qmp: isolate responses into io thread
> > aef4275536 qmp: let migrate-incoming allow out-of-band
> > 5e68beacf3 qmp: support out-of-band (oob) execution
> > 43c7215a30 qapi: introduce new cmd option "allow-oob"
> > e11127ba4b monitor: send event when request queue full
> > 45cef4f7e4 qmp: add new event "request-dropped"
> > 485da28be1 monitor: separate QMP parser and dispatcher
> > 4892fe9ca2 monitor: let monitor_{suspend|resume} thread safe
> > 1b86166d9c monitor: introduce monitor_qmp_respond()
> > 0f48093add qmp: introduce some capability helpers
> > 8d3f33043d qmp: negociate QMP capabilities
> > 023b386d0e qmp: introduce QMPCapability
> > 2bde5ca8ce monitor: allow to use IO thread for parsing
> > f4cc112f80 monitor: create monitor dedicate iothread
> > 3590fdc1d4 monitor: let mon_list be tail queue
> > 11c818a9ac monitor: unify global init
> > 36d3efb87d monitor: move the cur_mon hack deeper for QMP
> > bf3e493a86 qjson: add "opaque" field to JSONMessageParser
> > 17367fe7a1 monitor: move skip_flush into monitor_data_init
> > 0c98d4baa4 qobject: let object_property_get_str() use new API
> > aa4b973dd5 qobject: introduce qobject_get_try_str()
> > 981ccebc1e qobject: introduce qstring_get_try_str()
> > d40ba38085 char-io: fix possible race on IOWatchPoll
> > 
> > === OUTPUT BEGIN ===
> > Checking PATCH 1/27: char-io: fix possible race on IOWatchPoll...
> > Checking PATCH 2/27: qobject: introduce qstring_get_try_str()...
> > Checking PATCH 3/27: qobject: introduce qobject_get_try_str()...
> > Checking PATCH 4/27: qobject: let object_property_get_str() use new API...
> > Checking PATCH 5/27: monitor: move skip_flush into monitor_data_init...
> > Checking PATCH 6/27: qjson: add "opaque" field to JSONMessageParser...
> > Checking PATCH 7/27: monitor: move the cur_mon hack deeper for QMP...
> > Checking PATCH 8/27: monitor: unify global init...
> > Checking PATCH 9/27: monitor: let mon_list be tail queue...
> > Checking PATCH 10/27: monitor: create monitor dedicate iothread...
> > Checking PATCH 11/27: monitor: allow to use IO thread for parsing...
> > Checking PATCH 12/27: qmp: introduce QMPCapability...
> > Checking PATCH 13/27: qmp: negociate QMP capabilities...
> > Checking PATCH 14/27: qmp: introduce some capability helpers...
> > Checking PATCH 15/27: monitor: introduce monitor_qmp_respond()...
> > Checking PATCH 16/27: monitor: let monitor_{suspend|resume} thread safe...
> > ERROR: braces {} are necessary for all arms of this statement
> > #28: FILE: monitor.c:4014:
> > +    if (atomic_dec_fetch(&mon->suspend_cnt) == 0)
> > [...]
> > 
> > ERROR: Missing Signed-off-by: line(s)
> 
> Will add it in next post.  I still haven't found a good way to let
> magit add this line for me every time automatically.

If you don't want to type "git commit -s" all the time you could use the
"format.signOff = on" git-config(1) variable.

On Wed, Nov 15, 2017 at 09:42:46AM +0000, Stefan Hajnoczi wrote:
> On Mon, Nov 06, 2017 at 09:08:00PM +0800, Peter Xu wrote:
> > On Mon, Nov 06, 2017 at 02:12:17AM -0800, no-reply@patchew.org wrote:
> > > Hi,
> > > 
> > > This series seems to have some coding style problems. See output below for
> > > more information:
> > > 
> > > Subject: [Qemu-devel] [RFC v3 00/27] QMP: out-of-band (OOB) execution support
> > > Type: series
> > > Message-id: 20171106094643.14881-1-peterx@redhat.com
> > > 
> > > === TEST SCRIPT BEGIN ===
> > > #!/bin/bash
> > > 
> > > BASE=base
> > > n=1
> > > total=$(git log --oneline $BASE.. | wc -l)
> > > failed=0
> > > 
> > > git config --local diff.renamelimit 0
> > > git config --local diff.renames True
> > > 
> > > commits="$(git log --format=%H --reverse $BASE..)"
> > > for c in $commits; do
> > >     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> > >     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> > >         failed=1
> > >         echo
> > >     fi
> > >     n=$((n+1))
> > > done
> > > 
> > > exit $failed
> > > === TEST SCRIPT END ===
> > > 
> > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > > From https://github.com/patchew-project/qemu
> > >  * [new tag]               patchew/20171106094643.14881-1-peterx@redhat.com -> patchew/20171106094643.14881-1-peterx@redhat.com
> > > Switched to a new branch 'test'
> > > 5525c4e791 tests: qmp-test: add oob test
> > > ccc9e4c399 tests: qmp-test: verify command batching
> > > 7f45b4c6c0 docs: update QMP documents for OOB commands
> > > 58cfe877d2 monitor: enable IO thread for (qmp & !mux) typed
> > > 5e1d56ce74 qmp: isolate responses into io thread
> > > aef4275536 qmp: let migrate-incoming allow out-of-band
> > > 5e68beacf3 qmp: support out-of-band (oob) execution
> > > 43c7215a30 qapi: introduce new cmd option "allow-oob"
> > > e11127ba4b monitor: send event when request queue full
> > > 45cef4f7e4 qmp: add new event "request-dropped"
> > > 485da28be1 monitor: separate QMP parser and dispatcher
> > > 4892fe9ca2 monitor: let monitor_{suspend|resume} thread safe
> > > 1b86166d9c monitor: introduce monitor_qmp_respond()
> > > 0f48093add qmp: introduce some capability helpers
> > > 8d3f33043d qmp: negociate QMP capabilities
> > > 023b386d0e qmp: introduce QMPCapability
> > > 2bde5ca8ce monitor: allow to use IO thread for parsing
> > > f4cc112f80 monitor: create monitor dedicate iothread
> > > 3590fdc1d4 monitor: let mon_list be tail queue
> > > 11c818a9ac monitor: unify global init
> > > 36d3efb87d monitor: move the cur_mon hack deeper for QMP
> > > bf3e493a86 qjson: add "opaque" field to JSONMessageParser
> > > 17367fe7a1 monitor: move skip_flush into monitor_data_init
> > > 0c98d4baa4 qobject: let object_property_get_str() use new API
> > > aa4b973dd5 qobject: introduce qobject_get_try_str()
> > > 981ccebc1e qobject: introduce qstring_get_try_str()
> > > d40ba38085 char-io: fix possible race on IOWatchPoll
> > > 
> > > === OUTPUT BEGIN ===
> > > Checking PATCH 1/27: char-io: fix possible race on IOWatchPoll...
> > > Checking PATCH 2/27: qobject: introduce qstring_get_try_str()...
> > > Checking PATCH 3/27: qobject: introduce qobject_get_try_str()...
> > > Checking PATCH 4/27: qobject: let object_property_get_str() use new API...
> > > Checking PATCH 5/27: monitor: move skip_flush into monitor_data_init...
> > > Checking PATCH 6/27: qjson: add "opaque" field to JSONMessageParser...
> > > Checking PATCH 7/27: monitor: move the cur_mon hack deeper for QMP...
> > > Checking PATCH 8/27: monitor: unify global init...
> > > Checking PATCH 9/27: monitor: let mon_list be tail queue...
> > > Checking PATCH 10/27: monitor: create monitor dedicate iothread...
> > > Checking PATCH 11/27: monitor: allow to use IO thread for parsing...
> > > Checking PATCH 12/27: qmp: introduce QMPCapability...
> > > Checking PATCH 13/27: qmp: negociate QMP capabilities...
> > > Checking PATCH 14/27: qmp: introduce some capability helpers...
> > > Checking PATCH 15/27: monitor: introduce monitor_qmp_respond()...
> > > Checking PATCH 16/27: monitor: let monitor_{suspend|resume} thread safe...
> > > ERROR: braces {} are necessary for all arms of this statement
> > > #28: FILE: monitor.c:4014:
> > > +    if (atomic_dec_fetch(&mon->suspend_cnt) == 0)
> > > [...]
> > > 
> > > ERROR: Missing Signed-off-by: line(s)
> > 
> > Will add it in next post.  I still haven't found a good way to let
> > magit add this line for me every time automatically.
> 
> If you don't want to type "git commit -s" all the time you could use the
> "format.signOff = on" git-config(1) variable.

I tried it, but it seems to be for when formatting patches rather than
putting that s-o-b line into commit messages.  I think what I wanted
was something like commit.signOff but after some searches I found that
idea was proposed but rejected somehow:

http://git.661346.n2.nabble.com/PATCH-Add-a-commit-signoff-configuration-variable-to-always-use-signoff-td1886619.html#a2093996

So I think I'll just live with my current approach.  Anway, thanks for
the hint!

-- 
Peter Xu

On Thu, 11/16 13:32, Peter Xu wrote:
> On Wed, Nov 15, 2017 at 09:42:46AM +0000, Stefan Hajnoczi wrote:
> > On Mon, Nov 06, 2017 at 09:08:00PM +0800, Peter Xu wrote:
> > > On Mon, Nov 06, 2017 at 02:12:17AM -0800, no-reply@patchew.org wrote:
> > > > Hi,
> > > > 
> > > > This series seems to have some coding style problems. See output below for
> > > > more information:
> > > > 
> > > > Subject: [Qemu-devel] [RFC v3 00/27] QMP: out-of-band (OOB) execution support
> > > > Type: series
> > > > Message-id: 20171106094643.14881-1-peterx@redhat.com
> > > > 
> > > > === TEST SCRIPT BEGIN ===
> > > > #!/bin/bash
> > > > 
> > > > BASE=base
> > > > n=1
> > > > total=$(git log --oneline $BASE.. | wc -l)
> > > > failed=0
> > > > 
> > > > git config --local diff.renamelimit 0
> > > > git config --local diff.renames True
> > > > 
> > > > commits="$(git log --format=%H --reverse $BASE..)"
> > > > for c in $commits; do
> > > >     echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
> > > >     if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
> > > >         failed=1
> > > >         echo
> > > >     fi
> > > >     n=$((n+1))
> > > > done
> > > > 
> > > > exit $failed
> > > > === TEST SCRIPT END ===
> > > > 
> > > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
> > > > From https://github.com/patchew-project/qemu
> > > >  * [new tag]               patchew/20171106094643.14881-1-peterx@redhat.com -> patchew/20171106094643.14881-1-peterx@redhat.com
> > > > Switched to a new branch 'test'
> > > > 5525c4e791 tests: qmp-test: add oob test
> > > > ccc9e4c399 tests: qmp-test: verify command batching
> > > > 7f45b4c6c0 docs: update QMP documents for OOB commands
> > > > 58cfe877d2 monitor: enable IO thread for (qmp & !mux) typed
> > > > 5e1d56ce74 qmp: isolate responses into io thread
> > > > aef4275536 qmp: let migrate-incoming allow out-of-band
> > > > 5e68beacf3 qmp: support out-of-band (oob) execution
> > > > 43c7215a30 qapi: introduce new cmd option "allow-oob"
> > > > e11127ba4b monitor: send event when request queue full
> > > > 45cef4f7e4 qmp: add new event "request-dropped"
> > > > 485da28be1 monitor: separate QMP parser and dispatcher
> > > > 4892fe9ca2 monitor: let monitor_{suspend|resume} thread safe
> > > > 1b86166d9c monitor: introduce monitor_qmp_respond()
> > > > 0f48093add qmp: introduce some capability helpers
> > > > 8d3f33043d qmp: negociate QMP capabilities
> > > > 023b386d0e qmp: introduce QMPCapability
> > > > 2bde5ca8ce monitor: allow to use IO thread for parsing
> > > > f4cc112f80 monitor: create monitor dedicate iothread
> > > > 3590fdc1d4 monitor: let mon_list be tail queue
> > > > 11c818a9ac monitor: unify global init
> > > > 36d3efb87d monitor: move the cur_mon hack deeper for QMP
> > > > bf3e493a86 qjson: add "opaque" field to JSONMessageParser
> > > > 17367fe7a1 monitor: move skip_flush into monitor_data_init
> > > > 0c98d4baa4 qobject: let object_property_get_str() use new API
> > > > aa4b973dd5 qobject: introduce qobject_get_try_str()
> > > > 981ccebc1e qobject: introduce qstring_get_try_str()
> > > > d40ba38085 char-io: fix possible race on IOWatchPoll
> > > > 
> > > > === OUTPUT BEGIN ===
> > > > Checking PATCH 1/27: char-io: fix possible race on IOWatchPoll...
> > > > Checking PATCH 2/27: qobject: introduce qstring_get_try_str()...
> > > > Checking PATCH 3/27: qobject: introduce qobject_get_try_str()...
> > > > Checking PATCH 4/27: qobject: let object_property_get_str() use new API...
> > > > Checking PATCH 5/27: monitor: move skip_flush into monitor_data_init...
> > > > Checking PATCH 6/27: qjson: add "opaque" field to JSONMessageParser...
> > > > Checking PATCH 7/27: monitor: move the cur_mon hack deeper for QMP...
> > > > Checking PATCH 8/27: monitor: unify global init...
> > > > Checking PATCH 9/27: monitor: let mon_list be tail queue...
> > > > Checking PATCH 10/27: monitor: create monitor dedicate iothread...
> > > > Checking PATCH 11/27: monitor: allow to use IO thread for parsing...
> > > > Checking PATCH 12/27: qmp: introduce QMPCapability...
> > > > Checking PATCH 13/27: qmp: negociate QMP capabilities...
> > > > Checking PATCH 14/27: qmp: introduce some capability helpers...
> > > > Checking PATCH 15/27: monitor: introduce monitor_qmp_respond()...
> > > > Checking PATCH 16/27: monitor: let monitor_{suspend|resume} thread safe...
> > > > ERROR: braces {} are necessary for all arms of this statement
> > > > #28: FILE: monitor.c:4014:
> > > > +    if (atomic_dec_fetch(&mon->suspend_cnt) == 0)
> > > > [...]
> > > > 
> > > > ERROR: Missing Signed-off-by: line(s)
> > > 
> > > Will add it in next post.  I still haven't found a good way to let
> > > magit add this line for me every time automatically.
> > 
> > If you don't want to type "git commit -s" all the time you could use the
> > "format.signOff = on" git-config(1) variable.
> 
> I tried it, but it seems to be for when formatting patches rather than
> putting that s-o-b line into commit messages.  I think what I wanted
> was something like commit.signOff but after some searches I found that
> idea was proposed but rejected somehow:
> 
> http://git.661346.n2.nabble.com/PATCH-Add-a-commit-signoff-configuration-variable-to-always-use-signoff-td1886619.html#a2093996
> 
> So I think I'll just live with my current approach.  Anway, thanks for
> the hint!

I think what you could better use here is a checkpatch.pl hook, either before
committing or before posting. I personally prefer the checkpatch.pl check in the
pre-publish-tag hook of git-publish so I can freely do dirty commits that
doesn't need s-o-b lines when I'm working on the branch, but still remember to
add it before posting.

Fam

On Thu, Nov 16, 2017 at 01:39:51PM +0800, Fam Zheng wrote:

[...]

> I think what you could better use here is a checkpatch.pl hook, either before
> committing or before posting. I personally prefer the checkpatch.pl check in the
> pre-publish-tag hook of git-publish so I can freely do dirty commits that
> doesn't need s-o-b lines when I'm working on the branch, but still remember to
> add it before posting.

Noted. :)

-- 
Peter Xu