qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 6eba9f: iotests: Fix stuck NBD process on 33


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 6eba9f: iotests: Fix stuck NBD process on 33
Date: Fri, 16 Mar 2018 07:14:17 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 6eba9f01bbfbd92ddd5520be83520df06805dabb
      
https://github.com/qemu/qemu/commit/6eba9f01bbfbd92ddd5520be83520df06805dabb
  Author: Eric Blake <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M tests/qemu-iotests/033

  Log Message:
  -----------
  iotests: Fix stuck NBD process on 33

Commit afe35cde6 added additional actions to test 33, but forgot
to reset the image between tests.  As a result, './check -nbd 33'
fails because the qemu-nbd process from the first half is still
occupying the port, preventing the second half from starting a
new qemu-nbd process.  Worse, the failure leaves a rogue qemu-nbd
process behind even after the test fails, which causes knock-on
failures to later tests that also want to start qemu-nbd.

Reported-by: Max Reitz <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Max Reitz <address@hidden>


  Commit: 60ace2bacfbadb0cfae265929024dbece1ecf9af
      
https://github.com/qemu/qemu/commit/60ace2bacfbadb0cfae265929024dbece1ecf9af
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: move nbd_co_send_structured_error up

To be reused in nbd_co_send_sparse_read() in the following patch.

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


  Commit: 37e02aebf88f8a12f02457de207b09b607c1e8a8
      
https://github.com/qemu/qemu/commit/37e02aebf88f8a12f02457de207b09b607c1e8a8
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: fix sparse read

In case of io error in nbd_co_send_sparse_read we should not
"goto reply:", as it was a fatal error and the common behavior
is to disconnect in this case. We should not try to send the
client an additional error reply, since we already hit a
channel-io error on our previous attempt to send one.

Fix this by handling block-status error in nbd_co_send_sparse_read,
so nbd_co_send_sparse_read fails only on io error. Then just skip
common "reply:" code path in nbd_trip.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
[eblake: grammar tweaks]
Signed-off-by: Eric Blake <address@hidden>


  Commit: a0d7ce20a94ba66e1d9577429a62d133f1cd19cf
      
https://github.com/qemu/qemu/commit/a0d7ce20a94ba66e1d9577429a62d133f1cd19cf
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: fix: check client->closing before sending reply

Since the unchanged code has just set client->recv_coroutine to
NULL before calling nbd_client_receive_next_request(), we are
spawning a new coroutine unconditionally, but the first thing
that coroutine will do is check for client->closing, making it
a no-op if we have already detected that the client is going
away.  Furthermore, for any error other than EIO (where we
disconnect, which itself sets client->closing), if the client
has already gone away, we'll probably encounter EIO later
in the function and attempt disconnect at that point.  Logically,
as soon as we know the connection is closing, there is no need
to try a likely-to-fail a response or spawn a no-op coroutine.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: squash in further reordering: hoist check before spawning
next coroutine, and document rationale in commit message]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 6a4175997b942d74f95d18433cacd98c264c5dc6
      
https://github.com/qemu/qemu/commit/6a4175997b942d74f95d18433cacd98c264c5dc6
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: refactor nbd_trip: cmd_read and generic reply

nbd_trip has difficult logic when sending replies: it tries to use one
code path for all replies. It is ok for simple replies, but is not
comfortable for structured replies. Also, two types of error (and
corresponding messages in local_err) - fatal (leading to disconnect)
and not-fatal (just to be sent to the client) are difficult to follow.

To make things a bit clearer, the following is done:
 - split CMD_READ logic to separate function. It is the most difficult
   command for now, and it is definitely cramped inside nbd_trip. Also,
   it is difficult to follow CMD_READ logic, shared between
   "case NBD_CMD_READ" and "if"s under "reply:" label.
 - create separate helper function nbd_send_generic_reply() and use it
   both in new nbd_do_cmd_read and for other commands in nbd_trip instead
   of common code-path under "reply:" label in nbd_trip. The helper
   supports an error message, so logic with local_err in nbd_trip is
   simplified.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: grammar tweaks and blank line placement]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 6f302e60931a1f95ea873a8bd6bd8c1edf179ec2
      
https://github.com/qemu/qemu/commit/6f302e60931a1f95ea873a8bd6bd8c1edf179ec2
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: refactor nbd_trip: split out nbd_handle_request

Split out request handling logic.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: touch up blank line placement]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 65529782f8fec531fd114cc0f58399e88202153c
      
https://github.com/qemu/qemu/commit/65529782f8fec531fd114cc0f58399e88202153c
  Author: Eric Blake <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: Honor FUA request on NBD_CMD_TRIM

