[PATCH 00/20] qdev/qom: Remove explicit type names from error_setg() calls

[PATCH 00/20] qdev/qom: Remove explicit type names from error_setg() calls

[Eduardo Habkost]

Based-on: 20201029220246.472693-1-ehabkost@redhat.com
Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/prop-error-reporting

One of the obstacles when refactoring the QOM property parsing
code are the references to the object itself in the error code,
to generate "Property '<TYPE>.<PROP>' can't take value ..." error
messages.  This makes it hard to isolate the string parsing code
into functions that are independent from QOM.

The simple solution for that would be to just remove the prefix
and have less descriptive error messages.

A slightly better solution is to make the code that parses
-device and -object add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>': "
prefix automatically when encountering an error
when setting a property.  This is the approach implemented in
this series.

Eduardo Habkost (20):
  qom: Add prefix to error message inside object_property_parse()
  qdev: Stop using error_set_from_qdev_prop_error() for netdev property
  qdev: Stop using error_set_from_qdev_prop_error() for audiodev
    property
  qdev: Stop using error_set_from_qdev_prop_error() for mac property
  qdev: Stop using error_set_from_qdev_prop_error() for devfn property
  qdev: Stop using error_set_from_qdev_prop_error() for PCI host device
    property
  qdev: Stop using error_set_from_qdev_prop_error() for css devno
    property
  qdev: Delete unused error_set_from_qdev_prop_error() function
  cryptodev: Remove unnecessary prefix from error message
  memfd: Remove unnecessary prefix from error message
  tpm_util: Remove unnecessary prefix from error message
  qdev: drive: Remove unnecessary prefix from error message
  qdev: chardev: Remove unnecessary prefix from error message
  i386: Remove unnecessary prefix from error message
  qerror: Delete unused QERR_PROPERTY_VALUE_BAD macro
  nvdimm: Remove unnecessary prefix from error message
  colo-compare: Remove unnecessary prefix from error message
  filter-dump: Remove unnecessary prefix from error message
  filter-buffer: Remove unnecessary prefix from error message
  qom: Remove error prefix check at object_property_parse()

 include/hw/qdev-properties.h     |  2 --
 include/qapi/qmp/qerror.h        |  3 ---
 backends/cryptodev.c             |  3 +--
 backends/hostmem-memfd.c         |  3 +--
 backends/tpm/tpm_util.c          |  3 +--
 hw/core/qdev-properties-system.c | 28 +++++++++++-----------------
 hw/core/qdev-properties.c        | 22 ----------------------
 hw/mem/nvdimm.c                  |  6 ++----
 hw/s390x/css.c                   |  2 +-
 net/colo-compare.c               |  9 +++------
 net/dump.c                       |  3 +--
 net/filter-buffer.c              |  3 +--
 qom/object.c                     |  5 +++++
 target/i386/cpu.c                |  2 +-
 14 files changed, 28 insertions(+), 66 deletions(-)

-- 
2.28.0

[PATCH 01/20] qom: Add prefix to error message inside object_property_parse()

[Eduardo Habkost]

Make object_property_parse() automatically add a error message
prefix mentioning the QOM type and property name when
encountering errors.

As we have a large number of functions that add their own
"Property '...'" to the error messages, add a temporary check for
existing prefixes before prepending our own.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 qom/object.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/qom/object.c b/qom/object.c
index 20726e4584..6fb1657724 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1635,9 +1635,20 @@ int object_property_get_enum(Object *obj, const char *name,
 bool object_property_parse(Object *obj, const char *name,
                            const char *string, Error **errp)
 {
+    ERRP_GUARD();
     Visitor *v = string_input_visitor_new(string);
     bool ok = object_property_set(obj, name, v, errp);
 
+    if (!ok) {
+        /*
+         * Temporary check for existing prefix, until all error reporting
+         * functions remove their own prefix.
+         */
+        if (!g_str_has_prefix(error_get_pretty(*errp), "Property '")) {
+            error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
+                        object_get_typename(obj), name, string);
+        }
+    }
     visit_free(v);
     return ok;
 }
-- 
2.28.0

