[Qemu-commits] [qemu/qemu] 80c7c2: nbd: Don't take address of fields in packed struct...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 80c7c2b00d607221bb43815d2c1951d54229b3ee
      
https://github.com/qemu/qemu/commit/80c7c2b00d607221bb43815d2c1951d54229b3ee
  Author: Peter Maydell <address@hidden>
  Date:   2018-10-03 (Wed, 03 Oct 2018)

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

  Log Message:
  -----------
  nbd: Don't take address of fields in packed structs

Taking the address of a field in a packed struct is a bad idea, because
it might not be actually aligned enough for that pointer type (and
thus cause a crash on dereference on some host architectures). Newer
versions of clang warn about this. Avoid the bug by not using the
"modify in place" byte swapping functions.

This patch was produced with the following spatch script:
@@
expression E;
@@
-be16_to_cpus(&E);
+E = be16_to_cpu(E);
@@
expression E;
@@
-be32_to_cpus(&E);
+E = be32_to_cpu(E);
@@
expression E;
@@
-be64_to_cpus(&E);
+E = be64_to_cpu(E);
@@
expression E;
@@
-cpu_to_be16s(&E);
+E = cpu_to_be16(E);
@@
expression E;
@@
-cpu_to_be32s(&E);
+E = cpu_to_be32(E);
@@
expression E;
@@
-cpu_to_be64s(&E);
+E = cpu_to_be64(E);

Signed-off-by: Peter Maydell <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: rebase, and squash in missed changes]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 2f454defc23e1be78f2a96bad2877ce7829f61b4
      
https://github.com/qemu/qemu/commit/2f454defc23e1be78f2a96bad2877ce7829f61b4
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-10-03 (Wed, 03 Oct 2018)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd/server: fix NBD_CMD_CACHE

We should not go to structured-read branch on CACHE command, fix that.

Bug introduced in bc37b06a5cde24 "nbd/server: introduce NBD_CMD_CACHE"
with the whole feature and affects 3.0.0 release.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
CC: address@hidden
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: commit message typo fix]
Signed-off-by: Eric Blake <address@hidden>


  Commit: f7812df77d7830c6b375066a4e656f3b79232c13
      
https://github.com/qemu/qemu/commit/f7812df77d7830c6b375066a4e656f3b79232c13
  Author: Eric Blake <address@hidden>
  Date:   2018-10-03 (Wed, 03 Oct 2018)

  Changed paths:
    M qemu-nbd.c

  Log Message:
  -----------
  qemu-nbd: Document --tls-creds

Commit 145614a1 introduced --tls-creds and documented it in
qemu-nbd.texi, but forgot to document it in 'qemu-nbd --help'.

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


  Commit: f5cd0bb5174dcd6e8c160d7992fb89f09f264ef0
      
https://github.com/qemu/qemu/commit/f5cd0bb5174dcd6e8c160d7992fb89f09f264ef0
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-10-03 (Wed, 03 Oct 2018)

  Changed paths:
    M qemu-nbd.c

  Log Message:
  -----------
  qemu-nbd: drop old-style negotiation

Use new-style negotiation always, with default "" (empty) export name
if it is not specified with '-x' option.

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

Furthermore, if a client that only speaks oldstyle still needs to
communicate to qemu, nbdkit remains available to perform the
translation between the two protocols.

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


  Commit: 7f7dfe2a53446072c136d349e3150c84d322b2bc
      
https://github.com/qemu/qemu/commit/7f7dfe2a53446072c136d349e3150c84d322b2bc
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2018-10-03 (Wed, 03 Oct 2018)

  Changed paths:
    M blockdev-nbd.c
    M include/block/nbd.h
    M nbd/server.c
    M qemu-nbd.c

  Log Message:
  -----------
  nbd/server: drop old-style negotiation

After the previous commit, nbd_client_new's first parameter is always
NULL. Let's drop it with all corresponding old-style negotiation code
path which is unreachable now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: re-wrap short line]
Signed-off-by: Eric Blake <address@hidden>


  Commit: df91328adab8490367776d2b21b35d790a606120
      
https://github.com/qemu/qemu/commit/df91328adab8490367776d2b21b35d790a606120
  Author: Denis V. Lunev <address@hidden>
  Date:   2018-10-04 (Thu, 04 Oct 2018)

  Changed paths:
    M include/block/nbd.h

  Log Message:
  -----------
  nbd: fix NBD_FLAG_SEND_CACHE value

