[PATCH 00/10] Reduce usage of QERR_ macros further

[PATCH 00/10] Reduce usage of QERR_ macros further

[Markus Armbruster]

Philippe posted "[PATCH v2 00/22] qapi: Kill 'qapi/qmp/qerror.h' for
good" a couple of months ago.  I cherry-picked just its simplest parts
for now.

Markus Armbruster (1):
  error: Drop superfluous #include "qapi/qmp/qerror.h"

Philippe Mathieu-Daudé (9):
  qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition
  qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition
  qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition
  qapi: Inline and remove QERR_INVALID_PARAMETER definition
  qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)
  qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition
  qapi: Correct error message for 'vcpu_dirty_limit' parameter
  qapi: Inline and remove QERR_MIGRATION_ACTIVE definition
  qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition

 include/qapi/qmp/qerror.h    | 21 ---------------------
 backends/iommufd.c           |  1 -
 block/snapshot.c             |  7 ++++---
 blockdev.c                   |  2 +-
 chardev/char-fe.c            |  1 -
 hw/core/qdev-properties.c    |  3 +--
 hw/core/qdev.c               |  4 ++--
 hw/ppc/spapr_pci.c           |  5 ++---
 migration/migration.c        |  2 +-
 migration/options.c          |  9 ++++-----
 migration/savevm.c           |  2 +-
 qapi/opts-visitor.c          |  2 +-
 qapi/qobject-input-visitor.c | 32 ++++++++++++++++----------------
 qapi/string-input-visitor.c  |  8 ++++----
 qom/object.c                 | 16 ++++++++++------
 system/qdev-monitor.c        | 10 ++++++----
 system/rtc.c                 |  1 -
 util/qemu-option.c           | 10 +++++-----
 scripts/qapi/visit.py        |  5 ++---
 19 files changed, 60 insertions(+), 81 deletions(-)

-- 
2.44.0

[PATCH 01/10] error: Drop superfluous #include "qapi/qmp/qerror.h"

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 backends/iommufd.c | 1 -
 chardev/char-fe.c  | 1 -
 system/rtc.c       | 1 -
 3 files changed, 3 deletions(-)

diff --git a/backends/iommufd.c b/backends/iommufd.c
index 1ef683c7b0..922d75e49e 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -13,7 +13,6 @@
 #include "qemu/osdep.h"
 #include "sysemu/iommufd.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qerror.h"
 #include "qemu/module.h"
 #include "qom/object_interfaces.h"
 #include "qemu/error-report.h"
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 66cee8475a..b214ba3802 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -24,7 +24,6 @@
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qerror.h"
 #include "sysemu/replay.h"
 
 #include "chardev/char-fe.h"
diff --git a/system/rtc.c b/system/rtc.c
index 4904581abe..dc44576686 100644
--- a/system/rtc.c
+++ b/system/rtc.c
@@ -25,7 +25,6 @@
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qerror.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include "qemu/timer.h"
-- 
2.44.0

Re: [PATCH 01/10] error: Drop superfluous #include "qapi/qmp/qerror.h"

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:34PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:34 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 01/10] error: Drop superfluous #include "qapi/qmp/qerror.h"
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  backends/iommufd.c | 1 -
>  chardev/char-fe.c  | 1 -
>  system/rtc.c       | 1 -
>  3 files changed, 3 deletions(-)
> 

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
 

[PATCH 02/10] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition

[Markus Armbruster]

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

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, and manual cleanup.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 hw/ppc/spapr_pci.c        | 5 ++---
 system/qdev-monitor.c     | 8 +++++---
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 0c2689cf8a..06a1d2248e 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,9 +17,6 @@
  * add new ones!
  */
 
-#define QERR_BUS_NO_HOTPLUG \
-    "Bus '%s' does not support hotplugging"
-
 #define QERR_DEVICE_HAS_NO_MEDIUM \
     "Device '%s' has no medium"
 
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 25e0295d6f..72cfba419a 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -39,7 +39,6 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
-#include "qapi/qmp/qerror.h"
 #include "hw/ppc/fdt.h"
 #include "hw/pci/pci_bridge.h"
 #include "hw/pci/pci_bus.h"
