• P
    monitor: flush qmp responses when CLOSED · c73a843b
    Peter Xu 提交于
    Previously we clean up the queues when we got CLOSED event.  It was used
    to make sure we won't send leftover replies/events of a old client to a
    new client which makes perfect sense. However this will also drop the
    replies/events even if the output port of the previous chardev backend
    is still open, which can lead to missing of the last replies/events.
    Now this patch does an extra operation to flush the response queue
    before cleaning up.
    
    In most cases, a QMP session will be based on a bidirectional channel (a
    TCP port, for example, we read/write to the same socket handle), so in
    port and out port of the backend chardev are fundamentally the same
    port. In these cases, it does not really matter much on whether we'll
    flush the response queue since flushing will fail anyway.  However there
    can be cases where in & out ports of the QMP monitor's backend chardev
    are separated.  Here is an example:
    
      cat $QMP_COMMANDS | qemu -qmp stdio ... | filter_commands
    
    In this case, the backend is fd-typed, and it is connected to stdio
    where in port is stdin and out port is stdout.  Now if we drop all the
    events on the response queue then filter_command process might miss some
    events that it might expect.  The thing is that, when stdin closes,
    stdout might still be there alive!
    
    In practice, I encountered SHUTDOWN event missing when running test with
    iotest 087 with Out-Of-Band enabled.  Here is one of the ways that this
    can happen (after "quit" command is executed and QEMU quits the main
    loop):
    
    1. [main thread] QEMU queues a SHUTDOWN event into response queue.
    
    2. "cat" terminates (to distinguish it from the animal, I quote it).
    
    3. [monitor iothread] QEMU's monitor iothread reads EOF from stdin.
    
    4. [monitor iothread] QEMU's monitor iothread calls the CLOSED event
       hook for the monitor, which will destroy the response queue of the
       monitor, then the SHUTDOWN event is dropped.
    
    5. [main thread] QEMU's main thread cleans up the monitors in
       monitor_cleanup().  When trying to flush pending responses, it sees
       nothing.  SHUTDOWN is lost forever.
    
    Note that before the monitor iothread was introduced, step [4]/[5] could
    never happen since the main loop was the only place to detect the EOF
    event of stdin and run the CLOSED event hooks.  Now things can happen in
    parallel in the iothread.
    
    Without this patch, iotest 087 will have ~10% chance to miss the
    SHUTDOWN event and fail when with Out-Of-Band enabled:
    
      --- /home/peterx/git/qemu/tests/qemu-iotests/087.out
      +++ /home/peterx/git/qemu/bin/tests/qemu-iotests/087.out.bad
      @@ -8,7 +8,6 @@
      {"return": {}}
      {"error": {"class": "GenericError", "desc": "'node-name' must be
      specified for the root node"}}
      {"return": {}}
      -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
    
      === Duplicate ID ===
      @@ -53,7 +52,6 @@
      {"return": {}}
      {"return": {}}
      {"return": {}}
    
      -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
    
    This patch fixes the problem.
    
    Fixes: 6d2d563f ("qmp: cleanup qmp queues properly", 2018-03-27)
    Suggested-by: NMarkus Armbruster <armbru@redhat.com>
    Signed-off-by: NPeter Xu <peterx@redhat.com>
    Message-Id: <20180620073223.31964-4-peterx@redhat.com>
    Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
    [Commit message and a comment touched up]
    Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
    c73a843b
monitor.c 132.4 KB