Re: [Qemu-devel] [PATCH 1/4] block: Move external snapshot image creation in the block drivers.

[Stefan Hajnoczi]

On Mon, Jan 28, 2013 at 02:22:15PM +0100, Benoît Canet wrote:
> diff --git a/block.c b/block.c
> index 6fa7c90..af22ee8 100644
> --- a/block.c
> +++ b/block.c
> @@ -4626,3 +4626,45 @@ out:
>          bdrv_delete(bs);
>      }
>  }
> +
> +void bdrv_generic_ext_snapshot_img_create(BlockDriverState *bs,
> +                                          const char *filename,
> +                                          const char *fmt,
> +                                          const char *base_filename,
> +                                          const char *base_fmt,
> +                                          char *options,
> +                                          uint64_t img_size,
> +                                          int flags,
> +                                          Error **errp)
> +{
> +    bdrv_img_create(filename, fmt, base_filename, base_fmt, options, 
> img_size,
> +                    flags, errp);
> +}

It would be easier to inline the bdrv_img_create() call into
bdrv_ext_snapshot_img_create() so that BlockDriver structs do not need
to explicitly reference bdrv_generic_ext_snapshot_img_create().

That way folks writing new block drivers don't need to worry about
.bdrv_ext_snapshot_img_create().

> +
> +void bdrv_ext_snapshot_img_create(BlockDriverState *old_bs,
> +                                  const char *filename, const char *fmt,
> +                                  const char *base_filename,
> +                                  const char *base_fmt,
> +                                  char *options, uint64_t img_size,
> +                                  int flags, Error **errp)
> +{
> +    if (!old_bs || !old_bs->file || !old_bs->file->drv) {
> +        error_setg(errp, "Block driver not reachable.");
> +        return;
> +    }
> +
> +    if (!old_bs->file->drv->bdrv_ext_snapshot_img_create) {
> +        error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> +                  bdrv_get_format_name(old_bs->file),
> +                  bdrv_get_device_name(old_bs),
> +                  "external snapshots");
> +        return;
> +    }
> +
> +    old_bs->file->drv->bdrv_ext_snapshot_img_create(old_bs->file,
> +                                                    filename, fmt,
> +                                                    base_filename,
> +                                                    base_fmt,
> +                                                    options, img_size,
> +                                                    flags, errp);

Why old_bs->file?  This means the function cannot be called on a
protocol BlockDriverState.