Re: [Qemu-devel] [PATCH 14/14] block: use proper qerrors in qmp_block_resize

[Markus Armbruster]

A bit late, but here goes anyway:

Luiz Capitulino <address@hidden> writes:

> From: Stefan Hajnoczi <address@hidden>
>
> Let's report specific errors so that management tools and users can
> identify the problem.
>
> Two new qerrors are needed:
>  * QERR_DEVICE_HAS_NO_MEDIUM for ENOMEDIUM
>  * QERR_DEVICE_IS_READ_ONLY for EACCES
[...]
> diff --git a/blockdev.c b/blockdev.c
> index 5d16137..1f83c88 100644
> --- a/blockdev.c
> +++ b/blockdev.c
[...]
> @@ -864,12 +859,27 @@ void qmp_block_resize(const char *device, int64_t size, 
> Error **errp)
>      }
>  
>      if (size < 0) {
> -        error_set(errp, QERR_UNDEFINED_ERROR);
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "size", "a >0 size");
>          return;
>      }
>  
> -    if (bdrv_truncate(bs, size)) {
> +    switch (bdrv_truncate(bs, size)) {
> +    case 0:
> +        break;
> +    case -ENOMEDIUM:
> +        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> +        break;
> +    case -ENOTSUP:
> +        error_set(errp, QERR_UNSUPPORTED);
> +        break;
> +    case -EACCES:
> +        error_set(errp, QERR_DEVICE_IS_READ_ONLY, device);
> +        break;

Are you sure "read only" is (and will remain) the only possible reason
for EACCES here?  I mean bdrv_truncate() obviously uses it for that, but
what about all the driver methods?

Aside: bdrv_truncate()'s use of EACCES is a somewhat unusual.  System
calls generally use EBADF when refusing to change a read-only file.
Outside this patch's scope.

> +    case -EBUSY:
> +        error_set(errp, QERR_DEVICE_IN_USE, device);
> +        break;
> +    default:
>          error_set(errp, QERR_UNDEFINED_ERROR);
> -        return;
> +        break;
>      }
>  }
[...]