Re: [Qemu-devel] [PATCH 03/11] snapshot: create bdrv_all_delete_snapshot helper

[Stefan Hajnoczi]

On Wed, Nov 04, 2015 at 08:19:34PM +0300, Denis V. Lunev wrote:
> to delete snapshots from all loaded block drivers.
> 
> The patch also ensures proper locking.
> 
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: Juan Quintela <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Kevin Wolf <address@hidden>
> ---
>  block/snapshot.c         | 22 ++++++++++++++++++++
>  include/block/snapshot.h |  2 ++
>  migration/savevm.c       | 54 
> +++++++++---------------------------------------
>  3 files changed, 34 insertions(+), 44 deletions(-)
> 
> diff --git a/block/snapshot.c b/block/snapshot.c
> index d729c76..1b4b846 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -384,3 +384,25 @@ bool bdrv_all_can_snapshot(BlockDriverState 
> **first_bad_bs)
>      *first_bad_bs = bs;
>      return ok;
>  }
> +
> +int bdrv_all_delete_snapshot(const char *name, BlockDriverState 
> **first_bad_bs,
> +                             Error **err)
> +{
> +    int ret = 0;
> +    BlockDriverState *bs = NULL;
> +    QEMUSnapshotInfo sn1, *snapshot = &sn1;
> +
> +    while ((bs = bdrv_next(bs)) && ret == 0) {

If ret != 0 we will iterate to the next bs.  first_bad_bs will be
incorrect.

> +        AioContext *ctx = bdrv_get_aio_context(bs);
> +
> +        aio_context_acquire(ctx);
> +        if (bdrv_can_snapshot(bs) &&
> +                bdrv_snapshot_find(bs, snapshot, name) >= 0) {
> +            ret = bdrv_snapshot_delete_by_id_or_name(bs, name, err);
> +        }
> +        aio_context_release(ctx);
> +    }
> +
> +    *first_bad_bs = bs;
> +    return ret;
> +}

Similar approach without the int return value:

bool bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
                              Error **err)
{
    Error *local_err = NULL;
    BlockDriverState *bs = NULL;
    QEMUSnapshotInfo sn1, *snapshot = &sn1;

    while ((bs = bdrv_next(bs)) {
        AioContext *ctx = bdrv_get_aio_context(bs);

        aio_context_acquire(bs);
        if (bdrv_can_snapshot(bs) &&
            bdrv_snapshot_find(bs, snapshot, name) >= 0) {
            bdrv_snapshot_delete_by_id_or_name(bs, name, &local_err);
        }
        aio_context_release(bs);

        if (local_err) {
            error_propagate(err, local_err);
            return false;
        }
    }
    return true;
}

signature.asc
Description: PGP signature