@@ -1554,7 +1553,7 @@ static void spapr_pci_pre_plug(HotplugHandler *plug_handler,
          * we need to let them know it's not enabled
          */
         if (plugged_dev->hotplugged) {
-            error_setg(errp, QERR_BUS_NO_HOTPLUG,
+            error_setg(errp, "Bus '%s' does not support hotplugging",
                        phb->parent_obj.bus->qbus.name);
             return;
         }
@@ -1675,7 +1674,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
     SpaprDrc *drc = drc_from_dev(phb, pdev);
 
     if (!phb->dr_enabled) {
-        error_setg(errp, QERR_BUS_NO_HOTPLUG,
+        error_setg(errp, "Bus '%s' does not support hotplugging",
                    phb->parent_obj.bus->qbus.name);
         return;
     }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 09e07cab9b..842c142c79 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -661,7 +661,8 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
 
     if (qdev_should_hide_device(opts, from_json, errp)) {
         if (bus && !qbus_is_hotpluggable(bus)) {
-            error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
+            error_setg(errp, "Bus '%s' does not support hotplugging",
+                       bus->name);
         }
         return NULL;
     } else if (*errp) {
@@ -669,7 +670,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
     }
 
     if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
-        error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
+        error_setg(errp, "Bus '%s' does not support hotplugging", bus->name);
         return NULL;
     }
 
@@ -911,7 +912,8 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     }
 
     if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) {
-        error_setg(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
+        error_setg(errp, "Bus '%s' does not support hotplugging",
+                   dev->parent_bus->name);
         return;
     }
 
-- 
2.44.0

Re: [PATCH 02/10] qapi: Inline and remove QERR_BUS_NO_HOTPLUG definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:35PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:35 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 02/10] qapi: Inline and remove QERR_BUS_NO_HOTPLUG
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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, and manual cleanup.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  hw/ppc/spapr_pci.c        | 5 ++---
>  system/qdev-monitor.c     | 8 +++++---
>  3 files changed, 7 insertions(+), 9 deletions(-)
>

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 03/10] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition

[Markus Armbruster]

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

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, and manual cleanup.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 block/snapshot.c          | 7 ++++---
 blockdev.c                | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 06a1d2248e..daa889809b 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_NO_HOTPLUG \
     "Device '%s' does not support hotplugging"
 
diff --git a/block/snapshot.c b/block/snapshot.c
index 8694fc0a3e..e2c18d3f8f 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -28,7 +28,6 @@
 #include "block/qdict.h"
 #include "qapi/error.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qstring.h"
 #include "qemu/option.h"
 #include "sysemu/block-backend.h"
@@ -359,7 +358,8 @@ 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));
         return -ENOMEDIUM;
     }
     if (!snapshot_id && !name) {
@@ -437,7 +437,8 @@ int bdrv_snapshot_load_tmp(BlockDriverState *bs,
     GRAPH_RDLOCK_GUARD_MAINLOOP();
 
     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));
         return -ENOMEDIUM;
     }
     if (!snapshot_id && !name) {
diff --git a/blockdev.c b/blockdev.c
index d8fb3399f5..bd408e3e75 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1395,7 +1395,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);
         return;
     }
 
-- 
2.44.0

Re: [PATCH 03/10] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:36PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:36 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 03/10] qapi: Inline and remove QERR_DEVICE_HAS_NO_MEDIUM
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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, and manual cleanup.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  block/snapshot.c          | 7 ++++---
>  blockdev.c                | 2 +-
>  3 files changed, 5 insertions(+), 7 deletions(-)
>

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 04/10] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition

[Markus Armbruster]

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

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, and manual cleanup.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 hw/core/qdev.c            | 4 ++--
 system/qdev-monitor.c     | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index daa889809b..e93211085a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,9 +17,6 @@
  * add new ones!
  */
 
-#define QERR_DEVICE_NO_HOTPLUG \
-    "Device '%s' does not support hotplugging"
-
 #define QERR_INVALID_PARAMETER \
     "Invalid parameter '%s'"
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c68d0f7c51..00efaf1bd1 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -29,7 +29,6 @@
 #include "qapi/error.h"
 #include "qapi/qapi-events-qdev.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qerror.h"
 #include "qapi/visitor.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
