[Qemu-commits] [qemu/qemu] 53dd40: qemu-iotests/109: Fix lock race condition

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 53dd4015acb48bad5aee5ba707848370dc442077
      
https://github.com/qemu/qemu/commit/53dd4015acb48bad5aee5ba707848370dc442077
  Author: Cleber Rosa <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M tests/qemu-iotests/109
    M tests/qemu-iotests/109.out

  Log Message:
  -----------
  qemu-iotests/109: Fix lock race condition

A race condition is currently present between the clean up attempt of
the QEMU process and the execution of qemu-img.  The actual (bad)
output is:

 -Warning: Image size mismatch!
 -Images are identical.
 +qemu-img: Could not open '<build_dir>/tests/qemu-iotests/scratch/t.raw': 
Failed to get "consistent read" lock
 +Is another process using the image?

A KILL signal is sent to the QEMU process, but qemu-img may begin to
run before the QEMU process is really gone.  qemu-img will then
attempt to open the TEST_IMG file before it can secure a lock on it.

This attempts a more graceful shutdown, and waits for the QEMU process
to exit.

Signed-off-by: Cleber Rosa <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 795be0621a643f3d103d112dfcbddee2992f5035
      
https://github.com/qemu/qemu/commit/795be0621a643f3d103d112dfcbddee2992f5035
  Author: Alberto Garcia <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/quorum.c

  Log Message:
  -----------
  quorum: Set sectors-count to 0 when reporting a flush error

The QUORUM_REPORT_BAD event has fields to report the sector in which
the error was detected and the number of affected sectors starting
from that one. This is important for read and write errors, but not
for flush errors.

For flush errors the current code reports the total size of the disk
image. That is however not useful information in this case. Moreover,
the bdrv_getlength() call can fail, and there's no good way of
handling that failure.

Since we're reporting useless information and we cannot even guarantee
to do it in a consistent way, this patch changes the code to report 0
instead in all cases.

Reported-by: Markus Armbruster <address@hidden>
Signed-off-by: Alberto Garcia <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 3f910692c287e1c611c00e763ebeb95ed0e017f8
      
https://github.com/qemu/qemu/commit/3f910692c287e1c611c00e763ebeb95ed0e017f8
  Author: Jeff Cody <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/vhdx-log.c
    M block/vhdx.c

  Log Message:
  -----------
  block/vhdx: check error return of bdrv_getlength()

Calls to bdrv_getlength() were not checking for error.  In vhdx.c, this
can lead to truncating an image file, so it is a definite bug.  In
vhdx-log.c, the path for improper behavior is less clear, but it is best
to check in any case.

Some minor code movement of the log_guid intialization, as well.

Reported-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Jeff Cody <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 27539ac53154cf9da6990ce9bec1064d37cf2b1d
      
https://github.com/qemu/qemu/commit/27539ac53154cf9da6990ce9bec1064d37cf2b1d
  Author: Jeff Cody <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/vhdx-log.c
    M block/vhdx.c

  Log Message:
  -----------
  block/vhdx: check for offset overflow to bdrv_truncate()

VHDX uses uint64_t types for most offsets, following the VHDX spec.
However, bdrv_truncate() takes an int64_t value for the truncating
offset.  Check for overflow before calling bdrv_truncate().

While we are here, replace the bit shifting with QEMU_ALIGN_UP as well.

N.B.: For a compliant image this is not an issue, as the maximum VHDX
image size is defined per the spec to be 64TB.

Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Kevin Wolf <address@hidden>
Signed-off-by: Jeff Cody <address@hidden>
Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: c6572fa0d2b81bc3a9ca5716f975f2bf59c62e6c
      
https://github.com/qemu/qemu/commit/c6572fa0d2b81bc3a9ca5716f975f2bf59c62e6c
  Author: Jeff Cody <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/vhdx-log.c

  Log Message:
  -----------
  block/vhdx: check error return of bdrv_flush()

Reported-by: Kevin Wolf <address@hidden>
Signed-off-by: Jeff Cody <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 95d729835f3ceeed977eaf326a7ebb92788dee6d
      
https://github.com/qemu/qemu/commit/95d729835f3ceeed977eaf326a7ebb92788dee6d
  Author: Jeff Cody <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/vhdx-log.c

  Log Message:
  -----------
  block/vhdx: check error return of bdrv_truncate()

