13.06.2019 1:09, Max Reitz wrote: > 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). > > > *** This series depends on some others. *** > > Dependencies: > - [PATCH 0/4] block: Keep track of parent quiescing > - [PATCH 0/2] vl: Drain before (block) job cancel when quitting > - [PATCH v2 0/2] blockdev: Overlays are not snapshots > > Based-on: <20190605161118.14544-1-mre...@redhat.com> > Based-on: <20190612220839.1374-1-mre...@redhat.com> > Based-on: <20190603202236.1342-1-mre...@redhat.com>
Could you please export a branch? > > > v5: > - Split the huge patches 2 and 3 from the previous version into many > smaller patches to maintain the potential reviewers’ sanity [Vladimir] Thank you! In spite of frightening amount of patches, reviewing became a lot simpler. > > - Added support for compressed writes to the COR and throttle filter > drivers to demonstrate how that looks, because the backup job needs to > deal with filters that have such support > > - Added differentiation between bdrv_storage_child(), > bdrv_primary_child(), and bdrv_metadata_child() > > - A whole lot of things Vladimir has noted > > - Made the block jobs really work with filters. In case of commit and > stream, this now means that filters go away if they are between top > and base. I think that’s OK because it’s the user’s choice to include > filters or not. (They can move the filters around if they prefer a > different result.) > - This changes the “Add filter commit test cases” from checking that > most things do not work to checking that they do > > - Added the “blockdev: Fix active commit choice” patch because it turned > out this became necessary after I allowed committing through and with > filters. > > > 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: Use CAF in 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: Use child access functions when flushing > block: Use CAFs in bdrv_refresh_limits() > block: Use CAFs in bdrv_refresh_filename() > block: Use CAF in bdrv_co_rw_vmstate() > block/snapshot: Fall back to storage child > block: Use CAFs for debug breakpoints > block: Use CAFs in bdrv_get_allocated_file_size() > 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 | 2 + > include/block/block_int.h | 109 ++++--- > block.c | 523 +++++++++++++++++++++++++++++----- > block/backup.c | 9 +- > block/blkdebug.c | 7 +- > block/blklogwrites.c | 1 - > block/block-backend.c | 16 +- > block/commit.c | 100 +++++-- > block/copy-on-read.c | 13 +- > block/io.c | 115 ++++---- > block/mirror.c | 113 ++++++-- > block/qapi.c | 42 +-- > block/qcow2.c | 9 + > block/snapshot.c | 74 +++-- > block/stream.c | 23 +- > block/throttle.c | 11 +- > blockdev.c | 139 +++++++-- > nbd/server.c | 6 +- > qemu-img.c | 36 +-- > 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, 1610 insertions(+), 339 deletions(-) > -- Best regards, Vladimir