Re: [Qemu-devel] [PATCH 04/11] snapshot: create bdrv_all_goto_snapshot helper

[Juan Quintela <quintela@redhat.com>]

"Denis V. Lunev" <den@openvz.org> wrote:
> to switch to snapshot on all loaded block drivers.
>
> The patch also ensures proper locking.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Fam Zheng <famz@redhat.com>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

suggestion:

un-export bdrv_snapshot_goto()?

I haven't looked if it is used anywhere else, but looks like one idea.


> ---
>  block/snapshot.c         | 20 ++++++++++++++++++++
>  include/block/snapshot.h |  1 +
>  migration/savevm.c       | 15 +++++----------
>  3 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 61a6ad1..9f07a63 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -403,3 +403,23 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
>      *first_bad_bs = bs;
>      return ret;
>  }
> +
> +
> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
> +{
> +    int err = 0;
> +    BlockDriverState *bs = NULL;
> +
> +    while (err == 0 && (bs = bdrv_next(bs))) {
> +        AioContext *ctx = bdrv_get_aio_context(bs);
> +
> +        aio_context_acquire(ctx);
> +        if (bdrv_can_snapshot(bs)) {
> +            err = bdrv_snapshot_goto(bs, name);
> +        }
> +        aio_context_release(ctx);
> +    }
> +
> +    *first_bad_bs = bs;
> +    return err;
> +}
> diff --git a/include/block/snapshot.h b/include/block/snapshot.h
> index d02d2b1..0a176c7 100644
> --- a/include/block/snapshot.h
> +++ b/include/block/snapshot.h
> @@ -84,5 +84,6 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
>  bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
>  int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
>                               Error **err);
> +int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
>  
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c52cbbe..254e51d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2093,16 +2093,11 @@ int load_vmstate(const char *name)
>      /* Flush all IO requests so they don't interfere with the new state.  */
>      bdrv_drain_all();
>  
> -    bs = NULL;
> -    while ((bs = bdrv_next(bs))) {
> -        if (bdrv_can_snapshot(bs)) {
> -            ret = bdrv_snapshot_goto(bs, name);
> -            if (ret < 0) {
> -                error_report("Error %d while activating snapshot '%s' on '%s'",
> -                             ret, name, bdrv_get_device_name(bs));
> -                return ret;
> -            }
> -        }
> +    ret = bdrv_all_goto_snapshot(name, &bs);
> +    if (ret < 0) {
> +        error_report("Error %d while activating snapshot '%s' on '%s'",
> +                     ret, name, bdrv_get_device_name(bs));
> +        return ret;
>      }
>  
>      /* restore the VM state */