Re: [Qemu-devel] [PATCH v7 07/10] block: Pass error in bdrv_snapshot_create

[Markus Armbruster]

Fam Zheng <address@hidden> writes:

> This allows descent error information to be reported.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/snapshot.c         | 5 +++--
>  blockdev.c               | 2 +-
>  include/block/snapshot.h | 3 ++-
>  qemu-img.c               | 2 +-
>  savevm.c                 | 2 +-
>  5 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 9047f8d..02cfb07 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -157,7 +157,8 @@ int bdrv_can_snapshot(BlockDriverState *bs)
>  }
>  
>  int bdrv_snapshot_create(BlockDriverState *bs,
> -                         QEMUSnapshotInfo *sn_info)
> +                         QEMUSnapshotInfo *sn_info,
> +                         Error **errp)
>  {
>      BlockDriver *drv = bs->drv;
>      if (!drv) {
> @@ -167,7 +168,7 @@ int bdrv_snapshot_create(BlockDriverState *bs,
>          return drv->bdrv_snapshot_create(bs, sn_info);
>      }
>      if (bs->file) {
> -        return bdrv_snapshot_create(bs->file, sn_info);
> +        return bdrv_snapshot_create(bs->file, sn_info, errp);
>      }
>      return -ENOTSUP;
>  }
> diff --git a/blockdev.c b/blockdev.c
> index 369d8da..404159e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1138,7 +1138,7 @@ static void 
> internal_snapshot_prepare(BlkTransactionState *common,
>      sn->date_nsec = tv.tv_usec * 1000;
>      sn->vm_clock_nsec = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>  
> -    ret1 = bdrv_snapshot_create(bs, sn);
> +    ret1 = bdrv_snapshot_create(bs, sn, errp);
>      if (ret1 < 0) {
>          error_setg_errno(errp, -ret1,
>                           "Failed to create snapshot '%s' on device '%s'",

I'm afraid this will fail the assertion in error_set_errno() as soon as
bdrv_snapshot_create() actually uses errp, and errp isn't null.  Do you
defuse this time bomb later in your series?

[...]