[PATCH 02/20] qdev: Stop using error_set_from_qdev_prop_error() for netdev property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-x86_64 -device e1000,netdev=n0
  qemu-system-x86_64: -device e1000,netdev=n0: Property 'e1000.netdev' can't find value 'n0'
  $ qemu-system-x86_64 -netdev id=n0,type=user -device e1000,netdev=n0 -device e1000,netdev=n0
  qemu-system-x86_64: -device e1000,netdev=n0: Property 'e1000.netdev' can't take value 'n0', it's in use

After:

  $ ./qemu-system-x86_64 -device e1000,netdev=n0
  qemu-system-x86_64: -device e1000,netdev=n0: Property 'e1000.netdev' can't take value 'n0': netdev not found
  $ ./qemu-system-x86_64 -netdev id=n0,type=user -device e1000,netdev=n0 -device e1000,netdev=n0
  qemu-system-x86_64: -device e1000,netdev=n0: Property 'e1000.netdev' can't take value 'n0': netdev is in use

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 232ff955fa..60be09efba 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -376,7 +376,7 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
     NICPeers *peers_ptr = object_static_prop_ptr(obj, prop);
     NetClientState **ncs = peers_ptr->ncs;
     NetClientState *peers[MAX_QUEUE_NUM];
-    int queues, err = 0, i = 0;
+    int queues, i = 0;
     char *str;
 
     if (!visit_type_str(v, name, &str, errp)) {
@@ -387,7 +387,7 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
                                           NET_CLIENT_DRIVER_NIC,
                                           MAX_QUEUE_NUM);
     if (queues == 0) {
-        err = -ENOENT;
+        error_setg(errp, "netdev not found");
         goto out;
     }
 
