Hi all!

I started this as a follow-up to
"block: Attempt on fixing 030-reported errors" by Hanna.

In Hanna's series I really like how bs->children handling moved to
.attach/.detach handlers.

.file and .backing are kind of "shortcuts" or "links" to some elementes
of this list, they duplicate the information. So they should be updated
in the same place to be in sync.

On the way to this target, I do also the following cleanups:

 - establish, which restrictions we have on how much children of
 different roles should node has, and which of the should be linked in
 .file / .backing. Add documentation and assertions.

 - drop all the complicated logic around passing pointer to bs->backing
 / bs->file  (BdrvChild **c), so that the field be automatically
 updated. Now they are natively automatically updated in
 .attach/.detach, so the rest of the code becomes simpler.

 - now .file / .backing are updated ONLY in .attach / .detach, no other
 code modify these fields

Vladimir Sementsov-Ogievskiy (14):
  block: BlockDriver: add .filtered_child_is_backing field
  block: introduce bdrv_open_file_child() helper
  block/blklogwrites: don't care to remove bs->file child on failure
  test-bdrv-graph-mod: update test_parallel_perm_update test case
  tests-bdrv-drain: bdrv_replace_test driver: declare supports_backing
  test-bdrv-graph-mod: fix filters to be filters
  block: document connection between child roles and
    bs->backing/bs->file
  block/snapshot: stress that we fallback to primary child
  Revert "block: Let replace_child_noperm free children"
  Revert "block: Let replace_child_tran keep indirect pointer"
  Revert "block: Restructure remove_file_or_backing_child()"
  Revert "block: Pass BdrvChild ** to replace_child_noperm"
  block: Manipulate bs->file / bs->backing pointers in .attach/.detach
  block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr

 include/block/block.h            |  45 +++++
 include/block/block_int.h        |  30 ++-
 block.c                          | 335 ++++++++++---------------------
 block/blkdebug.c                 |   9 +-
 block/blklogwrites.c             |  11 +-
 block/blkreplay.c                |   7 +-
 block/blkverify.c                |   9 +-
 block/bochs.c                    |   7 +-
 block/cloop.c                    |   7 +-
 block/commit.c                   |   1 +
 block/copy-before-write.c        |   9 +-
 block/copy-on-read.c             |   9 +-
 block/crypto.c                   |  11 +-
 block/dmg.c                      |   7 +-
 block/filter-compress.c          |   6 +-
 block/mirror.c                   |   1 +
 block/parallels.c                |   7 +-
 block/preallocate.c              |   9 +-
 block/qcow.c                     |   6 +-
 block/qcow2.c                    |   8 +-
 block/qed.c                      |   8 +-
 block/raw-format.c               |   4 +-
 block/replication.c              |   8 +-
 block/snapshot.c                 |  60 ++----
 block/throttle.c                 |   8 +-
 block/vdi.c                      |   7 +-
 block/vhdx.c                     |   7 +-
 block/vmdk.c                     |   7 +-
 block/vpc.c                      |   7 +-
 tests/unit/test-bdrv-drain.c     |  11 +-
 tests/unit/test-bdrv-graph-mod.c |  94 ++++++---
 31 files changed, 343 insertions(+), 412 deletions(-)

-- 
2.31.1

Ping

03.12.2021 23:25, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> I started this as a follow-up to
> "block: Attempt on fixing 030-reported errors" by Hanna.
> 
> In Hanna's series I really like how bs->children handling moved to
> .attach/.detach handlers.
> 
> .file and .backing are kind of "shortcuts" or "links" to some elementes
> of this list, they duplicate the information. So they should be updated
> in the same place to be in sync.
> 
> On the way to this target, I do also the following cleanups:
> 
>   - establish, which restrictions we have on how much children of
>   different roles should node has, and which of the should be linked in
>   .file / .backing. Add documentation and assertions.
> 
>   - drop all the complicated logic around passing pointer to bs->backing
>   / bs->file  (BdrvChild **c), so that the field be automatically
>   updated. Now they are natively automatically updated in
>   .attach/.detach, so the rest of the code becomes simpler.
> 
>   - now .file / .backing are updated ONLY in .attach / .detach, no other
>   code modify these fields
> 
> Vladimir Sementsov-Ogievskiy (14):
>    block: BlockDriver: add .filtered_child_is_backing field
>    block: introduce bdrv_open_file_child() helper
>    block/blklogwrites: don't care to remove bs->file child on failure
>    test-bdrv-graph-mod: update test_parallel_perm_update test case
>    tests-bdrv-drain: bdrv_replace_test driver: declare supports_backing
>    test-bdrv-graph-mod: fix filters to be filters
>    block: document connection between child roles and
>      bs->backing/bs->file
>    block/snapshot: stress that we fallback to primary child
>    Revert "block: Let replace_child_noperm free children"
>    Revert "block: Let replace_child_tran keep indirect pointer"
>    Revert "block: Restructure remove_file_or_backing_child()"
>    Revert "block: Pass BdrvChild ** to replace_child_noperm"
>    block: Manipulate bs->file / bs->backing pointers in .attach/.detach
>    block/snapshot: drop indirection around bdrv_snapshot_fallback_ptr
> 
>   include/block/block.h            |  45 +++++
>   include/block/block_int.h        |  30 ++-
>   block.c                          | 335 ++++++++++---------------------
>   block/blkdebug.c                 |   9 +-
>   block/blklogwrites.c             |  11 +-
>   block/blkreplay.c                |   7 +-
>   block/blkverify.c                |   9 +-
>   block/bochs.c                    |   7 +-
>   block/cloop.c                    |   7 +-
>   block/commit.c                   |   1 +
>   block/copy-before-write.c        |   9 +-
>   block/copy-on-read.c             |   9 +-
>   block/crypto.c                   |  11 +-
>   block/dmg.c                      |   7 +-
>   block/filter-compress.c          |   6 +-
>   block/mirror.c                   |   1 +
>   block/parallels.c                |   7 +-
>   block/preallocate.c              |   9 +-
>   block/qcow.c                     |   6 +-
>   block/qcow2.c                    |   8 +-
>   block/qed.c                      |   8 +-
>   block/raw-format.c               |   4 +-
>   block/replication.c              |   8 +-
>   block/snapshot.c                 |  60 ++----
>   block/throttle.c                 |   8 +-
>   block/vdi.c                      |   7 +-
>   block/vhdx.c                     |   7 +-
>   block/vmdk.c                     |   7 +-
>   block/vpc.c                      |   7 +-
>   tests/unit/test-bdrv-drain.c     |  11 +-
>   tests/unit/test-bdrv-graph-mod.c |  94 ++++++---
>   31 files changed, 343 insertions(+), 412 deletions(-)
> 


-- 
Best regards,
Vladimir