@@ -479,7 +478,8 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     static int unattached_count;
 
     if (dev->hotplugged && !dc->hotpluggable) {
-        error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
+        error_setg(errp, "Device '%s' does not support hotplugging",
+                   object_get_typename(obj));
         return;
     }
 
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 842c142c79..e2eea7d96e 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -918,7 +918,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
     }
 
     if (!dc->hotpluggable) {
-        error_setg(errp, QERR_DEVICE_NO_HOTPLUG,
+        error_setg(errp, "Device '%s' does not support hotplugging",
                    object_get_typename(OBJECT(dev)));
         return;
     }
-- 
2.44.0

Re: [PATCH 04/10] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:37PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:37 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 04/10] qapi: Inline and remove QERR_DEVICE_NO_HOTPLUG
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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, and manual cleanup.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  hw/core/qdev.c            | 4 ++--
>  system/qdev-monitor.c     | 2 +-
>  3 files changed, 3 insertions(+), 6 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 05/10] qapi: Inline and remove QERR_INVALID_PARAMETER definition

[Markus Armbruster]

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

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_INVALID_PARAMETER,/\"Invalid parameter '%s'\",/" \
    $(git grep -lw QERR_INVALID_PARAMETER)

Manually simplify qemu_opts_create(), and remove the macro definition
in include/qapi/qmp/qerror.h.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h |  3 ---
 qapi/opts-visitor.c       |  2 +-
 util/qemu-option.c        | 10 +++++-----
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index e93211085a..63ab775dc5 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,9 +17,6 @@
  * add new ones!
  */
 
-#define QERR_INVALID_PARAMETER \
-    "Invalid parameter '%s'"
-
 #define QERR_INVALID_PARAMETER_TYPE \
     "Invalid parameter type for '%s', expected: %s"
 
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 8f1efab8b9..3d1a28b419 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -184,7 +184,7 @@ opts_check_struct(Visitor *v, Error **errp)
         const QemuOpt *first;
 
         first = g_queue_peek_head(any);
-        error_setg(errp, QERR_INVALID_PARAMETER, first->name);
+        error_setg(errp, "Invalid parameter '%s'", first->name);
         return false;
     }
     return true;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index eedd08929b..201f7a87f3 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -498,7 +498,7 @@ static bool opt_validate(QemuOpt *opt, Error **errp)
 
     desc = find_desc_by_name(list->desc, opt->name);
     if (!desc && !opts_accepts_any(list)) {
-        error_setg(errp, QERR_INVALID_PARAMETER, opt->name);
+        error_setg(errp, "Invalid parameter '%s'", opt->name);
         return false;
     }
 
@@ -531,7 +531,7 @@ bool qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val,
 
     desc = find_desc_by_name(list->desc, name);
     if (!desc && !opts_accepts_any(list)) {
-        error_setg(errp, QERR_INVALID_PARAMETER, name);
+        error_setg(errp, "Invalid parameter '%s'", name);
         return false;
     }
 
@@ -554,7 +554,7 @@ bool qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val,
 
     desc = find_desc_by_name(list->desc, name);
     if (!desc && !opts_accepts_any(list)) {
-        error_setg(errp, QERR_INVALID_PARAMETER, name);
+        error_setg(errp, "Invalid parameter '%s'", name);
         return false;
     }
 
@@ -612,7 +612,7 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
 
     if (list->merge_lists) {
         if (id) {
-            error_setg(errp, QERR_INVALID_PARAMETER, "id");
+            error_setg(errp, "Invalid parameter 'id'");
             return NULL;
         }
         opts = qemu_opts_find(list, NULL);
@@ -1103,7 +1103,7 @@ bool qemu_opts_validate(QemuOpts *opts, const QemuOptDesc *desc, Error **errp)
     QTAILQ_FOREACH(opt, &opts->head, next) {
         opt->desc = find_desc_by_name(desc, opt->name);
         if (!opt->desc) {
-            error_setg(errp, QERR_INVALID_PARAMETER, opt->name);
+            error_setg(errp, "Invalid parameter '%s'", opt->name);
             return false;
         }
 
-- 
2.44.0

Re: [PATCH 05/10] qapi: Inline and remove QERR_INVALID_PARAMETER definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:38PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:38 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 05/10] qapi: Inline and remove QERR_INVALID_PARAMETER
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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_INVALID_PARAMETER,/\"Invalid parameter '%s'\",/" \
>     $(git grep -lw QERR_INVALID_PARAMETER)
> 
> Manually simplify qemu_opts_create(), and remove the macro definition
> in include/qapi/qmp/qerror.h.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h |  3 ---
>  qapi/opts-visitor.c       |  2 +-
>  util/qemu-option.c        | 10 +++++-----
>  3 files changed, 6 insertions(+), 9 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 06/10] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)