The NBD spec states that since trim requests can affect disk contents,
then they should allow for FUA semantics just like writes for ensuring
the disk has settled before returning.  As bdrv_[co_]pdiscard() does
not support a flags argument, we can't pass FUA down the block layer
stack, and must therefore emulate it with a flush at the NBD layer.

Note that in all reality, generic well-behaved clients will never
send TRIM+FUA (in fact, qemu as a client never does, and we have no
intention to plumb flags into bdrv_pdiscard).  This is because the
NBD protocol states that it is unspecified to READ a trimmed area
(you might read stale data, all zeroes, or even random unrelated
data) without first rewriting it, and even the experimental
BLOCK_STATUS extension states that TRIM need not affect reported
status.  Thus, in the general case, a client cannot tell the
difference between an arbitrary server that ignores TRIM, a server
that had a power outage without flushing to disk, and a server that
actually affected the disk before returning; so waiting for the
trim actions to flush to disk makes little sense.  However, for a
specific client and server pair, where the client knows the server
treats TRIM'd areas as guaranteed reads-zero, waiting for a flush
makes sense, hence why the protocol documents that FUA is valid on
trim.  So, even though the NBD protocol doesn't have a way for the
server to advertise what effects (if any) TRIM will actually have,
and thus any client that relies on specific effects is probably
in error, we can at least support a client that requests TRIM+FUA.

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


  Commit: d03654eacdf35f436fbe78ca46cea215d5165ab5
      
https://github.com/qemu/qemu/commit/d03654eacdf35f436fbe78ca46cea215d5165ab5
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M block/block-backend.c
    M block/trace-events

  Log Message:
  -----------
  block: let blk_add/remove_aio_context_notifier() tolerate BDS changes

Commit 2019ba0a0197 ("block: Add AioContextNotifier functions to BB")
added blk_add/remove_aio_context_notifier() and implemented them by
passing through the bdrv_*() equivalent.

This doesn't work across bdrv_append(), which detaches child->bs and
re-attaches it to a new BlockDriverState.  When
blk_remove_aio_context_notifier() is called we will access the new BDS
instead of the one where the notifier was added!

>From the point of view of the blk_*() API user, changes to the root BDS
should be transparent.

This patch maintains a list of AioContext notifiers in BlockBackend and
adds/removes them from the BlockDriverState as needed.

Reported-by: Stefano Panella <address@hidden>
Cc: Max Reitz <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 44a8174e0a2919693dc28f19c3ce8eef6d4ac2c3
      
https://github.com/qemu/qemu/commit/44a8174e0a2919693dc28f19c3ce8eef6d4ac2c3
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

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

  Log Message:
  -----------
  iotests: add 208 nbd-server + blockdev-snapshot-sync test case

This test case adds an NBD server export and then invokes
blockdev-snapshot-sync, which changes the BlockDriverState node that the
NBD server's BlockBackend points to.  This is an interesting scenario to
test and exercises the code path fixed by the previous commit.

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


  Commit: 2e425fd56849d36485131b3363ab596d39e0e13b
      
https://github.com/qemu/qemu/commit/2e425fd56849d36485131b3363ab596d39e0e13b
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: add nbd_opt_invalid helper

NBD_REP_ERR_INVALID is often parameter to nbd_opt_drop and it would
be used more in following patches. So, let's add a helper.

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


  Commit: 12296459f4319e5d0a0f720ffc3d52113cde026e
      
https://github.com/qemu/qemu/commit/12296459f4319e5d0a0f720ffc3d52113cde026e
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: add nbd_read_opt_name helper

Add helper to read name in format:

  uint32 len       (<= NBD_MAX_NAME_SIZE)
  len bytes string (not 0-terminated)

The helper will be reused in following patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: grammar fixes, actually check error]
Signed-off-by: Eric Blake <address@hidden>


  Commit: e7b1948d510d2e8805db8efaf704bd82d3966963
      
https://github.com/qemu/qemu/commit/e7b1948d510d2e8805db8efaf704bd82d3966963
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd: BLOCK_STATUS for standard get_block_status function: server part

Minimal realization: only one extent in server answer is supported.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: tweak whitespace, move constant from .h to .c, improve
logic of check_meta_export_name, simplify nbd_negotiate_options
by doing more in nbd_negotiate_meta_queries]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 1e98efc0299977a4b0815c2d0a216ae19881e389
      
https://github.com/qemu/qemu/commit/1e98efc0299977a4b0815c2d0a216ae19881e389
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M block/nbd-client.c

  Log Message:
  -----------
  block/nbd-client: save first fatal error in nbd_iter_error

