[Qemu-commits] [qemu/qemu] 61cc87: nbd: Advertise multi-conn for shared read-only con...

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 61cc872456492da24e36cfbd668abfbe127c739f
      
https://github.com/qemu/qemu/commit/61cc872456492da24e36cfbd668abfbe127c739f
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M blockdev-nbd.c
    M docs/interop/nbd.txt
    M include/block/nbd.h
    M nbd/server.c
    M qemu-nbd.c
    M tests/qemu-iotests/223.out

  Log Message:
  -----------
  nbd: Advertise multi-conn for shared read-only connections

The NBD specification defines NBD_FLAG_CAN_MULTI_CONN, which can be
advertised when the server promises cache consistency between
simultaneous clients (basically, rules that determine what FUA and
flush from one client are able to guarantee for reads from another
client).  When we don't permit simultaneous clients (such as qemu-nbd
without -e), the bit makes no sense; and for writable images, we
probably have a lot more work before we can declare that actions from
one client are cache-consistent with actions from another.  But for
read-only images, where flush isn't changing any data, we might as
well advertise multi-conn support.  What's more, advertisement of the
bit makes it easier for clients to determine if 'qemu-nbd -e' was in
use, where a second connection will succeed rather than hang until the
first client goes away.

This patch affects qemu as server in advertising the bit.  We may want
to consider patches to qemu as client to attempt parallel connections
for higher throughput by spreading the load over those connections
when a server advertises multi-conn, but for now sticking to one
connection per nbd:// BDS is okay.

See also: https://bugzilla.redhat.com/1708300
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[eblake: tweak blockdev-nbd.c to not request shared when writable,
fix iotest 233]
Reviewed-by: John Snow <address@hidden>


  Commit: df18c04edf57b55737ed5125bffe71a8196938a0
      
https://github.com/qemu/qemu/commit/df18c04edf57b55737ed5125bffe71a8196938a0
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M block/nbd.c
    M nbd/client.c
    M nbd/server.c

  Log Message:
  -----------
  nbd: Use g_autofree in a few places

Thanks to our recent move to use glib's g_autofree, I can join the
bandwagon.  Getting rid of gotos is fun ;)

There are probably more places where we could register cleanup
functions and get rid of more gotos; this patch just focuses on the
labels that existed merely to call g_free.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel P. Berrangé <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>


  Commit: 5de47735c79a030edc3e6258ab5476b630c61765
      
https://github.com/qemu/qemu/commit/5de47735c79a030edc3e6258ab5476b630c61765
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M nbd/client.c
    M nbd/trace-events
    M tests/qemu-iotests/233.out

  Log Message:
  -----------
  nbd: Tolerate more errors to structured reply request

A server may have a reason to reject a request for structured replies,
beyond just not recognizing them as a valid request; similarly, it may
have a reason for rejecting a request for a meta context.  It doesn't
hurt us to continue talking to such a server; otherwise 'qemu-nbd
--list' of such a server fails to display all available details about
the export.