Commit bc37b06a5 added NBD_CMD_CACHE support, but used the wrong value
for NBD_FLAG_SEND_CACHE flag for negotiation. That commit picked bit 8,
which had already been assigned by the NBD specification to mean
NBD_FLAG_CAN_MULTI_CONN, and which was already implemented in the
Linux kernel as a part of stable userspace-kernel API since 4.10:

"bit 8, NBD_FLAG_CAN_MULTI_CONN: Indicates that the server operates
entirely without cache, or that the cache it uses is shared among all
connections to the given device. In particular, if this flag is
present, then the effects of NBD_CMD_FLUSH and NBD_CMD_FLAG_FUA
MUST be visible across all connections when the server sends its reply
to that command to the client. In the absense of this flag, clients
SHOULD NOT multiplex their commands over more than one connection to
the export.
...
bit 10, NBD_FLAG_SEND_CACHE: documents that the server understands
NBD_CMD_CACHE; however, note that server implementations exist
which support the command without advertising this bit, and
conversely that this bit does not guarantee that the command will
succeed or have an impact."

Consequences:
- a client trying to use NBD_CMD_CACHE per the NBD spec will not
see the feature as available from a qemu 3.0 server (not fatal,
clients already have to be prepared for caching to not exist)
- a client accidentally coded to the qemu 3.0 bit value instead
of following the spec may interpret NBD_CMD_CACHE as being available
when it is not (probably not fatal, the spec says the server should
gracefully fail unknown commands, and that clients of NBD_CMD_CACHE
should be prepared for failure even when the feature is advertised);
such clients are unlikely (perhaps only in unreleased Virtuozzo code),
and will disappear over time
- a client prepared to use multiple connections based on
NBD_FLAG_CAN_MULTI_CONN may cause data corruption when it assumes
that caching is consistent when in reality qemu 3.0 did not have
a consistent cache. Partially mitigated by using read-only
connections (where nothing needs to be flushed, so caching is
indeed consistent) or when using qemu-nbd with the default -e 1
(at most one client at a time); visible only when using -e 2 or
more for a writable export.

Thus the commit fixes negotiation flag in QEMU according to the
specification.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Vladimir Sementsov-Ogievskiy <address@hidden>
CC: Valery Vdovin <address@hidden>
CC: Eric Blake <address@hidden>
CC: Paolo Bonzini <address@hidden>
CC: address@hidden
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: enhance commit message, add defines for unimplemented flags]
Signed-off-by: Eric Blake <address@hidden>


  Commit: d21ee59ae5b6e3e7001b877c10c286d111bcf5b7
      
https://github.com/qemu/qemu/commit/d21ee59ae5b6e3e7001b877c10c286d111bcf5b7
  Author: Peter Maydell <address@hidden>
  Date:   2018-10-05 (Fri, 05 Oct 2018)

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

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

nbd patches for 2018-10-03

Fix bugs in NBD_CMD_CACHE, drop support for oldstyle NBD server,
minor build and doc fixes

- Denis V. Lunev: nbd: fix NBD_CMD_CACHE negitiation... [retitled]
- Vladimir Sementsov-Ogievskiy: 0/2 server: drop old-style negotiation
- Eric Blake: qemu-nbd: Document --tls-creds
- Vladimir Sementsov-Ogievskiy: nbd/server: fix NBD_CMD_CACHE
- Peter Maydell: nbd: Don't take address of fields in packed structs

# gpg: Signature made Thu 04 Oct 2018 15:19:32 BST
# 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-10-03-v2:
  nbd: fix NBD_FLAG_SEND_CACHE value
  nbd/server: drop old-style negotiation
  qemu-nbd: drop old-style negotiation
  qemu-nbd: Document --tls-creds
  nbd/server: fix NBD_CMD_CACHE
  nbd: Don't take address of fields in packed structs

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


Compare: https://github.com/qemu/qemu/compare/638ad4ad17ae...d21ee59ae5b6
      **NOTE:** This service has been marked for deprecation: 
https://developer.github.com/changes/2018-04-25-github-services-deprecation/

      Functionality will be removed from GitHub.com on January 31st, 2019.