[Markus Armbruster]

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

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 the following
coccinelle semantic patch:

    @match@
    expression errp;
    expression param;
    constant value;
    @@
         error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value);

    @script:python strformat depends on match@
    value << match.value;
    fixedfmt; // new var
    @@
    fixedfmt = f'"Invalid parameter type for \'%s\', expected: {value[1:-1]}"'
    coccinelle.fixedfmt = cocci.make_ident(fixedfmt)

    @replace@
    expression match.errp;
    expression match.param;
    constant match.value;
    identifier strformat.fixedfmt;
    @@
    -    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value);
    +    error_setg(errp, fixedfmt, param);

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/qobject-input-visitor.c | 32 ++++++++++++++++----------------
 qapi/string-input-visitor.c  |  8 ++++----
 qom/object.c                 | 12 ++++++++----
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 3e8aca6b15..f110a804b2 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -288,8 +288,8 @@ static bool qobject_input_start_struct(Visitor *v, const char *name, void **obj,
         return false;
     }
     if (qobject_type(qobj) != QTYPE_QDICT) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "object");
+        error_setg(errp, "Invalid parameter type for '%s', expected: object",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -326,8 +326,8 @@ static bool qobject_input_start_list(Visitor *v, const char *name,
         return false;
     }
     if (qobject_type(qobj) != QTYPE_QLIST) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "array");
+        error_setg(errp, "Invalid parameter type for '%s', expected: array",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -405,8 +405,8 @@ static bool qobject_input_type_int64(Visitor *v, const char *name, int64_t *obj,
     }
     qnum = qobject_to(QNum, qobj);
     if (!qnum || !qnum_get_try_int(qnum, obj)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "integer");
+        error_setg(errp, "Invalid parameter type for '%s', expected: integer",
+                   full_name(qiv, name));
         return false;
     }
     return true;
@@ -494,8 +494,8 @@ static bool qobject_input_type_bool(Visitor *v, const char *name, bool *obj,
     }
     qbool = qobject_to(QBool, qobj);
     if (!qbool) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "boolean");
+        error_setg(errp, "Invalid parameter type for '%s', expected: boolean",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -534,8 +534,8 @@ static bool qobject_input_type_str(Visitor *v, const char *name, char **obj,
     }
     qstr = qobject_to(QString, qobj);
     if (!qstr) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "string");
+        error_setg(errp, "Invalid parameter type for '%s', expected: string",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -565,8 +565,8 @@ static bool qobject_input_type_number(Visitor *v, const char *name, double *obj,
     }
     qnum = qobject_to(QNum, qobj);
     if (!qnum) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "number");
+        error_setg(errp, "Invalid parameter type for '%s', expected: number",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -587,8 +587,8 @@ static bool qobject_input_type_number_keyval(Visitor *v, const char *name,
 
     if (qemu_strtod_finite(str, NULL, &val)) {
         /* TODO report -ERANGE more nicely */
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "number");
+        error_setg(errp, "Invalid parameter type for '%s', expected: number",
+                   full_name(qiv, name));
         return false;
     }
 
@@ -623,8 +623,8 @@ static bool qobject_input_type_null(Visitor *v, const char *name,
     }
 
     if (qobject_type(qobj) != QTYPE_QNULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
-                   full_name(qiv, name), "null");
+        error_setg(errp, "Invalid parameter type for '%s', expected: null",
+                   full_name(qiv, name));
         return false;
     }
     *obj = qnull();
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 197139c1c0..3f1b9e9b41 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -353,8 +353,8 @@ static bool parse_type_number(Visitor *v, const char *name, double *obj,
 
     assert(siv->lm == LM_NONE);
     if (qemu_strtod_finite(siv->string, NULL, &val)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "number");
+        error_setg(errp, "Invalid parameter type for '%s', expected: number",
+                   name ? name : "null");
         return false;
     }
 