@@ -399,7 +399,7 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
 
     for (i = 0; i < queues; i++) {
         if (peers[i]->peer) {
-            err = -EEXIST;
+            error_setg(errp, "netdev is in use");
             goto out;
         }
 
@@ -418,7 +418,6 @@ static void set_netdev(Object *obj, Visitor *v, const char *name,
     peers_ptr->queues = queues;
 
 out:
-    error_set_from_qdev_prop_error(errp, err, obj, prop, str);
     g_free(str);
 }
 
-- 
2.28.0

[PATCH 03/20] qdev: Stop using error_set_from_qdev_prop_error() for audiodev property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-x86_64 -device AC97,audiodev=a0
  qemu-system-x86_64: -device AC97,audiodev=a0: Property 'AC97.audiodev' can't find value 'a0'

After:

  $ ./qemu-system-x86_64 -device AC97,audiodev=a0
  qemu-system-x86_64: -device AC97,audiodev=a0: Property 'AC97.audiodev' can't take value 'a0': audiodev not found

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 60be09efba..a5a1a6ffee 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -447,7 +447,6 @@ static void set_audiodev(Object *obj, Visitor *v, const char* name,
     Property *prop = opaque;
     QEMUSoundCard *card = object_static_prop_ptr(obj, prop);
     AudioState *state;
-    int err = 0;
     char *str;
 
     if (!visit_type_str(v, name, &str, errp)) {
@@ -457,13 +456,12 @@ static void set_audiodev(Object *obj, Visitor *v, const char* name,
     state = audio_state_by_name(str);
 
     if (!state) {
-        err = -ENOENT;
+        error_setg(errp, "audiodev not found");
         goto out;
     }
     card->state = state;
 
 out:
-    error_set_from_qdev_prop_error(errp, err, obj, prop, str);
     g_free(str);
 }
 
-- 
2.28.0

[PATCH 04/20] qdev: Stop using error_set_from_qdev_prop_error() for mac property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-x86_64 -device e1000,mac=x
  qemu-system-x86_64: -device e1000,mac=x: Property 'e1000.mac' doesn't take value 'x'

After:

  $ qemu-system-x86_64 -device e1000,mac=x
  qemu-system-x86_64: -device e1000,mac=x: Property 'e1000.mac' can't take value 'x': invalid mac address

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index a5a1a6ffee..fc0abcae17 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -336,7 +336,7 @@ static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
     return;
 
 inval:
-    error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+    error_setg(errp, "invalid mac address");
     g_free(str);
 }
 
-- 
2.28.0

[PATCH 05/20] qdev: Stop using error_set_from_qdev_prop_error() for devfn property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-x86_64 -device e1000,addr=x
  qemu-system-x86_64: -device e1000,addr=x: Property 'e1000.addr' doesn't take value 'x'

After:

  $ qemu-system-x86_64 -device e1000,addr=x
  qemu-system-x86_64: -device e1000,addr=x: Property 'e1000.addr' can't take value 'x': invalid PCI address

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index fc0abcae17..5d3cb32708 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -742,7 +742,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, const char *name,
     return;
 
 invalid:
-    error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+    error_setg(errp, "invalid PCI address");
     g_free(str);
 }
 
-- 
2.28.0

[PATCH 06/20] qdev: Stop using error_set_from_qdev_prop_error() for PCI host device property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-x86_64 -device vfio-pci,host=x
  qemu-system-x86_64: -device vfio-pci,host=x: Property 'vfio-pci.host' doesn't take value 'x'

After:

  $ qemu-system-x86_64 -device vfio-pci,host=x
  qemu-system-x86_64: -device vfio-pci,host=x: Property 'vfio-pci.host' can't take value 'x': invalid host device address

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 5d3cb32708..0290d64bf1 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -859,7 +859,7 @@ static void set_pci_host_devaddr(Object *obj, Visitor *v, const char *name,
     return;
 
 inval:
-    error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+    error_setg(errp, "invalid host device address");
     g_free(str);
 }
 
-- 
2.28.0

[PATCH 07/20] qdev: Stop using error_set_from_qdev_prop_error() for css devno property

[Eduardo Habkost]

Just setting a reasonable error string using error_setg() is
simpler and makes error messages clearer.

Before:

  $ qemu-system-s390x -device x-terminal3270,devno=x
  qemu-system-s390x: -device x-terminal3270,devno=x: Property 'x-terminal3270.devno' doesn't take value 'x'

After:

  $ qemu-system-s390x -device x-terminal3270,devno=x
  qemu-system-s390x: -device x-terminal3270,devno=x: Property 'x-terminal3270.devno' can't take value 'x': invalid devno

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: David Hildenbrand <david@redhat.com>
Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: qemu-s390x@nongnu.org
Cc: qemu-devel@nongnu.org
---
 hw/s390x/css.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index c8e7ce232a..ad631bad29 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -2384,7 +2384,7 @@ static void set_css_devid(Object *obj, Visitor *v, const char *name,
 
     num = sscanf(str, "%2x.%1x%n.%4x%n", &cssid, &ssid, &n1, &devid, &n2);
     if (num != 3 || (n2 - n1) != 5 || strlen(str) != n2) {
-        error_set_from_qdev_prop_error(errp, EINVAL, obj, prop, str);
+        error_setg(errp, "invalid devno");
         goto out;
     }
     if ((cssid > MAX_CSSID) || (ssid > MAX_SSID)) {
-- 
2.28.0

[PATCH 08/20] qdev: Delete unused error_set_from_qdev_prop_error() function

[Eduardo Habkost]

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 include/hw/qdev-properties.h |  2 --
 hw/core/qdev-properties.c    | 22 ----------------------
 2 files changed, 24 deletions(-)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index a4ac1e6972..cca1a2153f 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -36,8 +36,6 @@ const GlobalProperty *qdev_find_global_prop(Object *obj,
                                             const char *name);
 int qdev_prop_check_globals(void);
 void qdev_prop_set_globals(DeviceState *dev);
-void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
-                                    Property *prop, const char *value);
 
 /**
  * qdev_property_add_static:
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index c03842e89a..5fdfb3f5cd 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -85,28 +85,6 @@ static Property *qdev_prop_find(DeviceState *dev, const char *name)
     return NULL;
 }
 
-void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
-                                    Property *prop, const char *value)
-{
-    switch (ret) {
-    case -EEXIST:
-        error_setg(errp, "Property '%s.%s' can't take value '%s', it's in use",
-                  object_get_typename(obj), prop->name, value);
-        break;
-    default:
-    case -EINVAL:
-        error_setg(errp, QERR_PROPERTY_VALUE_BAD,
-                   object_get_typename(obj), prop->name, value);
-        break;
-    case -ENOENT:
-        error_setg(errp, "Property '%s.%s' can't find value '%s'",
-                  object_get_typename(obj), prop->name, value);
-        break;
-    case 0:
-        break;
-    }
-}
-
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
     object_property_set_bool(OBJECT(dev), name, value, &error_abort);
-- 
2.28.0

[PATCH 09/20] cryptodev: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: "Gonglei (Arei)" <arei.gonglei@huawei.com>
Cc: qemu-devel@nongnu.org
---
 backends/cryptodev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 3f141f61ed..e3cd90619c 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -160,8 +160,7 @@ cryptodev_backend_set_queues(Object *obj, Visitor *v, const char *name,
         return;
     }
     if (!value) {
-        error_setg(errp, "Property '%s.%s' doesn't take value '%" PRIu32 "'",
-                   object_get_typename(obj), name, value);
+        error_setg(errp, "queues can't be zero");
         return;
     }
     backend->conf.peers.queues = value;
-- 
2.28.0

[PATCH 10/20] memfd: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org
---
 backends/hostmem-memfd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index e5626d4330..05cf743fe8 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -87,8 +87,7 @@ memfd_backend_set_hugetlbsize(Object *obj, Visitor *v, const char *name,
         return;
     }
     if (!value) {
-        error_setg(errp, "Property '%s.%s' doesn't take value '%" PRIu64 "'",
-                   object_get_typename(obj), name, value);
+        error_setg(errp, "hugetlbsize can't be zero");
         return;
     }
     m->hugetlbsize = value;
-- 
2.28.0

[PATCH 11/20] tpm_util: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: qemu-devel@nongnu.org
---
 backends/tpm/tpm_util.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 2b5f788861..012c71bc53 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -56,8 +56,7 @@ static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
 
     s = qemu_find_tpm_be(str);
     if (s == NULL) {
-        error_setg(errp, "Property '%s.%s' can't find value '%s'",
-                   object_get_typename(obj), prop->name, str);
+        error_setg(errp, "tpm backend not found");
     } else if (tpm_backend_init(s, TPM_IF(obj), errp) == 0) {
         *be = s; /* weak reference, avoid cyclic ref */
     }
-- 
2.28.0

[PATCH 12/20] qdev: drive: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 0290d64bf1..815eee8035 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -134,8 +134,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
         }
     }
     if (!blk) {
-        error_setg(errp, "Property '%s.%s' can't find value '%s'",
-                   object_get_typename(OBJECT(dev)), prop->name, str);
+        error_setg(errp, "block backend not found");
         goto fail;
     }
     if (blk_attach_dev(blk, dev) < 0) {
-- 
2.28.0

[PATCH 13/20] qdev: chardev: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/core/qdev-properties-system.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 815eee8035..90799412be 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -248,11 +248,9 @@ static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
 
     s = qemu_chr_find(str);
     if (s == NULL) {
-        error_setg(errp, "Property '%s.%s' can't find value '%s'",
-                   object_get_typename(obj), prop->name, str);
-    } else if (!qemu_chr_fe_init(be, s, errp)) {
-        error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
-                      object_get_typename(obj), prop->name, str);
+        error_setg(errp, "chardev not found");
+    } else {
+        qemu_chr_fe_init(be, s, errp);
     }
     g_free(str);
 }
