QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH v6 00/42] block: Deal with filters
@ 2019-08-09 16:13 Max Reitz
  2019-08-09 16:13 ` [Qemu-devel] [PATCH v6 01/42] block: Mark commit and mirror as filter drivers Max Reitz
                   ` (41 more replies)
  0 siblings, 42 replies; 136+ messages in thread
From: Max Reitz @ 2019-08-09 16:13 UTC (permalink / raw)
  To: qemu-block
  Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz

Hi,

When we introduced filters, we did it a bit casually.  Sure, we talked a
lot about them before, but that was mostly discussion about where
implicit filters should be added to the graph (note that we currently
only have two implicit filters, those being mirror and commit).  But in
the end, we really just designated some drivers filters (Quorum,
blkdebug, etc.) and added some specifically (throttle, COR), without
really looking through the block layer to see where issues might occur.

It turns out vast areas of the block layer just don’t know about filters
and cannot really handle them.  Many cases will work in practice, in
others, well, too bad, you cannot use some feature because some part
deep inside the block layer looks at your filters and thinks they are
format nodes.

This is one reason why this series is needed.  Over time (since v1), a
second reason has made its way in:

bs->file is not necessarily the place where a node’s data is stored.
qcow2 now has external data files, and currently there is no way for the
general block layer to know that the data is not stored in bs->file.
Right now, I do not think that has any real consequences (all functions
that need access to the actual data storage file should only do so as a
fallback if the driver does not provide some functionality, but qcow2
should provide it all), but it still shows that we need some way to let
the general block layer know about such data files.  (Also, I will need
this for v1 of my “Inquire images’ rotational info” series.)

I won’t go on and on about this series now, I think the patches pretty
much speak for themselves now.  If the cover letter gets too long,
nobody reads it anyway (see previous versions).


*** I’ve based this series on John’s bitmap branch, which I’ve rebased
    on my block-next branch. ***

I’ve pushed the patches here:

  https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
  https://github.com/XanClic/qemu child-access-functions-v6

I’ve also pushed the base branch to each of those repos, it’s
“child-access-functions-base”.


v6:
- Patch 9: Rename *freeze_backing_chain (etc.) to *freeze_chain (etc.)
- Patch 10: Drop bdrv_is_encrypted() instead of fixing it the wrong way
- Patch 15: Add a comment on why this works
- Patch 16: Just flush all children instead of one
- Patch 20: We have to snapshot all non-backing children, so both
            metadata and storage children; for simplification, just
            disallow the fallback path if there is more than one such
            child
- Patch 22: bdrv_get_allocated_file_size() should report all children’s
            sizes, not just the primary or storage child’s
- Patch 24: Make query-blockstats too report any filtered child under
            “backing”
- Patch 25:
  - bdrv_is_allocated_above()’s new @include_base parameter makes things
    a bit simpler
  - Forbid mirroring to a filter on top of the base
- Patch 27: bdrv_is_allocated_above()’s new @include_base parameter
            makes things a bit simpler
- Patch 28: Requires a few changes to keep stream independent of the
            base node
- Patch 30:
  - Sprinkle in a few bdrv_skip_implicit_filters()s
  - Conflicts with the convert --salvage patches


git backport-diff against v5:

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/42:[----] [-C] 'block: Mark commit and mirror as filter drivers'
002/42:[----] [--] 'copy-on-read: Support compressed writes'
003/42:[----] [--] 'throttle: Support compressed writes'
004/42:[----] [--] 'block: Add child access functions'
005/42:[----] [--] 'block: Add chain helper functions'
006/42:[----] [--] 'qcow2: Implement .bdrv_storage_child()'
007/42:[----] [--] 'block: *filtered_cow_child() for *has_zero_init()'
008/42:[----] [--] 'block: bdrv_set_backing_hd() is about bs->backing'
009/42:[0078] [FC] 'block: Include filters when freezing backing chain'
010/42:[down] 'block: Drop bdrv_is_encrypted()'
011/42:[----] [-C] 'block: Add bdrv_supports_compressed_writes()'
012/42:[----] [--] 'block: Use bdrv_filtered_rw* where obvious'
013/42:[----] [-C] 'block: Use CAFs in block status functions'
014/42:[----] [--] 'block: Use CAFs when working with backing chains'
015/42:[0017] [FC] 'block: Re-evaluate backing file handling in reopen'
016/42:[down] 'block: Flush all children in generic code'
017/42:[----] [--] 'block: Use CAFs in bdrv_refresh_limits()'
018/42:[----] [--] 'block: Use CAFs in bdrv_refresh_filename()'
019/42:[----] [--] 'block: Use CAF in bdrv_co_rw_vmstate()'
020/42:[down] 'block/snapshot: Fix fallback'
021/42:[----] [--] 'block: Use CAFs for debug breakpoints'
022/42:[down] 'block: Fix bdrv_get_allocated_file_size's fallback'
023/42:[----] [--] 'blockdev: Use CAF in external_snapshot_prepare()'
024/42:[0012] [FC] 'block: Use child access functions for QAPI queries'
025/42:[0019] [FC] 'mirror: Deal with filters'
026/42:[----] [--] 'backup: Deal with filters'
027/42:[0025] [FC] 'commit: Deal with filters'
028/42:[0048] [FC] 'stream: Deal with filters'
029/42:[----] [--] 'nbd: Use CAF when looking for dirty bitmap'
030/42:[0035] [FC] 'qemu-img: Use child access functions'
031/42:[----] [--] 'block: Drop backing_bs()'
032/42:[----] [-C] 'block: Make bdrv_get_cumulative_perm() public'
033/42:[----] [--] 'blockdev: Fix active commit choice'
034/42:[----] [-C] 'block: Inline bdrv_co_block_status_from_*()'
035/42:[----] [--] 'block: Fix check_to_replace_node()'
036/42:[----] [--] 'iotests: Add tests for mirror @replaces loops'
037/42:[----] [-C] 'block: Leave BDS.backing_file constant'
038/42:[----] [--] 'iotests: Let complete_and_wait() work with commit'
039/42:[----] [--] 'iotests: Add filter commit test cases'
040/42:[----] [--] 'iotests: Add filter mirror test cases'
041/42:[----] [--] 'iotests: Add test for commit in sub directory'
042/42:[----] [--] 'iotests: Test committing to overridden backing'


