[Qemu-commits] [qemu/qemu] 73e064: nbd: Fix regression with multiple meta contexts

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 73e064ccf09d908febc83761addcc6e76feabf78
      
https://github.com/qemu/qemu/commit/73e064ccf09d908febc83761addcc6e76feabf78
  Author: Eric Blake <address@hidden>
  Date:   2020-02-26 (Wed, 26 Feb 2020)

  Changed paths:
    M nbd/server.c

  Log Message:
  -----------
  nbd: Fix regression with multiple meta contexts

Detected by a hang in the libnbd testsuite.  If a client requests
multiple meta contexts (both base:allocation and qemu:dirty-bitmap:x)
at the same time, our attempt to silence a false-positive warning
about a potential uninitialized variable introduced botched logic: we
were short-circuiting the second context, and never sending the
NBD_REPLY_FLAG_DONE.  Combining two 'if' into one 'if/else' in
bdf200a55 was wrong (I'm a bit embarrassed that such a change was my
initial suggestion after the v1 patch, then I did not review the v2
patch that actually got committed). Revert that, and instead silence
the false positive warning by replacing 'return ret' with 'return 0'
(the value it always has at that point in the code, even though it
eluded the deduction abilities of the robot that reported the false
positive).

Fixes: bdf200a5535
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Laurent Vivier <address@hidden>


  Commit: 2485f22fe9211b2e80970f83199b320eee211319
      
https://github.com/qemu/qemu/commit/2485f22fe9211b2e80970f83199b320eee211319
  Author: Eric Blake <address@hidden>
  Date:   2020-02-26 (Wed, 26 Feb 2020)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  nbd-client: Support leading / in NBD URI

The NBD URI specification [1] states that only one leading slash at
the beginning of the URI path component is stripped, not all such
slashes.  This becomes important to a patch I just proposed to nbdkit
[2], which would allow the exportname to select a file embedded within
an ext2 image: ext2fs demands an absolute pathname beginning with '/',
and because qemu was inadvertantly stripping it, my nbdkit patch had
to work around the behavior.

[1] https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md
[2] https://www.redhat.com/archives/libguestfs/2020-February/msg00109.html

Note that the qemu bug only affects handling of URIs such as
nbd://host:port//abs/path (where '/abs/path' should be the export
name); it is still possible to use --image-opts and pass the desired
export name with a leading slash directly through JSON even without
this patch.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Ján Tomko <address@hidden>


  Commit: 7f493662be4045146a8f45119d8834c9088a0ad6
      
https://github.com/qemu/qemu/commit/7f493662be4045146a8f45119d8834c9088a0ad6
  Author: Pan Nengyuan <address@hidden>
  Date:   2020-02-26 (Wed, 26 Feb 2020)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: extract the common cleanup code

The BDRVNBDState cleanup code is common in two places, add
nbd_clear_bdrvstate() function to do these cleanups.

Suggested-by: Stefano Garzarella <address@hidden>
Signed-off-by: Pan Nengyuan <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
[eblake: fix compilation error and commit message]
Signed-off-by: Eric Blake <address@hidden>


  Commit: 8198cf5ef0ef98118b4176970d1cd998d93ec849
      
https://github.com/qemu/qemu/commit/8198cf5ef0ef98118b4176970d1cd998d93ec849
  Author: Pan Nengyuan <address@hidden>
  Date:   2020-02-26 (Wed, 26 Feb 2020)

  Changed paths:
    M block/nbd.c

  Log Message:
  -----------
  block/nbd: fix memory leak in nbd_open()

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to
reproduce:

1. run qemu-iotests as follow and check the result with asan:
    ./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
    #1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
    #2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
    #3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
    #4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
    #5 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
    #6 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
    #7 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
    #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
    #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
    #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
    #3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
    #4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
    #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
    #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
    #3 0x56281dab41a3 in qobject_input_type_str_keyval 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
    #4 0x56281dab2ee9 in visit_type_str 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
    #5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
qapi/qapi-visit-sockets.c:141
    #6 0x56281dad17b6 in visit_type_SocketAddress_members 
qapi/qapi-visit-sockets.c:366
    #7 0x56281dad186a in visit_type_SocketAddress qapi/qapi-visit-sockets.c:393
    #8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
    #9 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
    #10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Fixes: 8f071c9db506e03ab
Reported-by: Euler Robot <address@hidden>
Signed-off-by: Pan Nengyuan <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Cc: qemu-stable <address@hidden>
Cc: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Stefano Garzarella <address@hidden>
Signed-off-by: Eric Blake <address@hidden>


  Commit: a7cfd219d5b956fb189e787fc33dd9c54f14cdc9
      
https://github.com/qemu/qemu/commit/a7cfd219d5b956fb189e787fc33dd9c54f14cdc9
  Author: Peter Maydell <address@hidden>
  Date:   2020-02-27 (Thu, 27 Feb 2020)

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

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2020-02-26' into 
staging

nbd patches for 2020-02-26

- ensure multiple meta contexts work
- allow leading / in export names
- fix a failure path memory leak

# gpg: Signature made Thu 27 Feb 2020 01:53:03 GMT
# 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-2020-02-26:
  block/nbd: fix memory leak in nbd_open()
  block/nbd: extract the common cleanup code
  nbd-client: Support leading / in NBD URI
  nbd: Fix regression with multiple meta contexts

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


Compare: https://github.com/qemu/qemu/compare/8b6269c8ec14...a7cfd219d5b9