It is ok, that fatal error hides previous not fatal, but hiding
first fatal error is a bad feature.

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


  Commit: 78a33ab58782efdb206de14fa44ea626e2360bfc
      
https://github.com/qemu/qemu/commit/78a33ab58782efdb206de14fa44ea626e2360bfc
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

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

  Log Message:
  -----------
  nbd: BLOCK_STATUS for standard get_block_status function: client part

Minimal realization: only one extent in server answer is supported.
Flag NBD_CMD_FLAG_REQ_ONE is used to force this behavior.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: grammar tweaks, fix min_block check and 32-bit cap, use -1
instead of errno on failure in nbd_negotiate_simple_meta_context,
ensure that block status makes progress on success]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 02f3a9119955891abb2ead5e0bb9cc2f0af42150
      
https://github.com/qemu/qemu/commit/02f3a9119955891abb2ead5e0bb9cc2f0af42150
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M tests/qemu-iotests/iotests.py

  Log Message:
  -----------
  iotests.py: tiny refactor: move system imports up

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


  Commit: ef6e92280ef90f808adf0032f073331c36f58baf
      
https://github.com/qemu/qemu/commit/ef6e92280ef90f808adf0032f073331c36f58baf
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

  Changed paths:
    M tests/qemu-iotests/iotests.py

  Log Message:
  -----------
  iotests: add file_path helper

Simple way to have auto generated filenames with auto cleanup. Like
FilePath but without using 'with' statement and without additional
indentation of the whole test.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: grammar tweak]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 65374c1aa6263a4e2b566d15a9fd9b2105954a1b
      
https://github.com/qemu/qemu/commit/65374c1aa6263a4e2b566d15a9fd9b2105954a1b
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-03-13 (Tue, 13 Mar 2018)

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

  Log Message:
  -----------
  iotests: new test 209 for NBD BLOCK_STATUS

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


  Commit: 475fe4576f11e9389a188bd5698ae05458c397c2
      
https://github.com/qemu/qemu/commit/475fe4576f11e9389a188bd5698ae05458c397c2
  Author: Peter Maydell <address@hidden>
  Date:   2018-03-16 (Fri, 16 Mar 2018)

  Changed paths:
    M block/block-backend.c
    M block/nbd-client.c
    M block/nbd-client.h
    M block/nbd.c
    M block/trace-events
    M include/block/nbd.h
    M nbd/client.c
    M nbd/server.c
    M tests/qemu-iotests/033
    A tests/qemu-iotests/208
    A tests/qemu-iotests/208.out
    A tests/qemu-iotests/209
    A tests/qemu-iotests/209.out
    M tests/qemu-iotests/group
    M tests/qemu-iotests/iotests.py

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2018-03-13-v2' into 
staging

nbd patches for 2018-03-13

- Eric Blake: iotests: Fix stuck NBD process on 33
- Vladimir Sementsov-Ogievskiy: 0/5 nbd server fixing and refactoring before 
BLOCK_STATUS
- Eric Blake: nbd/server: Honor FUA request on NBD_CMD_TRIM
- Stefan Hajnoczi: 0/2 block: fix nbd-server-stop crash after 
blockdev-snapshot-sync
- Vladimir Sementsov-Ogievskiy: nbd block status base:allocation

# gpg: Signature made Tue 13 Mar 2018 20:48:37 GMT
# gpg:                using RSA key A7A16B4A2527436A
# 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-2018-03-13-v2:
  iotests: new test 209 for NBD BLOCK_STATUS
  iotests: add file_path helper
  iotests.py: tiny refactor: move system imports up
  nbd: BLOCK_STATUS for standard get_block_status function: client part
  block/nbd-client: save first fatal error in nbd_iter_error
  nbd: BLOCK_STATUS for standard get_block_status function: server part
  nbd/server: add nbd_read_opt_name helper
  nbd/server: add nbd_opt_invalid helper
  iotests: add 208 nbd-server + blockdev-snapshot-sync test case
  block: let blk_add/remove_aio_context_notifier() tolerate BDS changes
  nbd/server: Honor FUA request on NBD_CMD_TRIM
  nbd/server: refactor nbd_trip: split out nbd_handle_request
  nbd/server: refactor nbd_trip: cmd_read and generic reply
  nbd/server: fix: check client->closing before sending reply
  nbd/server: fix sparse read
  nbd/server: move nbd_co_send_structured_error up
  iotests: Fix stuck NBD process on 33

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


Compare: https://github.com/qemu/qemu/compare/3788c7b6e56f...475fe4576f11

reply via email to

[Prev in Thread] Current Thread [Next in Thread]