Re: [Qemu-devel] [PATCH v2 00/16] block: Get rid of bdrv_swap()

[Stefan Hajnoczi]

On Thu, Oct 01, 2015 at 03:13:18PM +0200, Kevin Wolf wrote:
> bdrv_swap() has always been an ugly hack that we would rather have
> avoided.  When it was introduced, we simply didn't have the
> infrastructure to update pointers instead of transplanting the contents
> of BDS object, so we grudgingly added bdrv_swap() as a quick solution.
> Meanwhile, most of the infrastructure exists and this series implements
> the final step necessary to implement the required functionality in a
> less adventurous way.
> 
> v2:
> - Patch 4 ('quorum: Convert to BdrvChild')
>   Use bdrv_unref_child() instead of bdrv_unref() [Berto]
> 
> - Patch 5 ('block: Convert bs->file to BdrvChild')
>   bdrv_close: Set bs->drv = NULL immediately after .bdrv_close [Berto]
>   bdrv_close: bdrv_unref_child() instead of bdrv_unref() for bs->file [Max]
> 
> - Patch 7 ('block: Convert bs->backing_hd to BdrvChild')
>   Use backing_bs() more consistently instead of open-coding and introducing
>   additional bugs while doing so [Max]
> 
> - Patch 8 ('block: Manage backing file references in bdrv_set_backing_hd()')
>   mirror: Use backing instead of s->base->backing->bs [Max]
>   stream: More simplication, remove close_unused_images() altogether [Berto]
> 
> - Patch 11 ('block-backend: Add blk_set_bs()')
>   assert(bs->blk == NULL) [Max]
> 
> - Patch 13 ('block: Implement bdrv_append() without bdrv_swap()')
>   bdrv_ref/unref in change_parent_backing_link [Max]
>   Improved throttle state handling in swap_feature_fields() [Berto]
>   Fixed external_snapshot_commit() [Berto]
> 
> - Patch 14 ('blockjob: Store device name at job creation')
>   Use new job->id more consistently [Max]
> 
> Kevin Wolf (16):
>   block: Introduce BDS.file_child
>   vmdk: Use BdrvChild instead of BDS for references to extents
>   blkverify: Convert s->test_file to BdrvChild
>   quorum: Convert to BdrvChild
>   block: Convert bs->file to BdrvChild
>   block: Remove bdrv_open_image()
>   block: Convert bs->backing_hd to BdrvChild
>   block: Manage backing file references in bdrv_set_backing_hd()
>   block: Split bdrv_move_feature_fields()
>   block/io: Make bdrv_requests_pending() public
>   block-backend: Add blk_set_bs()
>   block: Introduce parents list
>   block: Implement bdrv_append() without bdrv_swap()
>   blockjob: Store device name at job creation
>   block: Add and use bdrv_replace_in_backing_chain()
>   block: Remove bdrv_swap()
> 
>  block.c                   | 470 
> +++++++++++++++++++---------------------------
>  block/blkdebug.c          |  32 ++--
>  block/blkverify.c         |  68 ++++---
>  block/block-backend.c     |  17 ++
>  block/bochs.c             |   8 +-
>  block/cloop.c             |  10 +-
>  block/dmg.c               |  20 +-
>  block/io.c                |  76 ++++----
>  block/mirror.c            |  22 +--
>  block/parallels.c         |  38 ++--
>  block/qapi.c              |  10 +-
>  block/qcow.c              |  46 ++---
>  block/qcow2-cache.c       |  11 +-
>  block/qcow2-cluster.c     |  42 +++--
>  block/qcow2-refcount.c    |  45 ++---
>  block/qcow2-snapshot.c    |  30 +--
>  block/qcow2.c             |  68 +++----
>  block/qed-table.c         |   4 +-
>  block/qed.c               |  51 +++--
>  block/quorum.c            |  63 ++++---
>  block/raw_bsd.c           |  40 ++--
>  block/snapshot.c          |  12 +-
>  block/stream.c            |  34 +---
>  block/vdi.c               |  17 +-
>  block/vhdx-log.c          |  25 +--
>  block/vhdx.c              |  36 ++--
>  block/vmdk.c              | 133 ++++++-------
>  block/vpc.c               |  34 ++--
>  block/vvfat.c             |  19 +-
>  blockdev.c                |   6 +-
>  blockjob.c                |  15 +-
>  include/block/block.h     |  15 +-
>  include/block/block_int.h |  20 +-
>  include/block/blockjob.h  |   8 +
>  include/qemu/queue.h      |   6 -
>  qemu-img.c                |  20 +-
>  36 files changed, 762 insertions(+), 809 deletions(-)

Modulo review comments:

Reviewed-by: Stefan Hajnoczi <address@hidden>