[Qemu-commits] [qemu/qemu] dad394: nbd: Fix trace message for disconnect

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: dad3946ec64164b1855e09991be0dfc4358298b4
      
https://github.com/qemu/qemu/commit/dad3946ec64164b1855e09991be0dfc4358298b4
  Author: Eric Blake <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M nbd/common.c

  Log Message:
  -----------
  nbd: Fix trace message for disconnect

NBD_CMD_DISC is a disconnect request, not a data discard request.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: cbaddb25b20060fa0b0a2a46d5ccca65cffd1a6f
      
https://github.com/qemu/qemu/commit/cbaddb25b20060fa0b0a2a46d5ccca65cffd1a6f
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M tests/qemu-iotests/093

  Log Message:
  -----------
  qemu-iotests: step clock after each test iteration

The 093 throttling test submits twice as many requests as the throttle
limit in order to ensure that we reach the limit.  The remaining
requests are left in-flight at the end of each test iteration.

Commit 452589b6b47e8dc6353df257fc803dfc1383bed8 ("vl.c/exit: pause cpus
before closing block devices") exposed a hang in 093.  This happens
because requests are still in flight when QEMU terminates but
QEMU_CLOCK_VIRTUAL time is frozen.  bdrv_drain_all() hangs forever since
throttled requests cannot complete.

Step the clock at the end of each test iteration so in-flight requests
actually finish.  This solves the hang and is cleaner than leaving tests
in-flight.

Note that this could also be "fixed" by disabling throttling when drives
are closed in QEMU.  That approach has two issues:

1. We must drain requests before disabling throttling, so the hang
   cannot be easily avoided!

2. Any time QEMU disables throttling internally there is a chance that
   malicious users can abuse the code path to bypass throttling limits.

Therefore it makes more sense to fix the test case than to modify QEMU.

Signed-off-by: Stefan Hajnoczi <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 80adf54ecc3b456828f3d6fe71eeda5572369bb2
      
https://github.com/qemu/qemu/commit/80adf54ecc3b456828f3d6fe71eeda5572369bb2
  Author: Fam Zheng <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M stubs/Makefile.objs
    A stubs/change-state-handler.c

  Log Message:
  -----------
  stubs: Add vm state change handler stubs

They will be used by BlockBackend code in block-obj-y, which doesn't
always get linked with common-obj-y. Add stubs to keep ld happy.

Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 3dff24f2dffc5f3aa46dc014122012848bd7959d
      
https://github.com/qemu/qemu/commit/3dff24f2dffc5f3aa46dc014122012848bd7959d
  Author: Kevin Wolf <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache

The "inactive" state of BDS affects whether the permissions can be
granted, we must call bdrv_invalidate_cache before bdrv_set_perm to
support "-incoming defer" case.

Reported-by: Christian Ehrhardt <address@hidden>
Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 5f7772c4d0cf32f4e779fcd5a69ae4dae24aeebf
      
https://github.com/qemu/qemu/commit/5f7772c4d0cf32f4e779fcd5a69ae4dae24aeebf
  Author: Fam Zheng <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M block/block-backend.c

  Log Message:
  -----------
  block-backend: Defer shared_perm tightening migration completion

As in the case of nbd_export_new(), bdrv_invalidate_cache() can be
called when migration is still in progress. In this case we are not
ready to tighten the shared permissions fenced by blk->disable_perm.

Defer to a VM state change handler.

Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: dd7fdaad654d7484b66410b4b002b14644396587
      
https://github.com/qemu/qemu/commit/dd7fdaad654d7484b66410b4b002b14644396587
  Author: Fam Zheng <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    A tests/qemu-iotests/192
    A tests/qemu-iotests/192.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  iotests: Add non-shared storage migration case 192

Signed-off-by: Fam Zheng <address@hidden>
Message-Id: <address@hidden>
Tested-by: Eric Blake <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 72b6ffc76653214b69a94a7b1643ff80df134486
      
https://github.com/qemu/qemu/commit/72b6ffc76653214b69a94a7b1643ff80df134486
  Author: Eric Blake <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M block/nbd-client.c
    M block/nbd-client.h

  Log Message:
  -----------
  nbd-client: Fix regression when server sends garbage

When we switched NBD to use coroutines for qemu 2.9 (in particular,
commit a12a712a), we introduced a regression: if a server sends us
garbage (such as a corrupted magic number), we quit the read loop
but do not stop sending further queued commands, resulting in the
client hanging when it never reads the response to those additional
commands.  In qemu 2.8, we properly detected that the server is no
longer reliable, and cancelled all existing pending commands with
EIO, then tore down the socket so that all further command attempts
get EPIPE.

Restore the proper behavior of quitting (almost) all communication
with a broken server: Once we know we are out of sync or otherwise
can't trust the server, we must assume that any further incoming
data is unreliable and therefore end all pending commands with EIO,
and quit trying to send any further commands.  As an exception, we
still (try to) send NBD_CMD_DISC to let the server know we are going
away (in part, because it is easier to do that than to further
refactor nbd_teardown_connection, and in part because it is the
only command where we do not have to wait for a reply).

Based on a patch by Vladimir Sementsov-Ogievskiy.

A malicious server can be created with the following hack,
followed by setting NBD_SERVER_DEBUG to a non-zero value in the
environment when running qemu-nbd:

| --- a/nbd/server.c
| +++ b/nbd/server.c
| @@ -919,6 +919,17 @@ static int nbd_send_reply(QIOChannel *ioc, NBDReply 
*reply, Error **errp)
|      stl_be_p(buf + 4, reply->error);
|      stq_be_p(buf + 8, reply->handle);
|
| +    static int debug;
| +    static int count;
| +    if (!count++) {
| +        const char *str = getenv("NBD_SERVER_DEBUG");
| +        if (str) {
| +            debug = atoi(str);
| +        }
| +    }
| +    if (debug && !(count % debug)) {
| +        buf[0] = 0;
| +    }
|      return nbd_write(ioc, buf, sizeof(buf), errp);
|  }

Reported-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


  Commit: 09920c53549f6097a007bc6081b6f03b9dc13d9c
      
https://github.com/qemu/qemu/commit/09920c53549f6097a007bc6081b6f03b9dc13d9c
  Author: Peter Maydell <address@hidden>
  Date:   2017-08-15 (Tue, 15 Aug 2017)

  Changed paths:
    M block/block-backend.c
    M block/nbd-client.c
    M block/nbd-client.h
    M nbd/common.c
    M nbd/server.c
    M stubs/Makefile.objs
    A stubs/change-state-handler.c
    M tests/qemu-iotests/093
    A tests/qemu-iotests/192
    A tests/qemu-iotests/192.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-08-15' into 
staging

nbd patches for 2017-08-15

- Eric Blake: nbd: Fix trace message for disconnect
- Stefan Hajnoczi: qemu-iotests: step clock after each test iteration
- Fam Zheng: 0/4 block: Fix non-shared storage migration
- Eric Blake: nbd-client: Fix regression when server sends garbage

# gpg: Signature made Tue 15 Aug 2017 16:06:02 BST
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <address@hidden>"
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<address@hidden>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2017-08-15:
  nbd-client: Fix regression when server sends garbage
  iotests: Add non-shared storage migration case 192
  block-backend: Defer shared_perm tightening migration completion
  nbd: Fix order of bdrv_set_perm and bdrv_invalidate_cache
  stubs: Add vm state change handler stubs
  qemu-iotests: step clock after each test iteration
  nbd: Fix trace message for disconnect

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/72b384f4a7d1...09920c53549f