Signed-off-by: Jeff Cody <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 82346685b8cc0537819a3cf7fe2303b55b5f825f
      
https://github.com/qemu/qemu/commit/82346685b8cc0537819a3cf7fe2303b55b5f825f
  Author: Paolo Bonzini <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M include/block/block_int.h

  Log Message:
  -----------
  block: drop bdrv_set_key from BlockDriver

This is not used anymore since c01c214b69 ("block: remove all encryption
handling APIs", 2017-07-11).

Signed-off-by: Paolo Bonzini <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


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

  Changed paths:
    M block/null.c
    M tests/qemu-iotests/136

  Log Message:
  -----------
  block/null: Remove 'filename' option

This option was only added to allow 'null-co://' and 'null-aio://' as
filenames, its value never served any actual purpose and was ignored.
Nevertheless it was accepted as '-drive driver=null,filename=foo'.

The correct way to enable the protocol prefixes (and that without adding
a useless -drive option) is implementing .bdrv_parse_filename. This is
what this patch does.

Technically, this is an incompatible change, but the null block driver
is only used for benchmarking, testing and debugging, and an option
without effect isn't likely to be used by anyone anyway, so no bad
effects are to be expected.

Reported-by: Markus Armbruster <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>


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

  Changed paths:
    M block/vmdk.c

  Log Message:
  -----------
  vmdk: Fix error handling/reporting of vmdk_check

Errors from the callees must be captured and propagated to our caller,
ensure this for both find_extent() and bdrv_getlength().

Reported-by: Markus Armbruster <address@hidden>
Signed-off-by: Fam Zheng <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


  Commit: 70d9110b440e5252f637d54b5efd33bbc6237c1f
      
https://github.com/qemu/qemu/commit/70d9110b440e5252f637d54b5efd33bbc6237c1f
  Author: Denis V. Lunev <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/file-posix.c

  Log Message:
  -----------
  block: respect error code from bdrv_getlength in handle_aiocb_write_zeroes

Original idea beyond the code in question was the following: we have failed
to write zeroes with fallocate(FALLOC_FL_ZERO_RANGE) as the simplest
approach and via fallocate(FALLOC_FL_PUNCH_HOLE)/fallocate(0). We have the
only chance now: if the request comes beyond end of the file. Thus we
should calculate file length and respect the error code from that op.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Markus Armbruster <address@hidden>
CC: Kevin Wolf <address@hidden>
CC: Max Reitz <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: d8b83e37c381cf86ecd907301b7dcc65baaa0aea
      
https://github.com/qemu/qemu/commit/d8b83e37c381cf86ecd907301b7dcc65baaa0aea
  Author: Denis V. Lunev <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/parallels.c

  Log Message:
  -----------
  parallels: respect error code of bdrv_getlength() in allocate_clusters()

If we can not get the file length, the state of BDS is broken completely.
Return error to the caller.

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Markus Armbruster <address@hidden>
CC: Kevin Wolf <address@hidden>
CC: Max Reitz <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: e5e6268348972aaf415d7931bbd808b3fdba6cb1
      
https://github.com/qemu/qemu/commit/e5e6268348972aaf415d7931bbd808b3fdba6cb1
  Author: Denis V. Lunev <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/parallels.c

  Log Message:
  -----------
  parallels: drop check that bdrv_truncate() is working

This would be actually strange and error prone. If truncate() nowadays
will fail, there is something fatally wrong. Let's check for that during
the actual work.

The only fallback case is when the file is not zero initialized. In this
case we should switch to preallocation via fallocate().

Signed-off-by: Denis V. Lunev <address@hidden>
CC: Markus Armbruster <address@hidden>
CC: Kevin Wolf <address@hidden>
CC: Max Reitz <address@hidden>
CC: Stefan Hajnoczi <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


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

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Fix order in bdrv_replace_child()

Commit 8ee03995 refactored the code incorrectly and broke the release of
permissions on the old BDS. Instead of changing the permissions to the
new required values after removing the old BDS from the list of
children, it only re-obtains the permissions it already had.

Change the order of operations so that the old BDS is removed again
before calculating the new required permissions.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>


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

  Changed paths:
    M block.c
    M include/block/block.h

  Log Message:
  -----------
  block: Allow reopen rw without BDRV_O_ALLOW_RDWR

BDRV_O_ALLOW_RDWR is a flag that tells whether qemu can internally
reopen a node read-write temporarily because the user requested
read-write for the top-level image, but qemu decided that read-only is
enough for this node (a backing file).

bdrv_reopen() is different, it is also used for cases where the user
changed their mind and wants to update the options. There is no reason
to forbid making a node read-write in that case.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>


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

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: Set BDRV_O_ALLOW_RDWR during rw reopen

Reopening an image should be consistent with opening it, so we should
set BDRV_O_ALLOW_RDWR for any image that is reopened read-write like in
bdrv_open_inherit().

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>


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

  Changed paths:
    M qemu-io-cmds.c

  Log Message:
  -----------
  qemu-io: Allow reopen read-write

This allows qemu-iotests to test the switch between read-only and
read-write mode for block devices.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>


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

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

  Log Message:
  -----------
  qemu-iotests: Test reopen between read-only and read-write

This serves as a regression test for the bugs that were just fixed for
bdrv_reopen() between read-only and read-write mode.

Signed-off-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Reviewed-by: John Snow <address@hidden>


  Commit: 113fe792fd4931dd0538f03859278b8719ee4fa2
      
https://github.com/qemu/qemu/commit/113fe792fd4931dd0538f03859278b8719ee4fa2
  Author: Jeff Cody <address@hidden>
  Date:   2017-08-08 (Tue, 08 Aug 2017)

  Changed paths:
    M block/nfs.c

  Log Message:
  -----------
  block/nfs: fix mutex assertion in nfs_file_close()

Commit c096358e747e88fc7364e40e3c354ee0bb683960 introduced assertion
checks for when qemu_mutex() functions are called without the
corresponding qemu_mutex_init() having initialized the mutex.

This uncovered a latent bug in qemu's nfs driver - in
nfs_client_close(), the NFSClient structure is overwritten with zeros,
prior to the mutex being destroyed.

Go ahead and destroy the mutex in nfs_client_close(), and change where
we call qemu_mutex_init() so that it is correctly balanced.

There are also a couple of memory leaks obscured by the memset, so this
fixes those as well.

Finally, we should be able to get rid of the memset(), as it isn't
necessary.

Cc: address@hidden
Signed-off-by: Jeff Cody <address@hidden>
Reviewed-by: Peter Lieven <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Reviewed-by: John Snow <address@hidden>
Signed-off-by: Kevin Wolf <address@hidden>


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

  Changed paths:
    M block.c
    M block/file-posix.c
    M block/nfs.c
    M block/null.c
    M block/parallels.c
    M block/quorum.c
    M block/vhdx-log.c
    M block/vhdx.c
    M block/vmdk.c
    M include/block/block.h
    M include/block/block_int.h
    M qemu-io-cmds.c
    M tests/qemu-iotests/109
    M tests/qemu-iotests/109.out
    M tests/qemu-iotests/136
    A tests/qemu-iotests/187
    A tests/qemu-iotests/187.out
    M tests/qemu-iotests/group

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging

Block layer patches for 2.10.0-rc2

# gpg: Signature made Tue 08 Aug 2017 14:56:15 BST
# gpg:                using RSA key 0x7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <address@hidden>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74  56FE 7F09 B272 C88F 2FD6

* remotes/kevin/tags/for-upstream:
  block/nfs: fix mutex assertion in nfs_file_close()
  qemu-iotests: Test reopen between read-only and read-write
  qemu-io: Allow reopen read-write
  block: Set BDRV_O_ALLOW_RDWR during rw reopen
  block: Allow reopen rw without BDRV_O_ALLOW_RDWR
  block: Fix order in bdrv_replace_child()
  parallels: drop check that bdrv_truncate() is working
  parallels: respect error code of bdrv_getlength() in allocate_clusters()
  block: respect error code from bdrv_getlength in handle_aiocb_write_zeroes
  vmdk: Fix error handling/reporting of vmdk_check
  block/null: Remove 'filename' option
  block: drop bdrv_set_key from BlockDriver
  block/vhdx: check error return of bdrv_truncate()
  block/vhdx: check error return of bdrv_flush()
  block/vhdx: check for offset overflow to bdrv_truncate()
  block/vhdx: check error return of bdrv_getlength()
  quorum: Set sectors-count to 0 when reporting a flush error
  qemu-iotests/109: Fix lock race condition

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


Compare: https://github.com/qemu/qemu/compare/b4174c4b08a7...53b080fa83c3