Re: [Qemu-devel] [PATCH 3/7] block: fix snapshot with QED format

[Jeff Cody]

On 04/05/2012 11:42 AM, Paolo Bonzini wrote:
> QED's opaque data includes a pointer back to the BlockDriverState.
> Creating a snapshot whose format is QED breaks the relationship when
> bdrv_append shuffles data between bs_new and bs_top.  To avoid this,
> add a "rebind" function that tells the driver about the new relationship
> between the BlockDriverState and its opaque.
> 
> The patch also adds rebind to VVFAT for completeness, even though it is
> not used with live snapshots.
> 
> Thanks to Stefan and Jeff for help reproducing and debugging this.
> 
> Cc: Stefan Hajnoczi <address@hidden>
> Cc: Jeff Cody <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block.c       |   10 ++++++++++
>  block/qed.c   |    7 +++++++
>  block/vvfat.c |    7 +++++++
>  block_int.h   |    1 +
>  4 files changed, 25 insertions(+)
> 
> diff --git a/block.c b/block.c
> index b3117ef..a3a2e92 100644
> --- a/block.c
> +++ b/block.c
> @@ -885,6 +885,13 @@ void bdrv_make_anon(BlockDriverState *bs)
>      bs->device_name[0] = '\0';
>  }
>  
> +static void bdrv_rebind(BlockDriverState *bs)
> +{
> +    if (bs->drv && bs->drv->bdrv_rebind) {
> +        bs->drv->bdrv_rebind(bs);
> +    }
> +}
> +
>  /*
>   * Add new bs contents at the top of an image chain while the chain is
>   * live, while keeping required fields on the top layer.
> @@ -973,6 +980,9 @@ void bdrv_append(BlockDriverState *bs_new, 
> BlockDriverState *bs_top)
>      bs_new->slice_time        = 0;
>      bs_new->slice_start       = 0;
>      bs_new->slice_end         = 0;
> +
> +    bdrv_rebind(bs_new);
> +    bdrv_rebind(bs_top);
>  }
>  
>  void bdrv_delete(BlockDriverState *bs)
> diff --git a/block/qed.c b/block/qed.c
> index 19d87f3..9364212 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -367,6 +367,12 @@ static void qed_cancel_need_check_timer(BDRVQEDState *s)
>      qemu_del_timer(s->need_check_timer);
>  }
>  
> +static void bdrv_qed_rebind(BlockDriverState *bs)
> +{
> +    BDRVQEDState *s = bs->opaque;
> +    s->bs = bs;
> +}
> +
>  static int bdrv_qed_open(BlockDriverState *bs, int flags)
>  {
>      BDRVQEDState *s = bs->opaque;
> @@ -1549,6 +1555,7 @@ static BlockDriver bdrv_qed = {
>      .create_options           = qed_create_options,
>  
>      .bdrv_probe               = bdrv_qed_probe,
> +    .bdrv_rebind              = bdrv_qed_rebind,
>      .bdrv_open                = bdrv_qed_open,
>      .bdrv_close               = bdrv_qed_close,
>      .bdrv_create              = bdrv_qed_create,
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 9ef21dd..2dc9d50 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -982,6 +982,12 @@ static BDRVVVFATState *vvv = NULL;
>  static int enable_write_target(BDRVVVFATState *s);
>  static int is_consistent(BDRVVVFATState *s);
>  
> +static void vvfat_rebind(BlockDriverState *bs)
> +{
> +    BDRVVVFATState *s = bs->opaque;
> +    s->bs = bs;
> +}
> +
>  static int vvfat_open(BlockDriverState *bs, const char* dirname, int flags)
>  {
>      BDRVVVFATState *s = bs->opaque;
> @@ -2855,6 +2861,7 @@ static BlockDriver bdrv_vvfat = {
>      .format_name     = "vvfat",
>      .instance_size   = sizeof(BDRVVVFATState),
>      .bdrv_file_open  = vvfat_open,
> +    .bdrv_rebind     = vvfat_rebind,
>      .bdrv_read          = vvfat_co_read,
>      .bdrv_write         = vvfat_co_write,
>      .bdrv_close              = vvfat_close,
> diff --git a/block_int.h b/block_int.h
> index 0e5a032..58e3eea 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -139,6 +139,7 @@ struct BlockDriver {
>      int (*bdrv_write)(BlockDriverState *bs, int64_t sector_num,
>                        const uint8_t *buf, int nb_sectors);
>      void (*bdrv_close)(BlockDriverState *bs);
> +    void (*bdrv_rebind)(BlockDriverState *bs);
>      int (*bdrv_create)(const char *filename, QEMUOptionParameter *options);
>      int (*bdrv_set_key)(BlockDriverState *bs, const char *key);
>      int (*bdrv_make_empty)(BlockDriverState *bs);


Reviewed-by: Jeff Cody <address@hidden>