On Wed, Oct 28, 2015 at 06:01:05PM +0300, Denis V. Lunev wrote:
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 89500f2..f6fa17a 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -259,6 +259,9 @@ void bdrv_snapshot_delete_by_id_or_name(BlockDriverState
> *bs,
> {
> int ret;
> Error *local_err = NULL;
> + AioContext *aio_context = bdrv_get_aio_context(bs);
> +
> + aio_context_acquire(aio_context);
>
> ret = bdrv_snapshot_delete(bs, id_or_name, NULL, &local_err);
> if (ret == -ENOENT || ret == -EINVAL) {
> @@ -267,6 +270,8 @@ void bdrv_snapshot_delete_by_id_or_name(BlockDriverState
> *bs,
> ret = bdrv_snapshot_delete(bs, NULL, id_or_name, &local_err);
> }
>
> + aio_context_release(aio_context);
> +
> if (ret < 0) {
> error_propagate(errp, local_err);
> }Please make the caller acquire the AioContext instead of modifying bdrv_snapshot_delete_id_or_name() because no other functions in this file acquire AioContext and the API should be consistent. There's no harm in recursive locking but it is hard to write correct code if related functions differ in whether or not they acquire the AioContext. Either all of them should acquire AioContext or none of them.
signature.asc
Description: PGP signature
