Re: [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend

From: Stefan Hajnoczi <stefanha@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org, berto@igalia.com, pbonzini@redhat.com,
	mreitz@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v3 00/14] block: Move I/O throttling to BlockBackend
Date: Mon, 16 May 2016 15:46:46 -0700	[thread overview]
Message-ID: <20160516224646.GG24145@stefanha-x1.localdomain> (raw)
In-Reply-To: <1462973065-9876-1-git-send-email-kwolf@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

On Wed, May 11, 2016 at 03:24:11PM +0200, Kevin Wolf wrote:
> This is another feature that was "logically" part of the BlockBackend, but
> implemented as a BlockDriverState feature. It was always kept on top using
> swap_feature_fields().
> 
> This series moves it to be actually implemented in the BlockBackend, removing
> another obstacle for removing bs->blk and allowing multiple BBs per BDS.
> 
> Applies to block-next.
> 
> 
> v3:
> - Patch 5: 'block: Move throttling fields from BDS to BB'
>   Fixed a few commentions still mentioning BDS instead of BB [Berto]
> 
> - Patch 9: 'block: Drain throttling queue with BdrvChild'
>   Documented the semantics of BdrvChildRole.drained_begin/end [Stefan]
> 
> - Patch 10 (new): 'block/io: Quiesce parents between drained_begin/end'
>   Fix that parent requests were allowed again too early [Stefan]
> 
> - Patch 14 (was 13): 'block: Don't check throttled reqs in
>                       bdrv_requests_pending()'
>   Explained more cases in the commit message [Berto]
> 
> 
> v2:
> - Rebased on top of Paolo's 'bdrv_flush_io_queue removal, shared LinuxAioState'
>   Most notable this includes a complete rewrite of patch 9 (was 10): 'block:
>   Drain throttling queue with BdrvChild'. Instead of a single drain_queue()
>   callback we now have a drained_begin/end() pair.
> 
> - Patch 2 (was 3): 'block: Introduce BlockBackendPublic'
>   Add int dummy to yet empty struct BlockBackendPublic [Eric]
> 
> - Patch 11: 'block: Remove bdrv_move_feature_fields()'
>   After the rebase, the function ended up empty, we can remove it now
> 
> - Patch 12: 'Revert "block: Forbid I/O throttling on nodes with
>              multiple parents for 2.6"'
>   This was committed to master after v1 had been posted, so this one is new as
>   well. The reason for forbidding this was that patch 6 ('block: Move actual
>   I/O throttling to BlockBackend') would change the behaviour of the non-BB
>   parents. Now that the final behaviour is implemented, we can allow the setup.
> 
> Kevin Wolf (14):
>   block: Make sure throttled BDSes always have a BB
>   block: Introduce BlockBackendPublic
>   block: throttle-groups: Use BlockBackend pointers internally
>   block: Convert throttle_group_get_name() to BlockBackend
>   block: Move throttling fields from BDS to BB
>   block: Move actual I/O throttling to BlockBackend
>   block: Move I/O throttling configuration functions to BlockBackend
>   block: Introduce BdrvChild.opaque
>   block: Drain throttling queue with BdrvChild callback
>   block/io: Quiesce parents between drained_begin/end
>   block: Decouple throttling from BlockDriverState
>   block: Remove bdrv_move_feature_fields()
>   Revert "block: Forbid I/O throttling on nodes with multiple parents
>     for 2.6"
>   block: Don't check throttled reqs in bdrv_requests_pending()
> 
>  block.c                         |  58 +---------
>  block/block-backend.c           | 135 ++++++++++++++++++----
>  block/io.c                      |  86 ++++----------
>  block/qapi.c                    |   6 +-
>  block/throttle-groups.c         | 244 +++++++++++++++++++++-------------------
>  blockdev.c                      |  50 +++-----
>  include/block/block.h           |   4 -
>  include/block/block_int.h       |  35 ++----
>  include/block/throttle-groups.h |  14 +--
>  include/sysemu/block-backend.h  |  27 +++++
>  tests/test-throttle.c           |  62 +++++-----
>  11 files changed, 359 insertions(+), 362 deletions(-)

I'm happy with this series modulo the typos I mentioned in replies.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]