Re: [PATCH v2 14/22] qapi: Inline and remove QERR_IO_ERROR definition

[Markus Armbruster]

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 -i -e 's/QERR_IO_ERROR/"An IO error has occurred"/' \
>     $(git grep -wl QERR_IO_ERROR)
>
> then manually removing the definition in include/qapi/qmp/qerror.h.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  block/vmdk.c              | 8 ++++----
>  blockdev.c                | 2 +-
>  dump/win_dump.c           | 4 ++--
>  migration/savevm.c        | 4 ++--
>  softmmu/cpus.c            | 4 ++--
>  6 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index ac727d1c2d..d95c4b84b9 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -17,9 +17,6 @@
>   * add new ones!
>   */
>  
> -#define QERR_IO_ERROR \
> -    "An IO error has occurred"
> -
>  #define QERR_MIGRATION_ACTIVE \
>      "There's a migration process in progress"
>  
> diff --git a/block/vmdk.c b/block/vmdk.c
> index e90649c8bf..6779a181f0 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2246,12 +2246,12 @@ vmdk_init_extent(BlockBackend *blk, int64_t filesize, 
> bool flat, bool compress,
>      /* write all the data */
>      ret = blk_co_pwrite(blk, 0, sizeof(magic), &magic, 0);
>      if (ret < 0) {
> -        error_setg(errp, QERR_IO_ERROR);

As far as I can tell, blk_co_pwrite() returns a negative errno code.
Which we throw away, and claim "IO error".  I expect that to be
misleading at least sometimes.

I suspect the other uses of QERR_IO_ERROR are similarly problematic more
often than not.

Not your patch's problem, of course.

> +        error_setg(errp, "An IO error has occurred");

We should spell it "I/O", unless we're reporting trouble with Jupiter's
moon.

>          goto exit;
>      }
>      ret = blk_co_pwrite(blk, sizeof(magic), sizeof(header), &header, 0);
>      if (ret < 0) {
> -        error_setg(errp, QERR_IO_ERROR);
> +        error_setg(errp, "An IO error has occurred");
>          goto exit;
>      }
>  

[...]