[Qemu-commits] [qemu/qemu] 48557b: util/hbitmap: strict hbitmap_reset

[Peter Maydell]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 48557b138383aaf69c2617ca9a88bfb394fc50ec
      
https://github.com/qemu/qemu/commit/48557b138383aaf69c2617ca9a88bfb394fc50ec
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M include/qemu/hbitmap.h
    M tests/test-hbitmap.c
    M util/hbitmap.c

  Log Message:
  -----------
  util/hbitmap: strict hbitmap_reset

hbitmap_reset has an unobvious property: it rounds requested region up.
It may provoke bugs, like in recently fixed write-blocking mode of
mirror: user calls reset on unaligned region, not keeping in mind that
there are possible unrelated dirty bytes, covered by rounded-up region
and information of this unrelated "dirtiness" will be lost.

Make hbitmap_reset strict: assert that arguments are aligned, allowing
only one exception when @start + @count == hb->orig_size. It's needed
to comfort users of hbitmap_next_dirty_area, which cares about
hb->orig_size.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-Id: <address@hidden>
[Maintainer edit: Max's suggestions from on-list. --js]
[Maintainer edit: Eric's suggestion for aligned macro. --js]
Signed-off-by: John Snow <address@hidden>


  Commit: 85cc8a4f6b515404448de7f22db403807152b657
      
https://github.com/qemu/qemu/commit/85cc8a4f6b515404448de7f22db403807152b657
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block.c
    M block/dirty-bitmap.c

  Log Message:
  -----------
  block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c

block/dirty-bitmap.c seems to be more appropriate for it and
bdrv_remove_persistent_dirty_bitmap already in it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: b56a1e31759b750e111b4dd35171007bf493fc89
      
https://github.com/qemu/qemu/commit/b56a1e31759b750e111b4dd35171007bf493fc89
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/dirty-bitmap.c
    M block/qcow2-bitmap.c
    M block/qcow2.h
    M blockdev.c
    M include/block/block_int.h
    M include/block/dirty-bitmap.h

  Log Message:
  -----------
  block/dirty-bitmap: return int from bdrv_remove_persistent_dirty_bitmap

It's more comfortable to not deal with local_err.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: d2c3080e41fd2c9bc36c996cc9d33804462ba803
      
https://github.com/qemu/qemu/commit/d2c3080e41fd2c9bc36c996cc9d33804462ba803
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/dirty-bitmap.c
    M block/qcow2-bitmap.c
    M block/qcow2.c
    M block/qcow2.h
    M blockdev.c
    M include/block/block_int.h

  Log Message:
  -----------
  block/qcow2: proper locking on bitmap add/remove paths

qmp_block_dirty_bitmap_add and do_block_dirty_bitmap_remove do acquire
aio context since 0a6c86d024c52b. But this is not enough: we also must
lock qcow2 mutex when access in-image metadata. Especially it concerns
freeing qcow2 clusters.

To achieve this, move qcow2_can_store_new_dirty_bitmap and
qcow2_remove_persistent_dirty_bitmap to coroutine context.

Since we work in coroutines in correct aio context, we don't need
context acquiring in blockdev.c anymore, drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 767db3aad8a491c332870ae14d8cfad506a78662
      
https://github.com/qemu/qemu/commit/767db3aad8a491c332870ae14d8cfad506a78662
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/dirty-bitmap.c
    M include/block/dirty-bitmap.h

  Log Message:
  -----------
  block/dirty-bitmap: drop meta

Drop meta bitmaps, as they are unused.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 5deb6cbd1fae10197623a74b0734a468ec80c609
      
https://github.com/qemu/qemu/commit/5deb6cbd1fae10197623a74b0734a468ec80c609
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/backup.c
    M block/block-copy.c
    M block/dirty-bitmap.c
    M block/mirror.c
    M block/qcow2-bitmap.c
    M blockdev.c
    M include/block/dirty-bitmap.h
    M migration/block-dirty-bitmap.c
    M migration/block.c

  Log Message:
  -----------
  block/dirty-bitmap: add bs link

Add bs field to BdrvDirtyBitmap structure. Drop BlockDriverState
parameter from bitmap APIs where possible.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
[Rebased on top of block-copy. --js]
Signed-off-by: John Snow <address@hidden>


  Commit: 1e638301604ab8e05c54bee59584c7cd81ebe4d2
      
https://github.com/qemu/qemu/commit/1e638301604ab8e05c54bee59584c7cd81ebe4d2
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/dirty-bitmap.c

  Log Message:
  -----------
  block/dirty-bitmap: drop BdrvDirtyBitmap.mutex

mutex field is just a pointer to bs->dirty_bitmap_mutex, so no needs
to store it in BdrvDirtyBitmap when we have bs pointer in it (since
previous patch).

Drop mutex field. Constantly use bdrv_dirty_bitmaps_lock/unlock in
block/dirty-bitmap.c to make it more obvious that it's not per-bitmap
lock. Still, for simplicity, leave bdrv_dirty_bitmap_lock/unlock
functions as an external API.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: ef9041a7b8c46001c8c44eab3eac9920739d9b36
      
https://github.com/qemu/qemu/commit/ef9041a7b8c46001c8c44eab3eac9920739d9b36
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block.c
    M block/dirty-bitmap.c
    M block/qcow2-bitmap.c
    M include/block/dirty-bitmap.h
    M migration/block-dirty-bitmap.c

  Log Message:
  -----------
  block/dirty-bitmap: refactor bdrv_dirty_bitmap_next

bdrv_dirty_bitmap_next is always used in same pattern. So, split it
into _next and _first, instead of combining two functions into one and
add FOR_EACH_DIRTY_BITMAP macro.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 859443b0fb7eba258325f75f6444270c50008e65
      
https://github.com/qemu/qemu/commit/859443b0fb7eba258325f75f6444270c50008e65
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

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

  Log Message:
  -----------
  block: switch reopen queue from QSIMPLEQ to QTAILQ

We'll need reverse-foreach in the following commit, QTAILQ support it,
so move to QTAILQ.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: fcd6a4f42deef7e9f28c7b82f7d340200fb7846e
      
https://github.com/qemu/qemu/commit/fcd6a4f42deef7e9f28c7b82f7d340200fb7846e
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block.c

  Log Message:
  -----------
  block: reverse order for reopen commits

It's needed to fix reopening qcow2 with bitmaps to RW. Currently it
can't work, as qcow2 needs write access to file child, to mark bitmaps
in-image with IN_USE flag. But usually children goes after parents in
reopen queue and file child is still RO on qcow2 reopen commit. Reverse
reopen order to fix it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Acked-by: Max Reitz <address@hidden>
Acked-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 5752f89a788537cccc7a8e05904fb1b4ddf5eb43
      
https://github.com/qemu/qemu/commit/5752f89a788537cccc7a8e05904fb1b4ddf5eb43
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

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

  Log Message:
  -----------
  iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW

Reopening bitmaps to RW was broken prior to previous commit. Check that
it works now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: f88676c149d3c7cde2710c137aa5ef31e7f92eb4
      
https://github.com/qemu/qemu/commit/f88676c149d3c7cde2710c137aa5ef31e7f92eb4
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/dirty-bitmap.c
    M block/qcow2-bitmap.c
    M include/block/dirty-bitmap.h

  Log Message:
  -----------
  block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps

Firstly, no reason to optimize failure path. Then, function name is
ambiguous: it checks for readonly and similar things, but someone may
think that it will ignore normal bitmaps which was just unchanged, and
this is in bad relation with the fact that we should drop IN_USE flag
for unchanged bitmaps in the image.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: bd429a884cebde851e0fcfd144a8f8ca7185bd09
      
https://github.com/qemu/qemu/commit/bd429a884cebde851e0fcfd144a8f8ca7185bd09
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/qcow2-bitmap.c
    M block/qcow2.h

  Log Message:
  -----------
  block/qcow2-bitmap: drop qcow2_reopen_bitmaps_rw_hint()

The function is unused, drop it.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 644ddbb7540cfb69941f33629281819f53770666
      
https://github.com/qemu/qemu/commit/644ddbb7540cfb69941f33629281819f53770666
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/qcow2-bitmap.c
    M block/qcow2.c
    M block/qcow2.h

  Log Message:
  -----------
  block/qcow2-bitmap: do not remove bitmaps on reopen-ro

qcow2_reopen_bitmaps_ro wants to store bitmaps and then mark them all
readonly. But the latter don't work, as
qcow2_store_persistent_dirty_bitmaps removes bitmaps after storing.
It's OK for inactivation but bad idea for reopen-ro. And this leads to
the following bug:

Assume we have persistent bitmap 'bitmap0'.
Create external snapshot
  bitmap0 is stored and therefore removed
Commit snapshot
  now we have no bitmaps
Do some writes from guest (*)
  they are not marked in bitmap
Shutdown
Start
  bitmap0 is loaded as valid, but it is actually broken! It misses
  writes (*)
Incremental backup
  it will be inconsistent

So, let's stop removing bitmaps on reopen-ro. But don't rejoice:
reopening bitmaps to rw is broken too, so the whole scenario will not
work after this patch and we can't enable corresponding test cases in
260 iotests still. Reopening bitmaps rw will be fixed in the following
patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 5d9388d4b23a5302acb53052add07514285b2976
      
https://github.com/qemu/qemu/commit/5d9388d4b23a5302acb53052add07514285b2976
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

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

  Log Message:
  -----------
  iotests: add test 260 to check bitmap life after snapshot + commit

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-id: address@hidden
[Maintainer edit: removed 260 from auto group per Peter Maydell. --js]
Signed-off-by: John Snow <address@hidden>


  Commit: f6333cbf8bc78c5fed3311c893597ad8d65be235
      
https://github.com/qemu/qemu/commit/f6333cbf8bc78c5fed3311c893597ad8d65be235
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M block/qcow2-bitmap.c

  Log Message:
  -----------
  block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw

- Correct check for write access to file child, and in correct place
  (only if we want to write).
- Support reopen rw -> rw (which will be used in following commit),
  for example, !bdrv_dirty_bitmap_readonly() is not a corruption if
  bitmap is marked IN_USE in the image.
- Consider unexpected bitmap as a corruption and check other
  combinations of in-image and in-RAM bitmaps.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 4dd09f6223b25096d731615d279751e415224e3e
      
https://github.com/qemu/qemu/commit/4dd09f6223b25096d731615d279751e415224e3e
  Author: Vladimir Sementsov-Ogievskiy <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

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

  Log Message:
  -----------
  qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit

The only reason I can imagine for this strange code at the very-end of
bdrv_reopen_commit is the fact that bs->read_only updated after
calling drv->bdrv_reopen_commit in bdrv_reopen_commit. And in the same
time, prior to previous commit, qcow2_reopen_bitmaps_rw did a wrong
check for being writable, when actually it only need writable file
child not self.

So, as it's fixed, let's move things to correct place.

Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: John Snow <address@hidden>
Acked-by: Max Reitz <address@hidden>
Message-id: address@hidden
Signed-off-by: John Snow <address@hidden>


  Commit: 4a189fdfafe388f05cd7f5b58e2654b7cfd6dfb0
      
https://github.com/qemu/qemu/commit/4a189fdfafe388f05cd7f5b58e2654b7cfd6dfb0
  Author: John Snow <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M MAINTAINERS

  Log Message:
  -----------
  MAINTAINERS: Add Vladimir as a reviewer for bitmaps

I already try to make sure all bitmaps patches have been reviewed by both
Red Hat and Virtuozzo anyway, so this formalizes the arrangement.

Fam meanwhile is no longer as active, so I am removing him as a co-maintainer
simply to reflect the current practice.

Signed-off-by: John Snow <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-id: address@hidden


  Commit: 3264ffced3d087bbe72d759639ef84fd5bd924cc
      
https://github.com/qemu/qemu/commit/3264ffced3d087bbe72d759639ef84fd5bd924cc
  Author: John Snow <address@hidden>
  Date:   2019-10-17 (Thu, 17 Oct 2019)

  Changed paths:
    M blockdev.c
    M qapi/block-core.json
    M qemu-deprecated.texi

  Log Message:
  -----------
  dirty-bitmaps: remove deprecated autoload parameter

This parameter has been deprecated since 2.12.0 and is eligible for
removal. Remove this parameter as it is actually completely ignored;
let's not give false hope.

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


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

  Changed paths:
    M MAINTAINERS
    M block.c
    M block/backup.c
    M block/block-copy.c
    M block/dirty-bitmap.c
    M block/mirror.c
    M block/qcow2-bitmap.c
    M block/qcow2.c
    M block/qcow2.h
    M blockdev.c
    M include/block/block.h
    M include/block/block_int.h
    M include/block/dirty-bitmap.h
    M include/qemu/hbitmap.h
    M migration/block-dirty-bitmap.c
    M migration/block.c
    M qapi/block-core.json
    M qemu-deprecated.texi
    M tests/qemu-iotests/165
    M tests/qemu-iotests/165.out
    A tests/qemu-iotests/260
    A tests/qemu-iotests/260.out
    M tests/qemu-iotests/group
    M tests/test-hbitmap.c
    M util/hbitmap.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' into 
staging

pull request

# gpg: Signature made Thu 17 Oct 2019 22:54:14 BST
# gpg:                using RSA key F9B7ABDBBCACDF95BE76CBD07DEF8106AAFC390E
# gpg: Good signature from "John Snow (John Huston) <address@hidden>" [full]
# Primary key fingerprint: FAEB 9711 A12C F475 812F  18F2 88A9 064D 1835 61EB
#      Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76  CBD0 7DEF 8106 AAFC 390E

* remotes/jnsnow/tags/bitmaps-pull-request:
  dirty-bitmaps: remove deprecated autoload parameter
  MAINTAINERS: Add Vladimir as a reviewer for bitmaps
  qcow2-bitmap: move bitmap reopen-rw code to qcow2_reopen_commit
  block/qcow2-bitmap: fix and improve qcow2_reopen_bitmaps_rw
  iotests: add test 260 to check bitmap life after snapshot + commit
  block/qcow2-bitmap: do not remove bitmaps on reopen-ro
  block/qcow2-bitmap: drop qcow2_reopen_bitmaps_rw_hint()
  block/qcow2-bitmap: get rid of bdrv_has_changed_persistent_bitmaps
  iotests: add test-case to 165 to test reopening qcow2 bitmaps to RW
  block: reverse order for reopen commits
  block: switch reopen queue from QSIMPLEQ to QTAILQ
  block/dirty-bitmap: refactor bdrv_dirty_bitmap_next
  block/dirty-bitmap: drop BdrvDirtyBitmap.mutex
  block/dirty-bitmap: add bs link
  block/dirty-bitmap: drop meta
  block/qcow2: proper locking on bitmap add/remove paths
  block/dirty-bitmap: return int from bdrv_remove_persistent_dirty_bitmap
  block: move bdrv_can_store_new_dirty_bitmap to block/dirty-bitmap.c
  util/hbitmap: strict hbitmap_reset

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


Compare: https://github.com/qemu/qemu/compare/51cd65b18fbb...ca32646d4140