-- 
2.28.0

[PATCH 14/20] i386: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 0d8606958e..2c00f94308 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4529,7 +4529,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value,
     int i;
 
     if (strlen(value) != CPUID_VENDOR_SZ) {
-        error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value);
+        error_setg(errp, "invalid vendor ID");
         return;
     }
 
-- 
2.28.0

[PATCH 15/20] qerror: Delete unused QERR_PROPERTY_VALUE_BAD macro

[Eduardo Habkost]

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
---
 include/qapi/qmp/qerror.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 7c76e24aa7..646a42c4b4 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -70,9 +70,6 @@
 #define QERR_PERMISSION_DENIED \
     "Insufficient permission to perform this operation"
 
-#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 ")"
 
-- 
2.28.0

[PATCH 16/20] nvdimm: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: qemu-devel@nongnu.org
---
 hw/mem/nvdimm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index e1574bc07c..b9a99d58ed 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -56,8 +56,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name,
         return;
     }
     if (value < MIN_NAMESPACE_LABEL_SIZE) {
-        error_setg(errp, "Property '%s.%s' (0x%" PRIx64 ") is required"
-                   " at least 0x%lx", object_get_typename(obj), name, value,
+        error_setg(errp, "label size should be at least 0x%lx",
                    MIN_NAMESPACE_LABEL_SIZE);
         return;
     }
@@ -89,8 +88,7 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name,
     }
 
     if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) {
-        error_setg(errp, "Property '%s.%s' has invalid value",
-                   object_get_typename(obj), name);
+        error_setg(errp, "invalid UUID");
     }
 
     g_free(value);