@@ -371,8 +371,8 @@ static bool parse_type_null(Visitor *v, const char *name, QNull **obj,
     *obj = NULL;
 
     if (siv->string[0]) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "null");
+        error_setg(errp, "Invalid parameter type for '%s', expected: null",
+                   name ? name : "null");
         return false;
     }
 
diff --git a/qom/object.c b/qom/object.c
index d4a001cf41..3d96818f7d 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1495,7 +1495,8 @@ char *object_property_get_str(Object *obj, const char *name,
     }
     qstring = qobject_to(QString, ret);
     if (!qstring) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "string");
+        error_setg(errp, "Invalid parameter type for '%s', expected: string",
+                   name);
         retval = NULL;
     } else {
         retval = g_strdup(qstring_get_str(qstring));
@@ -1556,7 +1557,8 @@ bool object_property_get_bool(Object *obj, const char *name,
     }
     qbool = qobject_to(QBool, ret);
     if (!qbool) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "boolean");
+        error_setg(errp, "Invalid parameter type for '%s', expected: boolean",
+                   name);
         retval = false;
     } else {
         retval = qbool_get_bool(qbool);
@@ -1589,7 +1591,8 @@ int64_t object_property_get_int(Object *obj, const char *name,
 
     qnum = qobject_to(QNum, ret);
     if (!qnum || !qnum_get_try_int(qnum, &retval)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "int");
+        error_setg(errp, "Invalid parameter type for '%s', expected: int",
+                   name);
         retval = -1;
     }
 
@@ -1663,7 +1666,8 @@ uint64_t object_property_get_uint(Object *obj, const char *name,
     }
     qnum = qobject_to(QNum, ret);
     if (!qnum || !qnum_get_try_uint(qnum, &retval)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "uint");
+        error_setg(errp, "Invalid parameter type for '%s', expected: uint",
+                   name);
         retval = 0;
     }
 
-- 
2.44.0

Re: [PATCH 06/10] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition (constant value)

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:39PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:39 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 06/10] qapi: Inline QERR_INVALID_PARAMETER_TYPE definition
>  (constant value)
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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 the following
> coccinelle semantic patch:
> 
>     @match@
>     expression errp;
>     expression param;
>     constant value;
>     @@
>          error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value);
> 
>     @script:python strformat depends on match@
>     value << match.value;
>     fixedfmt; // new var
>     @@
>     fixedfmt = f'"Invalid parameter type for \'%s\', expected: {value[1:-1]}"'
>     coccinelle.fixedfmt = cocci.make_ident(fixedfmt)
> 
>     @replace@
>     expression match.errp;
>     expression match.param;
>     constant match.value;
>     identifier strformat.fixedfmt;
>     @@
>     -    error_setg(errp, QERR_INVALID_PARAMETER_TYPE, param, value);
>     +    error_setg(errp, fixedfmt, param);
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/qobject-input-visitor.c | 32 ++++++++++++++++----------------
>  qapi/string-input-visitor.c  |  8 ++++----
>  qom/object.c                 | 12 ++++++++----
>  3 files changed, 28 insertions(+), 24 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 07/10] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition

[Markus Armbruster]

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

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!
   */

Manual changes (escaping the format in qapi/visit.py).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 qom/object.c              | 4 ++--
 scripts/qapi/visit.py     | 5 ++---
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 63ab775dc5..b723830eff 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,9 +17,6 @@
  * add new ones!
  */
 
-#define QERR_INVALID_PARAMETER_TYPE \
-    "Invalid parameter type for '%s', expected: %s"
-
 #define QERR_INVALID_PARAMETER_VALUE \
     "Parameter '%s' expects %s"
 
diff --git a/qom/object.c b/qom/object.c
index 3d96818f7d..44ec8f6460 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -23,7 +23,6 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/forward-visitor.h"
 #include "qapi/qapi-builtin-visit.h"