Max Reitz (42):
  block: Mark commit and mirror as filter drivers
  copy-on-read: Support compressed writes
  throttle: Support compressed writes
  block: Add child access functions
  block: Add chain helper functions
  qcow2: Implement .bdrv_storage_child()
  block: *filtered_cow_child() for *has_zero_init()
  block: bdrv_set_backing_hd() is about bs->backing
  block: Include filters when freezing backing chain
  block: Drop bdrv_is_encrypted()
  block: Add bdrv_supports_compressed_writes()
  block: Use bdrv_filtered_rw* where obvious
  block: Use CAFs in block status functions
  block: Use CAFs when working with backing chains
  block: Re-evaluate backing file handling in reopen
  block: Flush all children in generic code
  block: Use CAFs in bdrv_refresh_limits()
  block: Use CAFs in bdrv_refresh_filename()
  block: Use CAF in bdrv_co_rw_vmstate()
  block/snapshot: Fix fallback
  block: Use CAFs for debug breakpoints
  block: Fix bdrv_get_allocated_file_size's fallback
  blockdev: Use CAF in external_snapshot_prepare()
  block: Use child access functions for QAPI queries
  mirror: Deal with filters
  backup: Deal with filters
  commit: Deal with filters
  stream: Deal with filters
  nbd: Use CAF when looking for dirty bitmap
  qemu-img: Use child access functions
  block: Drop backing_bs()
  block: Make bdrv_get_cumulative_perm() public
  blockdev: Fix active commit choice
  block: Inline bdrv_co_block_status_from_*()
  block: Fix check_to_replace_node()
  iotests: Add tests for mirror @replaces loops
  block: Leave BDS.backing_file constant
  iotests: Let complete_and_wait() work with commit
  iotests: Add filter commit test cases
  iotests: Add filter mirror test cases
  iotests: Add test for commit in sub directory
  iotests: Test committing to overridden backing

 qapi/block-core.json          |   4 +
 include/block/block.h         |  13 +-
 include/block/block_int.h     | 109 ++++---
 block.c                       | 552 ++++++++++++++++++++++++++++------
 block/backup.c                |   9 +-
 block/blkdebug.c              |   7 +-
 block/blklogwrites.c          |   1 -
 block/block-backend.c         |  16 +-
 block/commit.c                | 107 +++++--
 block/copy-on-read.c          |  13 +-
 block/io.c                    | 117 ++++---
 block/mirror.c                | 124 ++++++--
 block/qapi.c                  |  48 +--
 block/qcow2.c                 |   9 +
 block/snapshot.c              | 100 ++++--
 block/stream.c                |  52 ++--
 block/throttle.c              |  11 +-
 blockdev.c                    | 139 +++++++--
 nbd/server.c                  |   6 +-
 qemu-img.c                    |  33 +-
 tests/qemu-iotests/020        |  36 +++
 tests/qemu-iotests/020.out    |  10 +
 tests/qemu-iotests/040        | 238 +++++++++++++++
 tests/qemu-iotests/040.out    |   4 +-
 tests/qemu-iotests/041        | 270 ++++++++++++++++-
 tests/qemu-iotests/041.out    |   4 +-
 tests/qemu-iotests/184.out    |   7 +-
 tests/qemu-iotests/191.out    |   1 -
 tests/qemu-iotests/204.out    |   1 +
 tests/qemu-iotests/228        |   6 +-
 tests/qemu-iotests/228.out    |   6 +-
 tests/qemu-iotests/245        |   4 +-
 tests/qemu-iotests/iotests.py |  10 +-
 33 files changed, 1682 insertions(+), 385 deletions(-)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 136+ messages in thread