-- 
2.28.0

[PATCH 17/20] colo-compare: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Zhang Chen <chen.zhang@intel.com>
Cc: Li Zhijian <lizhijian@cn.fujitsu.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org
---
 net/colo-compare.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 3a45d64175..ae61e32c5b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -1097,8 +1097,7 @@ static void compare_set_timeout(Object *obj, Visitor *v,
         return;
     }
     if (!value) {
-        error_setg(errp, "Property '%s.%s' requires a positive value",
-                   object_get_typename(obj), name);
+        error_setg(errp, "timeout can't be zero");
         return;
     }
     s->compare_timeout = value;
@@ -1125,8 +1124,7 @@ static void compare_set_expired_scan_cycle(Object *obj, Visitor *v,
         return;
     }
     if (!value) {
-        error_setg(errp, "Property '%s.%s' requires a positive value",
-                   object_get_typename(obj), name);
+        error_setg(errp, "expired_scan_cycle can't be zero");
         return;
     }
     s->expired_scan_cycle = value;
@@ -1153,8 +1151,7 @@ static void set_max_queue_size(Object *obj, Visitor *v,
         goto out;
     }
     if (!value) {
-        error_setg(&local_err, "Property '%s.%s' requires a positive value",
-                   object_get_typename(obj), name);
+        error_setg(errp, "queue size can't be zero");
         goto out;
     }
     max_queue_size = value;
-- 
2.28.0

[PATCH 18/20] filter-dump: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org
---
 net/dump.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/dump.c b/net/dump.c
index 7fd448d2e1..16c4fa1588 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -197,8 +197,7 @@ static void filter_dump_set_maxlen(Object *obj, Visitor *v, const char *name,
         return;
     }
     if (value == 0) {
-        error_setg(errp, "Property '%s.%s' doesn't take value '%u'",
-                   object_get_typename(obj), name, value);
+        error_setg(errp, "maxlen can't be zero");
         return;
     }
     nfds->maxlen = value;
-- 
2.28.0

[PATCH 19/20] filter-buffer: Remove unnecessary prefix from error message

[Eduardo Habkost]

object_property_parse() will add a
  "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
prefix automatically for us.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Jason Wang <jasowang@redhat.com>
Cc: qemu-devel@nongnu.org
---
 net/filter-buffer.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index d8392be53c..2c70dc6c33 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -175,8 +175,7 @@ static void filter_buffer_set_interval(Object *obj, Visitor *v,
         return;
     }
     if (!value) {
-        error_setg(errp, "Property '%s.%s' requires a positive value",
-                   object_get_typename(obj), name);
+        error_setg(errp, "interval can't be zero");
         return;
     }
     s->interval = value;
-- 
2.28.0

[PATCH 20/20] qom: Remove error prefix check at object_property_parse()

[Eduardo Habkost]

All existing "Property '<TYPE>.<PROP>' ..." error messages were
rewritten, we can now add the error message prefix unconditionally.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org
---
 qom/object.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index 6fb1657724..4fc536716c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1640,14 +1640,8 @@ bool object_property_parse(Object *obj, const char *name,
     bool ok = object_property_set(obj, name, v, errp);
 
     if (!ok) {
-        /*
-         * Temporary check for existing prefix, until all error reporting
-         * functions remove their own prefix.
-         */
-        if (!g_str_has_prefix(error_get_pretty(*errp), "Property '")) {
-            error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
-                        object_get_typename(obj), name, string);
-        }
+        error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
+                      object_get_typename(obj), name, string);
     }
     visit_free(v);
     return ok;
