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 <stefa...@linux.vnet.ibm.com>
> Cc: Jeff Cody <jc...@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> 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 <jc...@redhat.com>
