Re: [Qemu-devel] [PATCH] blockdev: Snapshotting must not open second instance of old top

[Jeff Cody]

On Wed, Mar 02, 2016 at 12:27:01PM +0100, Kevin Wolf wrote:
> Calling bdrv_img_create() with a size of -1 means that it determines the
> size automatically by opening the backing file. However, in the case of
> live snapshots, the backing file is already opened and we must avoid
> opening the same image twice at the same time. Apart from that, just
> getting the size from the already existing BDS is a lot less overhead
> than opening a new instance.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  blockdev.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/blockdev.c b/blockdev.c
> index 151b800..3abfd0d 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1739,10 +1739,15 @@ static void external_snapshot_prepare(BlkActionState 
> *common,
>          /* create new image w/backing file */
>          mode = s->has_mode ? s->mode : NEW_IMAGE_MODE_ABSOLUTE_PATHS;
>          if (mode != NEW_IMAGE_MODE_EXISTING) {
> +            int64_t size = bdrv_getlength(state->old_bs);
> +            if (size < 0) {
> +                error_setg_errno(errp, -size, "bdrv_getlength failed");
> +                return;
> +            }
>              bdrv_img_create(new_image_file, format,
>                              state->old_bs->filename,
>                              state->old_bs->drv->format_name,
> -                            NULL, -1, flags, &local_err, false);
> +                            NULL, size, flags, &local_err, false);
>              if (local_err) {
>                  error_propagate(errp, local_err);
>                  return;
> -- 
> 1.8.3.1
>

Reviewed-by: Jeff Cody <address@hidden>