-- 
2.28.0

Re: [PATCH 10/20] memfd: Remove unnecessary prefix from error message

[Igor Mammedov]

On Fri, 30 Oct 2020 16:21:21 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> object_property_parse() will add a
>   "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
> prefix automatically for us.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  backends/hostmem-memfd.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index e5626d4330..05cf743fe8 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -87,8 +87,7 @@ memfd_backend_set_hugetlbsize(Object *obj, Visitor *v, const char *name,
>          return;
>      }
>      if (!value) {
> -        error_setg(errp, "Property '%s.%s' doesn't take value '%" PRIu64 "'",
> -                   object_get_typename(obj), name, value);
> +        error_setg(errp, "hugetlbsize can't be zero");
>          return;
>      }
>      m->hugetlbsize = value;

Re: [PATCH 16/20] nvdimm: Remove unnecessary prefix from error message

[Igor Mammedov]

On Fri, 30 Oct 2020 16:21:27 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> object_property_parse() will add a
>   "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
> prefix automatically for us.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  hw/mem/nvdimm.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
> index e1574bc07c..b9a99d58ed 100644
> --- a/hw/mem/nvdimm.c
> +++ b/hw/mem/nvdimm.c
> @@ -56,8 +56,7 @@ static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name,
>          return;
>      }
>      if (value < MIN_NAMESPACE_LABEL_SIZE) {
> -        error_setg(errp, "Property '%s.%s' (0x%" PRIx64 ") is required"
> -                   " at least 0x%lx", object_get_typename(obj), name, value,
> +        error_setg(errp, "label size should be at least 0x%lx",
>                     MIN_NAMESPACE_LABEL_SIZE);
>          return;
>      }
> @@ -89,8 +88,7 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const char *name,
>      }
>  
>      if (qemu_uuid_parse(value, &nvdimm->uuid) != 0) {
> -        error_setg(errp, "Property '%s.%s' has invalid value",
> -                   object_get_typename(obj), name);
> +        error_setg(errp, "invalid UUID");
>      }
>  
>      g_free(value);

Re: [PATCH 01/20] qom: Add prefix to error message inside object_property_parse()

[Igor Mammedov]

On Fri, 30 Oct 2020 16:21:12 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Make object_property_parse() automatically add a error message
> prefix mentioning the QOM type and property name when
> encountering errors.
> 
> As we have a large number of functions that add their own
> "Property '...'" to the error messages, add a temporary check for
> existing prefixes before prepending our own.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  qom/object.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 20726e4584..6fb1657724 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1635,9 +1635,20 @@ int object_property_get_enum(Object *obj, const char *name,
>  bool object_property_parse(Object *obj, const char *name,
>                             const char *string, Error **errp)
>  {
> +    ERRP_GUARD();
>      Visitor *v = string_input_visitor_new(string);
>      bool ok = object_property_set(obj, name, v, errp);
>  
> +    if (!ok) {
> +        /*
> +         * Temporary check for existing prefix, until all error reporting
> +         * functions remove their own prefix.
> +         */
> +        if (!g_str_has_prefix(error_get_pretty(*errp), "Property '")) {
> +            error_prepend(errp, "Property '%s.%s' can't take value '%s': ",
> +                        object_get_typename(obj), name, string);
> +        }
> +    }
>      visit_free(v);
>      return ok;
>  }

Re: [PATCH 14/20] i386: Remove unnecessary prefix from error message

[Igor Mammedov]

On Fri, 30 Oct 2020 16:21:25 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:

> object_property_parse() will add a
>   "Property '<TYPE>.<PROP>' can't take value '<VALUE>'"
> prefix automatically for us.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
>  target/i386/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 0d8606958e..2c00f94308 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4529,7 +4529,7 @@ static void x86_cpuid_set_vendor(Object *obj, const char *value,
>      int i;
>  
>      if (strlen(value) != CPUID_VENDOR_SZ) {
> -        error_setg(errp, QERR_PROPERTY_VALUE_BAD, "", "vendor", value);
> +        error_setg(errp, "invalid vendor ID");
>          return;
>      }
>