Re: [Qemu-devel] [PATCH 09/16] block: Split bdrv_move_feature_fields()

[Max Reitz]

On 17.09.2015 15:48, Kevin Wolf wrote:
> After bdrv_swap(), some fields must be moved back to their original BDS
> to compensate for the effects that a swap of the contents of the objects
> has while keeping the old addresses. Other fields must be moved back
> because they should logically be moved and must stay on top
> 
> When replacing bdrv_swap() with operations changing the pointers in the
> parents, we only need the latter and must avoid swapping the former.
> Split the function accordingly.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  block.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/block.c b/block.c
> index 743f82e..7930f3c 100644
> --- a/block.c
> +++ b/block.c
> @@ -1984,6 +1984,8 @@ static void bdrv_rebind(BlockDriverState *bs)
>      }
>  }
>  
> +/* Fields that need to stay with the top-level BDS, no matter whether the
> + * address of the top-level BDS stays the same or not. */
>  static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
>                                       BlockDriverState *bs_src)
>  {
> @@ -2019,7 +2021,13 @@ static void bdrv_move_feature_fields(BlockDriverState 
> *bs_dest,
>  
>      /* dirty bitmap */
>      bs_dest->dirty_bitmaps      = bs_src->dirty_bitmaps;
> +}
>  
> +/* Fields that only need to be swapped if the contents of BDSes is swapped
> + * rather than pointers being changed in the parents. */
> +static void bdrv_move_reference_fields(BlockDriverState *bs_dest,
> +                                       BlockDriverState *bs_src)
> +{
>      /* reference count */
>      bs_dest->refcnt             = bs_src->refcnt;
>  

I'm not sure whether the op blockers should be moved in this function...
I think they should be moved in bdrv_move_feasture_fields(), because
they generally depend on the position within the node graph and not on
the BDS itself, don't they?

Max

> @@ -2090,6 +2098,10 @@ void bdrv_swap(BlockDriverState *bs_new, 
> BlockDriverState *bs_old)
>      bdrv_move_feature_fields(bs_old, bs_new);
>      bdrv_move_feature_fields(bs_new, &tmp);
>  
> +    bdrv_move_reference_fields(&tmp, bs_old);
> +    bdrv_move_reference_fields(bs_old, bs_new);
> +    bdrv_move_reference_fields(bs_new, &tmp);
> +
>      /* bs_new must remain unattached */
>      assert(!bs_new->blk);
>  
>

signature.asc
Description: OpenPGP digital signature