-#include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
 #include "trace.h"
 
@@ -1912,7 +1911,8 @@ static Object *object_resolve_link(Object *obj, const char *name,
     } else if (!target) {
         target = object_resolve_path(path, &ambiguous);
         if (target || ambiguous) {
-            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, target_type);
+            error_setg(errp, "Invalid parameter type for '%s', expected: %s",
+                             name, target_type);
         } else {
             error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                       "Device '%s' not found", path);
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index c56ea4d724..a21b7b1468 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -278,8 +278,8 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str:
         abort();
     default:
         assert(visit_is_input(v));
-        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
-                   "%(name)s");
+        error_setg(errp, "Invalid parameter type for '%%s', expected: %(name)s",
+                         name ? name : "null");
         /* Avoid passing invalid *obj to qapi_free_%(c_name)s() */
         g_free(*obj);
         *obj = NULL;
@@ -356,7 +356,6 @@ def _begin_user_module(self, name: str) -> None:
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-#include "qapi/qmp/qerror.h"
 #include "%(visit)s.h"
 ''',
                                       visit=visit))
-- 
2.44.0

Re: [PATCH 07/10] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:40PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:40 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 07/10] qapi: Inline and remove QERR_INVALID_PARAMETER_TYPE
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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!
>    */
> 
> Manual changes (escaping the format in qapi/visit.py).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  qom/object.c              | 4 ++--
>  scripts/qapi/visit.py     | 5 ++---
>  3 files changed, 4 insertions(+), 8 deletions(-)
>

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Markus Armbruster]

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

QERR_INVALID_PARAMETER_VALUE is defined as:

  #define QERR_INVALID_PARAMETER_VALUE \
      "Parameter '%s' expects %s"

The current error is formatted as:

  "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s"

Replace by:

  "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 migration/options.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index 40eb930940..c5115f1b5c 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1264,9 +1264,8 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
 
     if (params->has_vcpu_dirty_limit &&
         (params->vcpu_dirty_limit < 1)) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "vcpu_dirty_limit",
-                   "is invalid, it must greater then 1 MB/s");
+        error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid,"
+                         " it must greater than 1 MB/s");
         return false;
     }
 
-- 
2.44.0

Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:41 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit'
>  parameter
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> QERR_INVALID_PARAMETER_VALUE is defined as:
> 
>   #define QERR_INVALID_PARAMETER_VALUE \
>       "Parameter '%s' expects %s"
> 
> The current error is formatted as:
> 
>   "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s"
> 
> Replace by:
> 
>   "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"

Is there a grammar error here? Maybe

s/it must greater/it must be greater/

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  migration/options.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/options.c b/migration/options.c
> index 40eb930940..c5115f1b5c 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -1264,9 +1264,8 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>  
>      if (params->has_vcpu_dirty_limit &&
>          (params->vcpu_dirty_limit < 1)) {
> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> -                   "vcpu_dirty_limit",
> -                   "is invalid, it must greater then 1 MB/s");
> +        error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid,"
> +                         " it must greater than 1 MB/s");
>          return false;
>      }
>

Otherwise,

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Philippe Mathieu-Daudé]

On 12/3/24 16:26, Zhao Liu wrote:
> On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote:
>> Date: Tue, 12 Mar 2024 15:13:41 +0100
>> From: Markus Armbruster <armbru@redhat.com>
>> Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit'
>>   parameter
>>
>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
>> QERR_INVALID_PARAMETER_VALUE is defined as:
>>
>>    #define QERR_INVALID_PARAMETER_VALUE \
>>        "Parameter '%s' expects %s"
>>
>> The current error is formatted as:
>>
>>    "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s"
>>
>> Replace by:
>>
>>    "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"
> 
> Is there a grammar error here? Maybe
> 
> s/it must greater/it must be greater/

Oops indeed!

> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   migration/options.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/options.c b/migration/options.c
>> index 40eb930940..c5115f1b5c 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -1264,9 +1264,8 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>>   
>>       if (params->has_vcpu_dirty_limit &&
>>           (params->vcpu_dirty_limit < 1)) {
>> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>> -                   "vcpu_dirty_limit",
>> -                   "is invalid, it must greater then 1 MB/s");
>> +        error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid,"
>> +                         " it must greater than 1 MB/s");

So s/it must greater/it must be greater/ :)

>>           return false;
>>       }
>>
> 
> Otherwise,
> 
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
> 

Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Markus Armbruster]

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

> On 12/3/24 16:26, Zhao Liu wrote:
>> On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote:
>>> Date: Tue, 12 Mar 2024 15:13:41 +0100
>>> From: Markus Armbruster <armbru@redhat.com>
>>> Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit'
>>>   parameter
>>>
>>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>> QERR_INVALID_PARAMETER_VALUE is defined as:
>>>
>>>    #define QERR_INVALID_PARAMETER_VALUE \
>>>        "Parameter '%s' expects %s"
>>>
>>> The current error is formatted as:
>>>
>>>    "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s"
>>>
>>> Replace by:
>>>
>>>    "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"
>>
>> Is there a grammar error here? Maybe
>> s/it must greater/it must be greater/
>
> Oops indeed!

What about dropping "is invalid, "?  I.e. go with

    "Parameter 'vcpu_dirty_limit' must be greater than 1 MB/s"

>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>   migration/options.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/migration/options.c b/migration/options.c
>>> index 40eb930940..c5115f1b5c 100644
>>> --- a/migration/options.c
>>> +++ b/migration/options.c
>>> @@ -1264,9 +1264,8 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>>>         if (params->has_vcpu_dirty_limit &&
>>>           (params->vcpu_dirty_limit < 1)) {
>>> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>>> -                   "vcpu_dirty_limit",
>>> -                   "is invalid, it must greater then 1 MB/s");
>>> +        error_setg(errp, "Parameter 'vcpu_dirty_limit' is invalid,"
>>> +                         " it must greater than 1 MB/s");
>
> So s/it must greater/it must be greater/ :)
>
>>>           return false;
>>>       }
>>>
>> Otherwise,
>> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>> 

Re: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit' parameter

[Philippe Mathieu-Daudé]

On 18/3/24 09:58, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 12/3/24 16:26, Zhao Liu wrote:
>>> On Tue, Mar 12, 2024 at 03:13:41PM +0100, Markus Armbruster wrote:
>>>> Date: Tue, 12 Mar 2024 15:13:41 +0100
>>>> From: Markus Armbruster <armbru@redhat.com>
>>>> Subject: [PATCH 08/10] qapi: Correct error message for 'vcpu_dirty_limit'
>>>>    parameter
>>>>
>>>> From: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>
>>>> QERR_INVALID_PARAMETER_VALUE is defined as:
>>>>
>>>>     #define QERR_INVALID_PARAMETER_VALUE \
>>>>         "Parameter '%s' expects %s"
>>>>
>>>> The current error is formatted as:
>>>>
>>>>     "Parameter 'vcpu_dirty_limit' expects is invalid, it must greater then 1 MB/s"
>>>>
>>>> Replace by:
>>>>
>>>>     "Parameter 'vcpu_dirty_limit' is invalid, it must greater than 1 MB/s"
>>>
>>> Is there a grammar error here? Maybe
>>> s/it must greater/it must be greater/
>>
>> Oops indeed!
> 
> What about dropping "is invalid, "?  I.e. go with
> 
>      "Parameter 'vcpu_dirty_limit' must be greater than 1 MB/s"

Yes.

> 
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>>    migration/options.c | 5 ++---
>>>>    1 file changed, 2 insertions(+), 3 deletions(-)

[PATCH 09/10] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition

[Markus Armbruster]

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

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>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 migration/migration.c     | 2 +-
 migration/options.c       | 4 ++--
 migration/savevm.c        | 2 +-
 4 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index b723830eff..385a4876d6 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -23,9 +23,6 @@
 #define QERR_IO_ERROR \
     "An IO error has occurred"
 
-#define QERR_MIGRATION_ACTIVE \
-    "There's a migration process in progress"
-
 #define QERR_MISSING_PARAMETER \
     "Parameter '%s' is missing"
 
diff --git a/migration/migration.c b/migration/migration.c
index a49fcd53ee..e4a68dfb62 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1961,7 +1961,7 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
     }
 
     if (migration_is_running(s->state)) {
-        error_setg(errp, QERR_MIGRATION_ACTIVE);
+        error_setg(errp, "There's a migration process in progress");
         return false;
     }
 
diff --git a/migration/options.c b/migration/options.c
index c5115f1b5c..5a446e8925 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -682,7 +682,7 @@ bool migrate_cap_set(int cap, bool value, Error **errp)
     bool new_caps[MIGRATION_CAPABILITY__MAX];
 
     if (migration_is_running(s->state)) {
-        error_setg(errp, QERR_MIGRATION_ACTIVE);
+        error_setg(errp, "There's a migration process in progress");
         return false;
     }
 
@@ -726,7 +726,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     bool new_caps[MIGRATION_CAPABILITY__MAX];
 
     if (migration_is_running(s->state) || migration_in_colo_state()) {
-        error_setg(errp, QERR_MIGRATION_ACTIVE);
+        error_setg(errp, "There's a migration process in progress");
         return;
     }
 
diff --git a/migration/savevm.c b/migration/savevm.c
index dc1fb9c0d3..19ca297e15 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1706,7 +1706,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     MigrationStatus status;
 
     if (migration_is_running(ms->state)) {
-        error_setg(errp, QERR_MIGRATION_ACTIVE);
+        error_setg(errp, "There's a migration process in progress");
         return -EINVAL;
     }
 
-- 
2.44.0

Re: [PATCH 09/10] qapi: Inline and remove QERR_MIGRATION_ACTIVE definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:42PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:42 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 09/10] qapi: Inline and remove QERR_MIGRATION_ACTIVE
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  migration/migration.c     | 2 +-
>  migration/options.c       | 4 ++--
>  migration/savevm.c        | 2 +-
>  4 files changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

[PATCH 10/10] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition

[Markus Armbruster]

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

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!
   */

Manual change. Remove the definition in
include/qapi/qmp/qerror.h.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 hw/core/qdev-properties.c | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 385a4876d6..00b18e9082 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -26,9 +26,6 @@
 #define QERR_MISSING_PARAMETER \
     "Parameter '%s' is missing"
 
-#define QERR_PROPERTY_VALUE_BAD \
-    "Property '%s.%s' doesn't take value '%s'"
-
 #define QERR_PROPERTY_VALUE_OUT_OF_RANGE \
     "Property %s.%s doesn't take value %" PRId64 " (minimum: %" PRId64 ", maximum: %" PRId64 ")"
 
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 7d6fa726fd..86a583574d 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -2,7 +2,6 @@
 #include "hw/qdev-properties.h"
 #include "qapi/error.h"
 #include "qapi/qapi-types-misc.h"
-#include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qlist.h"
 #include "qemu/ctype.h"
 #include "qemu/error-report.h"
@@ -792,7 +791,7 @@ void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
         break;
     default:
     case -EINVAL:
-        error_setg(errp, QERR_PROPERTY_VALUE_BAD,
+        error_setg(errp, "Property '%s.%s' doesn't take value '%s'",
                    object_get_typename(obj), name, value);
         break;
     case -ENOENT:
-- 
2.44.0

Re: [PATCH 10/10] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD definition

[Zhao Liu]

On Tue, Mar 12, 2024 at 03:13:43PM +0100, Markus Armbruster wrote:
> Date: Tue, 12 Mar 2024 15:13:43 +0100
> From: Markus Armbruster <armbru@redhat.com>
> Subject: [PATCH 10/10] qapi: Inline and remove QERR_PROPERTY_VALUE_BAD
>  definition
> 
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> 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!
>    */
> 
> Manual change. Remove the definition in
> include/qapi/qmp/qerror.h.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 3 ---
>  hw/core/qdev-properties.c | 3 +--
>  2 files changed, 1 insertion(+), 5 deletions(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 00/10] Reduce usage of QERR_ macros further

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Philippe posted "[PATCH v2 00/22] qapi: Kill 'qapi/qmp/qerror.h' for
> good" a couple of months ago.  I cherry-picked just its simplest parts
> for now.
>
> Markus Armbruster (1):
>   error: Drop superfluous #include "qapi/qmp/qerror.h"

Queued for 9.1 with PATCH 08's new error message fixed.  Thanks!