Encountered when temporarily tweaking nbdkit to reply with
NBD_REP_ERR_POLICY.  Present since structured reply support was first
added (commit d795299b reused starttls handling, but starttls is
different in that we can't fall back to other behavior on any error).

Note that for an unencrypted client trying to connect to a server that
requires encryption, this defers the point of failure to when we
finally execute a strict command (such as NBD_OPT_GO or NBD_OPT_LIST),
now that the intermediate NBD_OPT_STRUCTURED_REPLY does not diagnose
NBD_REP_ERR_TLS_REQD as fatal; but as the protocol eventually gets us
to a command where we can't continue onwards, the changed error
message doesn't cause any security concerns.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Daniel P. Berrangé <address@hidden>
[eblake: fix iotest 233]


  Commit: 294682cc3a7c5b3d1b2d45b686e2d8523750ce64
      
https://github.com/qemu/qemu/commit/294682cc3a7c5b3d1b2d45b686e2d8523750ce64
  Author: Andrey Shinkevich <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M block/file-posix.c
    M block/io.c

  Log Message:
  -----------
  block: workaround for unaligned byte range in fallocate()

Revert the commit 118f99442d 'block/io.c: fix for the allocation failure'
and use better error handling for file systems that do not support
fallocate() for an unaligned byte range. Allow falling back to pwrite
in case fallocate() returns EINVAL.

Suggested-by: Kevin Wolf <address@hidden>
Suggested-by: Eric Blake <address@hidden>
Signed-off-by: Andrey Shinkevich <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Denis V. Lunev <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: 0c61ebb0cdf66343c45f1337d8cd985b6c4ab25e
      
https://github.com/qemu/qemu/commit/0c61ebb0cdf66343c45f1337d8cd985b6c4ab25e
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M qemu-doc.texi

  Log Message:
  -----------
  docs: Update preferred NBD device syntax

Mention the preferred URI form, especially since NBD is trying to
standardize that form: https://lists.debian.org/nbd/2019/06/msg00012.html

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


  Commit: dbb38caac54af7cd2432bec715da6ad33b3bdc22
      
https://github.com/qemu/qemu/commit/dbb38caac54af7cd2432bec715da6ad33b3bdc22
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M blockdev-nbd.c
    M include/block/nbd.h
    M nbd/server.c
    M qemu-nbd.c
    M tests/qemu-iotests/223.out

  Log Message:
  -----------
  nbd: Improve per-export flag handling in server

When creating a read-only image, we are still advertising support for
TRIM and WRITE_ZEROES to the client, even though the client should not
be issuing those commands.  But seeing this requires looking across
multiple functions:

All callers to nbd_export_new() passed a single flag based solely on
whether the export allows writes.  Later, we then pass a constant set
of flags to nbd_negotiate_options() (namely, the set of flags which we
always support, at least for writable images), which is then further
dynamically modified with NBD_FLAG_SEND_DF based on client requests
for structured options.  Finally, when processing NBD_OPT_EXPORT_NAME
or NBD_OPT_EXPORT_GO we bitwise-or the original caller's flag with the
runtime set of flags we've built up over several functions.

Let's refactor things to instead compute a baseline of flags as soon
as possible which gets shared between multiple clients, in
nbd_export_new(), and changing the signature for the callers to pass
in a simpler bool rather than having to figure out flags.  We can then
get rid of the 'myflags' parameter to various functions, and instead
refer to client for everything we need (we still have to perform a
bitwise-OR for NBD_FLAG_SEND_DF during NBD_OPT_EXPORT_NAME and
NBD_OPT_EXPORT_GO, but it's easier to see what is being computed).
This lets us quit advertising senseless flags for read-only images, as
well as making the next patch for exposing FAST_ZERO support easier to
write.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
[eblake: improve commit message, update iotest 223]


  Commit: 0a4795455ceeebdca999a60583dd42434f2ec0d1
      
https://github.com/qemu/qemu/commit/0a4795455ceeebdca999a60583dd42434f2ec0d1
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M docs/interop/nbd.txt
    M include/block/nbd.h
    M nbd/common.c
    M nbd/server.c
    M qemu-nbd.c

  Log Message:
  -----------
  nbd: Prepare for NBD_CMD_FLAG_FAST_ZERO

Commit fe0480d6 and friends added BDRV_REQ_NO_FALLBACK as a way to
avoid wasting time on a preliminary write-zero request that will later
be rewritten by actual data, if it is known that the write-zero
request will use a slow fallback; but in doing so, could not optimize
for NBD.  The NBD specification is now considering an extension that
will allow passing on those semantics; this patch updates the new
protocol bits and 'qemu-nbd --list' output to recognize the bit, as
well as the new errno value possible when using the new flag; while
upcoming patches will improve the client to use the feature when
present, and the server to advertise support for it.

The NBD spec recommends (but not requires) that ENOTSUP be avoided for
all but failures of a fast zero (the only time it is mandatory to
avoid an ENOTSUP failure is when fast zero is supported but not
requested during write zeroes; the questionable use is for ENOTSUP to
other actions like a normal write request).  However, clients that get
an unexpected ENOTSUP will either already be treating it the same as
EINVAL, or may appreciate the extra bit of information.  We were
equally loose for returning EOVERFLOW in more situations than
recommended by the spec, so if it turns out to be a problem in
practice, a later patch can tighten handling for both error codes.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
[eblake: tweak commit message, also handle EOPNOTSUPP]


  Commit: f061656cc355161f9984237298c45762ce5187d3
      
https://github.com/qemu/qemu/commit/f061656cc355161f9984237298c45762ce5187d3
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd: Implement client use of NBD FAST_ZERO

The client side is fairly straightforward: if the server advertised
fast zero support, then we can map that to BDRV_REQ_NO_FALLBACK
support.  A server that advertises FAST_ZERO but not WRITE_ZEROES
is technically broken, but we can ignore that situation as it does
not change our behavior.

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


  Commit: b491dbb7f8e09ef864770c205a3b5bce6c5c1881
      
https://github.com/qemu/qemu/commit/b491dbb7f8e09ef864770c205a3b5bce6c5c1881
  Author: Eric Blake <address@hidden>
  Date:   2019-09-05 (Thu, 05 Sep 2019)

  Changed paths:
    M nbd/server.c
    M tests/qemu-iotests/223.out
    M tests/qemu-iotests/233.out

  Log Message:
  -----------
  nbd: Implement server use of NBD FAST_ZERO

The server side is fairly straightforward: we can always advertise
support for detection of fast zero, and implement it by mapping the
request to the block layer BDRV_REQ_NO_FALLBACK.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
[eblake: update iotests 223, 233]


  Commit: 019217c3b34f22b1c14cdf652d87dbf54cdc43a2
      
https://github.com/qemu/qemu/commit/019217c3b34f22b1c14cdf652d87dbf54cdc43a2
  Author: Peter Maydell <address@hidden>
  Date:   2019-09-06 (Fri, 06 Sep 2019)

  Changed paths:
    M block/file-posix.c
    M block/io.c
    M block/nbd.c
    M blockdev-nbd.c
    M docs/interop/nbd.txt
    M include/block/nbd.h
    M nbd/client.c
    M nbd/common.c
    M nbd/server.c
    M nbd/trace-events
    M qemu-doc.texi
    M qemu-nbd.c
    M tests/qemu-iotests/223.out
    M tests/qemu-iotests/233.out

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2019-09-05-v2' into 
staging

nbd patches for 2019-09-05

- Advertise NBD_FLAG_CAN_MULTI_CONN on readonly images
- Tolerate larger set of server error responses during handshake
- More precision on handling fallocate() failures due to alignment
- Better documentation of NBD connection URIs
- Implement new extension NBD_CMD_FLAG_FAST_ZERO to benefit qemu-img convert

# gpg: Signature made Thu 05 Sep 2019 22:08:17 BST
# gpg:                using RSA key 71C2CC22B1C4602927D2F3AAA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <address@hidden>" [full]
# gpg:                 aka "Eric Blake (Free Software Programmer) 
<address@hidden>" [full]
# gpg:                 aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2019-09-05-v2:
  nbd: Implement server use of NBD FAST_ZERO
  nbd: Implement client use of NBD FAST_ZERO
  nbd: Prepare for NBD_CMD_FLAG_FAST_ZERO
  nbd: Improve per-export flag handling in server
  docs: Update preferred NBD device syntax
  block: workaround for unaligned byte range in fallocate()
  nbd: Tolerate more errors to structured reply request
  nbd: Use g_autofree in a few places
  nbd: Advertise multi-conn for shared read-only connections

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


Compare: https://github.com/qemu/qemu/compare/90b1e3afd332...019217c3b34f