Re: [PATCH v2 02/22] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition

[Markus Armbruster <armbru@redhat.com>]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Address the comment added in commit 4629ed1e98
> ("qerror: Finally unused, clean up"), from 2015:
>
>   /*
>    * These macros will go away, please don't use
>    * in new code, and do not add new ones!
>    */
>
> Mechanical transformation using sed, manually
> removing the definition in include/qapi/qmp/qerror.h.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  block/snapshot.c          | 4 ++--
>  blockdev.c                | 2 +-
>  3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index 1a9c2d3502..168177bcd7 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -17,9 +17,6 @@
>   * add new ones!
>   */
>  
> -#define QERR_DEVICE_HAS_NO_MEDIUM \
> -    "Device '%s' has no medium"
> -
>  #define QERR_DEVICE_IN_USE \
>      "Device '%s' is in use"
>  
> diff --git a/block/snapshot.c b/block/snapshot.c
> index b86b5b24ad..eb43e957e1 100644
> --- a/block/snapshot.c
> +++ b/block/snapshot.c
> @@ -345,7 +345,7 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
>      GLOBAL_STATE_CODE();
>  
>      if (!drv) {
> -        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
> +        error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs));

Break the line after the last comma, please.

>          return -ENOMEDIUM;
>      }
>      if (!snapshot_id && !name) {
> @@ -420,7 +420,7 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
>      GLOBAL_STATE_CODE();
>  
>      if (!drv) {
> -        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
> +        error_setg(errp, "Device '%s' has no medium", bdrv_get_device_name(bs));

Likewise.

>          return -ENOMEDIUM;
>      }
>      if (!snapshot_id && !name) {
> diff --git a/blockdev.c b/blockdev.c
> index 325b7a3bef..e5617faf0f 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1422,7 +1422,7 @@ static void external_snapshot_action(TransactionAction *action,
>      bdrv_drained_begin(state->old_bs);
>  
>      if (!bdrv_is_inserted(state->old_bs)) {
> -        error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
> +        error_setg(errp, "Device '%s' has no medium", device);
>          goto out;
>      }

Outside this patch's scope, but here goes anyway...  Elsewhere in this
file, we have two instances of

        error_setg(errp, "Device has no medium");

The user gets to guess which device.

Looks like commit b6c1bae5df8 (block: Accept node-name for block-stream)
and commit 7a9877a0263 (block: Accept device model name for
block_set_io_throttle) sacrificed the '%s'.  Can we put it back?