[PATCH v3 00/21] block: Fix check_to_replace_node()

[Max Reitz]

Branch: https://github.com/XanClic/qemu.git fix-can-replace-v3
Branch: https://git.xanclic.moe/XanClic/qemu.git fix-can-replace-v3

v1: https://lists.nongnu.org/archive/html/qemu-block/2019-09/msg01027.html
v2: https://lists.nongnu.org/archive/html/qemu-block/2019-11/msg00370.html


Hi,

For what this series does, see the cover letter of v1 (linked above).


In v3 I’ve addressed Vladimir’s further comments:
- Patch 9: Skip the loop if c == NULL (and also renamed @i to @child_i)

- Patch 10: Drop local_err, check bdrv_child_refresh_perm()’s return
            value instead

- Patch 11: Amended commit message and comment

- Patch 12: Extended to cover the “new” compress filter driver

- Patch 15: Dropped; considering qemu never actually creates loops,
            there is never an unsafe configuration.  Sometimes the user
            may not get what they want, but then maybe this was actually
            what they wanted.  I think the discussion I had with
            Vladimir shows that this issue isn’t as clear-cut as I
            thought, so let’s wait with a “fix” until someone actually
            has a problem with the current behavior.

- Patch 17: Dropped; was superseded by a patch from Thomas.

- Patch 16 (was 18):
  - Moved doc string under def
  - Pointed out that @expected_node may be None, but this only means
    that the leaf must not exist.
  - Contracted asserts
  - %s/path_node/child_name/
  - Kept the prefix /, because, well, why not
  - Kept % for formatting, because I don’t see the advantage of
    .format() – I would have liked to use f strings, but those were only
    introduced in 3.6, and our minimum version is 3.5.

- Patch 17: Added

- Patch 18 (was 19): Dropped removal of self.vm.shutdown(), because
                     that’s done by patch 17 now

- Patch 20 (was 21): Moved doc strings under defs (kept R-b, because
                     that seemed right to do)

- Patch 21 (was 22): Moved doc string under def

- Patch 23: Dropped, because it only tested things changed by the old
            patch 17 (which I dropped, too)


git backport-diff against v2:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/21:[----] [--] 'blockdev: Allow external snapshots everywhere'
002/21:[----] [--] 'blockdev: Allow resizing everywhere'
003/21:[----] [--] 'block: Drop bdrv_is_first_non_filter()'
004/21:[----] [--] 'iotests: Let 041 use -blockdev for quorum children'
005/21:[----] [--] 'quorum: Fix child permissions'
006/21:[----] [--] 'block: Add bdrv_recurse_can_replace()'
007/21:[----] [--] 'blkverify: Implement .bdrv_recurse_can_replace()'
008/21:[----] [--] 'quorum: Store children in own structure'
009/21:[0014] [FC] 'quorum: Add QuorumChild.to_be_replaced'
010/21:[0011] [FC] 'quorum: Implement .bdrv_recurse_can_replace()'
011/21:[0002] [FC] 'block: Use bdrv_recurse_can_replace()'
012/21:[0009] [FC] 'block: Remove bdrv_recurse_is_first_non_filter()'
013/21:[----] [--] 'mirror: Double-check immediately before replacing'
014/21:[----] [--] 'quorum: Stop marking it as a filter'
015/21:[----] [--] 'iotests: Use complete_and_wait() in 155'
016/21:[0053] [FC] 'iotests: Add VM.assert_block_path()'
017/21:[down] 'iotests/041: Drop superfluous shutdowns'
018/21:[0001] [FC] 'iotests: Resolve TODOs in 041'
019/21:[----] [--] 'iotests: Use self.image_len in TestRepairQuorum'
020/21:[0016] [FC] 'iotests: Add tests for invalid Quorum @replaces'
021/21:[0006] [FC] 'iotests: Check that @replaces can replace filters'


Max Reitz (21):
  blockdev: Allow external snapshots everywhere
  blockdev: Allow resizing everywhere
  block: Drop bdrv_is_first_non_filter()
  iotests: Let 041 use -blockdev for quorum children
  quorum: Fix child permissions
  block: Add bdrv_recurse_can_replace()
  blkverify: Implement .bdrv_recurse_can_replace()
  quorum: Store children in own structure
  quorum: Add QuorumChild.to_be_replaced
  quorum: Implement .bdrv_recurse_can_replace()
  block: Use bdrv_recurse_can_replace()
  block: Remove bdrv_recurse_is_first_non_filter()
  mirror: Double-check immediately before replacing
  quorum: Stop marking it as a filter
  iotests: Use complete_and_wait() in 155
  iotests: Add VM.assert_block_path()
  iotests/041: Drop superfluous shutdowns
  iotests: Resolve TODOs in 041
  iotests: Use self.image_len in TestRepairQuorum
  iotests: Add tests for invalid Quorum @replaces
  iotests: Check that @replaces can replace filters

 block.c                       |  85 +++++++++---------
 block/blkverify.c             |  20 ++---
 block/copy-on-read.c          |   9 --
 block/filter-compress.c       |   9 --
 block/mirror.c                |  14 ++-
 block/quorum.c                | 160 ++++++++++++++++++++++++++--------
 block/replication.c           |   7 --
 block/throttle.c              |   8 --
 blockdev.c                    |  10 ---
 include/block/block.h         |   5 --
 include/block/block_int.h     |  16 ++--
 tests/qemu-iotests/041        | 138 +++++++++++++++++++++++++----
 tests/qemu-iotests/041.out    |   4 +-
 tests/qemu-iotests/155        |   7 +-
 tests/qemu-iotests/iotests.py |  56 ++++++++++++
 15 files changed, 375 insertions(+), 173 deletions(-)

-- 
2.24.1