diff mbox series

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

Message ID 20231005045041.52649-15-philmd@linaro.org
State New
Headers show
Series qapi: Kill 'qapi/qmp/qerror.h' for good | expand

Commit Message

Philippe Mathieu-Daudé Oct. 5, 2023, 4:50 a.m. UTC 
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(-)

Comments

Markus Armbruster Oct. 20, 2023, 11:50 a.m. UTC | #1 
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;
>      }
>  

[...]
diff mbox series

Patch

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);
+        error_setg(errp, "An IO error has occurred");
         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;
     }
 
@@ -2271,7 +2271,7 @@  vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
     ret = blk_co_pwrite(blk, le64_to_cpu(header.rgd_offset) * BDRV_SECTOR_SIZE,
                         gd_buf_size, gd_buf, 0);
     if (ret < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "An IO error has occurred");
         goto exit;
     }
 
@@ -2283,7 +2283,7 @@  vmdk_init_extent(BlockBackend *blk, int64_t filesize, bool flat, bool compress,
     ret = blk_co_pwrite(blk, le64_to_cpu(header.gd_offset) * BDRV_SECTOR_SIZE,
                         gd_buf_size, gd_buf, 0);
     if (ret < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "An IO error has occurred");
     }
 
     ret = 0;
diff --git a/blockdev.c b/blockdev.c
index ed90365329..228cebf9a2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1433,7 +1433,7 @@  static void external_snapshot_action(TransactionAction *action,
 
     if (!bdrv_is_read_only(state->old_bs)) {
         if (bdrv_flush(state->old_bs)) {
-            error_setg(errp, QERR_IO_ERROR);
+            error_setg(errp, "An IO error has occurred");
             goto out;
         }
     }
diff --git a/dump/win_dump.c b/dump/win_dump.c
index b7bfaff379..0115a609e0 100644
--- a/dump/win_dump.c
+++ b/dump/win_dump.c
@@ -67,7 +67,7 @@  static size_t write_run(uint64_t base_page, uint64_t page_count,
         l = qemu_write_full(fd, buf, len);
         cpu_physical_memory_unmap(buf, addr, false, len);
         if (l != len) {
-            error_setg(errp, QERR_IO_ERROR);
+            error_setg(errp, "An IO error has occurred");
             return 0;
         }
 
@@ -459,7 +459,7 @@  void create_win_dump(DumpState *s, Error **errp)
 
     s->written_size = qemu_write_full(s->fd, h, hdr_size);
     if (s->written_size != hdr_size) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "An IO error has occurred");
         goto out_restore;
     }
 
diff --git a/migration/savevm.c b/migration/savevm.c
index bb3e99194c..41c7f39ef5 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3099,7 +3099,7 @@  void qmp_xen_save_devices_state(const char *filename, bool has_live, bool live,
     object_unref(OBJECT(ioc));
     ret = qemu_save_device_state(f);
     if (ret < 0 || qemu_fclose(f) < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "An IO error has occurred");
     } else {
         /* libxl calls the QMP command "stop" before calling
          * "xen-save-devices-state" and in case of migration failure, libxl
@@ -3148,7 +3148,7 @@  void qmp_xen_load_devices_state(const char *filename, Error **errp)
     ret = qemu_loadvm_state(f);
     qemu_fclose(f);
     if (ret < 0) {
-        error_setg(errp, QERR_IO_ERROR);
+        error_setg(errp, "An IO error has occurred");
     }
     migration_incoming_state_destroy();
 }
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index 7fc70f9166..f7c743b0ce 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -773,7 +773,7 @@  void qmp_memsave(int64_t addr, int64_t size, const char *filename,
             goto exit;
         }
         if (fwrite(buf, 1, l, f) != l) {
-            error_setg(errp, QERR_IO_ERROR);
+            error_setg(errp, "An IO error has occurred");
             goto exit;
         }
         addr += l;
@@ -803,7 +803,7 @@  void qmp_pmemsave(int64_t addr, int64_t size, const char *filename,
             l = size;
         cpu_physical_memory_read(addr, buf, l);
         if (fwrite(buf, 1, l, f) != l) {
-            error_setg(errp, QERR_IO_ERROR);
+            error_setg(errp, "An IO error has occurred");
             goto exit;
         }
         addr += l;