[PATCH 00/10] Chipping away at qerror.h

[PATCH 00/10] Chipping away at qerror.h

[Markus Armbruster]

Obviously not for 5.2.  Please review anyway.

Markus Armbruster (10):
  qerror: Drop unused QERR_ macros
  qerror: Eliminate QERR_ macros used in just one place
  block: Improve some block-commit, block-stream error messages
  ui: Improve some set_passwd, expire_password error messages
  ui: Improve a client_migrate_info error message
  ui: Tweak a client_migrate_info error message
  qga: Replace an unreachable error by abort()
  qga: Tweak a guest-shutdown error message
  qom: Improve {qom,device}-list-properties error messages
  Tweak a few "Parameter 'NAME' expects THING" error message

 include/qapi/qmp/qerror.h        | 23 -------------------
 block/quorum.c                   |  2 +-
 blockdev.c                       | 17 ++++++++------
 chardev/char.c                   |  2 +-
 hw/core/qdev-properties-system.c |  2 +-
 monitor/misc.c                   | 12 +++++-----
 monitor/qmp-cmds.c               | 38 +++++++++++++-------------------
 net/net.c                        |  2 +-
 qga/commands-win32.c             |  5 ++---
 qom/qom-qmp-cmds.c               | 17 +++++---------
 softmmu/qdev-monitor.c           |  4 ++--
 tests/qemu-iotests/040           | 12 +++++-----
 12 files changed, 51 insertions(+), 85 deletions(-)

-- 
2.26.2

[PATCH 01/10] qerror: Drop unused QERR_ macros

[Markus Armbruster]

QERR_INVALID_BLOCK_FORMAT is dead since commit e6641719fe "block:
Always pass NULL as drv for bdrv_open()", 2015-09-14.

QERR_INVALID_PASSWORD is dead since commit c01c214b69 "block: remove
all encryption handling APIs", 2017-07-11.

Bury them.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 7c76e24aa7..3eabd451d8 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -43,9 +43,6 @@
 #define QERR_FEATURE_DISABLED \
     "The feature '%s' is not enabled"
 
-#define QERR_INVALID_BLOCK_FORMAT \
-    "Invalid block format '%s'"
-
 #define QERR_INVALID_PARAMETER \
     "Invalid parameter '%s'"
 
@@ -55,9 +52,6 @@
 #define QERR_INVALID_PARAMETER_VALUE \
     "Parameter '%s' expects %s"
 
-#define QERR_INVALID_PASSWORD \
-    "Password incorrect"
-
 #define QERR_IO_ERROR \
     "An IO error has occurred"
 
-- 
2.26.2

Re: [PATCH 01/10] qerror: Drop unused QERR_ macros

[Philippe Mathieu-Daudé]

On 11/13/20 9:26 AM, Markus Armbruster wrote:
> QERR_INVALID_BLOCK_FORMAT is dead since commit e6641719fe "block:
> Always pass NULL as drv for bdrv_open()", 2015-09-14.
> 
> QERR_INVALID_PASSWORD is dead since commit c01c214b69 "block: remove
> all encryption handling APIs", 2017-07-11.
> 
> Bury them.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qerror.h | 6 ------
>  1 file changed, 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[PATCH 02/10] qerror: Eliminate QERR_ macros used in just one place

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 9 ---------
 monitor/misc.c            | 8 ++++----
 net/net.c                 | 2 +-
 3 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 3eabd451d8..c272e3fc29 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -25,21 +25,12 @@
 #define QERR_DEVICE_HAS_NO_MEDIUM \
     "Device '%s' has no medium"
 
-#define QERR_DEVICE_INIT_FAILED \
-    "Device '%s' could not be initialized"
-
 #define QERR_DEVICE_IN_USE \
     "Device '%s' is in use"
 
 #define QERR_DEVICE_NO_HOTPLUG \
     "Device '%s' does not support hotplugging"
 
-#define QERR_FD_NOT_FOUND \
-    "File descriptor named '%s' not found"
-
-#define QERR_FD_NOT_SUPPLIED \
-    "No file descriptor supplied via SCM_RIGHTS"
-
 #define QERR_FEATURE_DISABLED \
     "The feature '%s' is not enabled"
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 32e6a8c13d..9aa2505cfa 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1232,7 +1232,7 @@ void qmp_getfd(const char *fdname, Error **errp)
 
     fd = qemu_chr_fe_get_msgfd(&cur_mon->chr);
     if (fd == -1) {
-        error_setg(errp, QERR_FD_NOT_SUPPLIED);
+        error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
         return;
     }
 
@@ -1286,7 +1286,7 @@ void qmp_closefd(const char *fdname, Error **errp)
     }
 
     qemu_mutex_unlock(&cur_mon->mon_lock);
-    error_setg(errp, QERR_FD_NOT_FOUND, fdname);
+    error_setg(errp, "File descriptor named '%s' not found", fdname);
 }
 
 int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)
@@ -1357,7 +1357,7 @@ AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
 
     fd = qemu_chr_fe_get_msgfd(&mon->chr);
     if (fd == -1) {
-        error_setg(errp, QERR_FD_NOT_SUPPLIED);
+        error_setg(errp, "No file descriptor supplied via SCM_RIGHTS");
         goto error;
     }
 
@@ -1410,7 +1410,7 @@ error:
     } else {
         snprintf(fd_str, sizeof(fd_str), "fdset-id:%" PRId64, fdset_id);
     }
-    error_setg(errp, QERR_FD_NOT_FOUND, fd_str);
+    error_setg(errp, "File descriptor named '%s' not found", fd_str);
 }
 
 FdsetInfoList *qmp_query_fdsets(Error **errp)
diff --git a/net/net.c b/net/net.c
index 794c652282..5e1b57a608 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1008,7 +1008,7 @@ static int net_client_init1(const Netdev *netdev, bool is_netdev, Error **errp)
     if (net_client_init_fun[netdev->type](netdev, netdev->id, peer, errp) < 0) {
         /* FIXME drop when all init functions store an Error */
         if (errp && !*errp) {
-            error_setg(errp, QERR_DEVICE_INIT_FAILED,
+            error_setg(errp, "Device '%s' could not be initialized",
                        NetClientDriver_str(netdev->type));
         }
         return -1;
-- 
2.26.2

[PATCH 03/10] block: Improve some block-commit, block-stream error messages

[Markus Armbruster]

block-commit defaults @base-node to the deepest backing image.  When
there is none, it fails with "Base 'NULL' not found".  Improve to
"There is no backing image".

block-commit and block-stream reject a @base argument that doesn't
resolve with "Base 'BASE' not found".  Commit 6b33f3ae8b "qemu-img:
Improve commit invalid base message" improved this message in
qemu-img.  Improve it here, too: "Can't find '%s' in the backing
chain".

QERR_BASE_NOT_FOUND is now unused.  Drop.

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h |  2 --
 blockdev.c                | 15 +++++++++------
 tests/qemu-iotests/040    | 12 ++++++------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c272e3fc29..5d7e69cc1f 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -16,8 +16,6 @@
  * These macros will go away, please don't use in new code, and do not
  * add new ones!
  */
-#define QERR_BASE_NOT_FOUND \
-    "Base '%s' not found"
 
 #define QERR_BUS_NO_HOTPLUG \
     "Bus '%s' does not support hotplugging"
diff --git a/blockdev.c b/blockdev.c
index fe6fb5dc1d..d05a8740f4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2531,7 +2531,7 @@ void qmp_block_stream(bool has_job_id, const char *job_id, const char *device,
     if (has_base) {
         base_bs = bdrv_find_backing_image(bs, base);
         if (base_bs == NULL) {
-            error_setg(errp, QERR_BASE_NOT_FOUND, base);
+            error_setg(errp, "Can't find '%s' in the backing chain", base);
             goto out;
         }
         assert(bdrv_get_aio_context(base_bs) == aio_context);
@@ -2703,13 +2703,16 @@ void qmp_block_commit(bool has_job_id, const char *job_id, const char *device,
         }
     } else if (has_base && base) {
         base_bs = bdrv_find_backing_image(top_bs, base);
+        if (base_bs == NULL) {
+            error_setg(errp, "Can't find '%s' in the backing chain", base);
+            goto out;
+        }
     } else {
         base_bs = bdrv_find_base(top_bs);
-    }
-
-    if (base_bs == NULL) {
-        error_setg(errp, QERR_BASE_NOT_FOUND, base ? base : "NULL");
-        goto out;
+        if (base_bs == NULL) {
+            error_setg(errp, "There is no backimg image");
+            goto out;
+        }
     }
 
     assert(bdrv_get_aio_context(base_bs) == aio_context);
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index caf286571a..dc6069edc0 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -156,7 +156,7 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % backing_img, base='%s' % backing_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % backing_img)
+        self.assert_qmp(result, 'error/desc', "Can't find '%s' in the backing chain" % backing_img)
 
     def test_top_invalid(self):
         self.assert_no_active_block_jobs()
@@ -168,7 +168,7 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % mid_img, base='badfile')
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
+        self.assert_qmp(result, 'error/desc', "Can't find 'badfile' in the backing chain")
 
     def test_top_node_invalid(self):
         self.assert_no_active_block_jobs()
@@ -208,7 +208,7 @@ class TestSingleDrive(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % backing_img, base='%s' % mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % mid_img)
+        self.assert_qmp(result, 'error/desc', "Can't find '%s' in the backing chain" % mid_img)
 
     def test_top_and_base_node_reversed(self):
         self.assert_no_active_block_jobs()
@@ -349,7 +349,7 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img, base='%s' % self.mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % self.mid_img)
+        self.assert_qmp(result, 'error/desc', "Can't find '%s' in the backing chain" % self.mid_img)
 
     def test_top_invalid(self):
         self.assert_no_active_block_jobs()
@@ -361,7 +361,7 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.mid_img, base='badfile')
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'badfile\' not found')
+        self.assert_qmp(result, 'error/desc', "Can't find 'badfile' in the backing chain")
 
     def test_top_is_active(self):
         self.run_commit_test(self.test_img, self.backing_img)
@@ -372,7 +372,7 @@ class TestRelativePaths(ImageCommitTestCase):
         self.assert_no_active_block_jobs()
         result = self.vm.qmp('block-commit', device='drive0', top='%s' % self.backing_img, base='%s' % self.mid_img)
         self.assert_qmp(result, 'error/class', 'GenericError')
-        self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % self.mid_img)
+        self.assert_qmp(result, 'error/desc', "Can't find '%s' in the backing chain" % self.mid_img)
 
 
 class TestSetSpeed(ImageCommitTestCase):
-- 
2.26.2

Re: [PATCH 03/10] block: Improve some block-commit, block-stream error messages

[Max Reitz]

On 13.11.20 09:26, Markus Armbruster wrote:
> block-commit defaults @base-node to the deepest backing image.  When
> there is none, it fails with "Base 'NULL' not found".  Improve to
> "There is no backing image".
> 
> block-commit and block-stream reject a @base argument that doesn't
> resolve with "Base 'BASE' not found".  Commit 6b33f3ae8b "qemu-img:
> Improve commit invalid base message" improved this message in
> qemu-img.  Improve it here, too: "Can't find '%s' in the backing
> chain".
> 
> QERR_BASE_NOT_FOUND is now unused.  Drop.
> 
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/qmp/qerror.h |  2 --
>   blockdev.c                | 15 +++++++++------
>   tests/qemu-iotests/040    | 12 ++++++------
>   3 files changed, 15 insertions(+), 14 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

[PATCH 04/10] ui: Improve some set_passwd, expire_password error messages

[Markus Armbruster]

set_passwd and expire_password reject invalid "protocol" with "Invalid
parameter 'protocol'".  Misleading; the parameter is valid, its value
isn't.  Improve to "Parameter 'protocol' expects 'vnc' or 'spice'".

expire_password fails with "Could not set password".  Misleading;
improve to "Could not set password expire time".

QERR_SET_PASSWD_FAILED is now unused.  Drop.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h |  3 ---
 monitor/qmp-cmds.c        | 38 +++++++++++++++-----------------------
 2 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 5d7e69cc1f..d8267129bc 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -65,9 +65,6 @@
 #define QERR_REPLAY_NOT_SUPPORTED \
     "Record/replay feature is not supported for '%s'"
 
-#define QERR_SET_PASSWD_FAILED \
-    "Could not set password"
-
 #define QERR_UNDEFINED_ERROR \
     "An undefined error has occurred"
 
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index a08143b323..ffbf948d55 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -199,13 +199,7 @@ void qmp_set_password(const char *protocol, const char *password,
         }
         rc = qemu_spice.set_passwd(password, fail_if_connected,
                                    disconnect_if_connected);
-        if (rc != 0) {
-            error_setg(errp, QERR_SET_PASSWD_FAILED);
-        }
-        return;
-    }
-
-    if (strcmp(protocol, "vnc") == 0) {
+    } else if (strcmp(protocol, "vnc") == 0) {
         if (fail_if_connected || disconnect_if_connected) {
             /* vnc supports "connected=keep" only */
             error_setg(errp, QERR_INVALID_PARAMETER, "connected");
@@ -214,13 +208,15 @@ void qmp_set_password(const char *protocol, const char *password,
         /* Note that setting an empty password will not disable login through
          * this interface. */
         rc = vnc_display_password(NULL, password);
-        if (rc < 0) {
-            error_setg(errp, QERR_SET_PASSWD_FAILED);
-        }
+    } else {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
+                   "'vnc' or 'spice'");
         return;
     }
 
-    error_setg(errp, QERR_INVALID_PARAMETER, "protocol");
+    if (rc != 0) {
+        error_setg(errp, "Could not set password");
+    }
 }
 
 void qmp_expire_password(const char *protocol, const char *whenstr,
@@ -244,28 +240,24 @@ void qmp_expire_password(const char *protocol, const char *whenstr,
             return;
         }
         rc = qemu_spice.set_pw_expire(when);
-        if (rc != 0) {
-            error_setg(errp, QERR_SET_PASSWD_FAILED);
-        }
-        return;
-    }
-
-    if (strcmp(protocol, "vnc") == 0) {
+    } else if (strcmp(protocol, "vnc") == 0) {
         rc = vnc_display_pw_expire(NULL, when);
-        if (rc != 0) {
-            error_setg(errp, QERR_SET_PASSWD_FAILED);
-        }
+    } else {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol",
+                   "'vnc' or 'spice'");
         return;
     }
 
-    error_setg(errp, QERR_INVALID_PARAMETER, "protocol");
+    if (rc != 0) {
+        error_setg(errp, "Could not set password expire time");
+    }
 }
 
 #ifdef CONFIG_VNC
 void qmp_change_vnc_password(const char *password, Error **errp)
 {
     if (vnc_display_password(NULL, password) < 0) {
-        error_setg(errp, QERR_SET_PASSWD_FAILED);
+        error_setg(errp, "Could not set password");
     }
 }
 
-- 
2.26.2

[PATCH 05/10] ui: Improve a client_migrate_info error message

[Markus Armbruster]

client_migrate_info reports spice_server_migrate_connect() failure as
"An undefined error has occurred".  Improve to "Could not set up
display for migration".

QERR_UNDEFINED_ERROR is now unused.  Drop.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qerror.h | 3 ---
 monitor/misc.c            | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index d8267129bc..596fce0c54 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -65,9 +65,6 @@
 #define QERR_REPLAY_NOT_SUPPORTED \
     "Record/replay feature is not supported for '%s'"
 
-#define QERR_UNDEFINED_ERROR \
-    "An undefined error has occurred"
-
 #define QERR_UNSUPPORTED \
     "this feature or command is not currently supported"
 
diff --git a/monitor/misc.c b/monitor/misc.c
index 9aa2505cfa..f0d3d41753 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -441,7 +441,7 @@ void qmp_client_migrate_info(const char *protocol, const char *hostname,
                                     has_port ? port : -1,
                                     has_tls_port ? tls_port : -1,
                                     cert_subject)) {
-            error_setg(errp, QERR_UNDEFINED_ERROR);
+            error_setg(errp, "Could not set up display for migration");
             return;
         }
         return;
-- 
2.26.2

[PATCH 06/10] ui: Tweak a client_migrate_info error message

[Markus Armbruster]

Change

    Parameter 'protocol' expects spice

to

    Parameter 'protocol' expects 'spice'

for consistency with similar error messages elsewhere.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor/misc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/misc.c b/monitor/misc.c
index f0d3d41753..354ac87736 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -447,7 +447,7 @@ void qmp_client_migrate_info(const char *protocol, const char *hostname,
         return;
     }
 
-    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol", "spice");
+    error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol", "'spice'");
 }
 
 static void hmp_logfile(Monitor *mon, const QDict *qdict)
-- 
2.26.2

[PATCH 07/10] qga: Replace an unreachable error by abort()

[Markus Armbruster]

check_suspend_mode()'s error message

    Parameter 'mode' expects GuestSuspendMode

makes no sense to users: GuestSuspendMode is a C enum.  Fortunately,
it is unreachable.  Replace it by abort().

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/commands-win32.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 300b87c859..87dc43e837 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1441,8 +1441,7 @@ static void check_suspend_mode(GuestSuspendMode mode, Error **errp)
         }
         break;
     default:
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode",
-                   "GuestSuspendMode");
+        abort();
     }
 }
 
-- 
2.26.2

[PATCH 08/10] qga: Tweak a guest-shutdown error message

[Markus Armbruster]

Change

    Parameter 'mode' expects halt|powerdown|reboot

to

    Parameter 'mode' expects 'halt', 'powerdown', or 'reboot'

for consistency with similar error messages elsewhere.

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/commands-win32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 87dc43e837..ba1fd07d06 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -334,7 +334,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
         shutdown_flag |= EWX_REBOOT;
     } else {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode",
-                   "halt|powerdown|reboot");
+                   "'halt', 'powerdown', or 'reboot'");
         return;
     }
 
-- 
2.26.2

[PATCH 09/10] qom: Improve {qom, device}-list-properties error messages

[Markus Armbruster]

qom-list-properties reports

    Parameter 'typename' expects device

when @typename exists, but isn't a TYPE_DEVICE.  Improve this to

    Parameter 'typename' expects a non-abstract device type

device-list-properties reports

    Parameter 'typename' expects object

when @typename exists, but isn't a TYPE_OBJECT.  Improve this to

    Parameter 'typename' expects a QOM type

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qom/qom-qmp-cmds.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 310ab2d048..2dd233f293 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -138,15 +138,10 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
         return NULL;
     }
 
-    klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
-    if (klass == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", TYPE_DEVICE);
-        return NULL;
-    }
-
-    if (object_class_is_abstract(klass)) {
+    if (!object_class_dynamic_cast(klass, TYPE_DEVICE)
+        || object_class_is_abstract(klass)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
-                   "non-abstract device type");
+                   "a non-abstract device type");
         return NULL;
     }
 
@@ -208,9 +203,9 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename,
         return NULL;
     }
 
-    klass = object_class_dynamic_cast(klass, TYPE_OBJECT);
-    if (klass == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", TYPE_OBJECT);
+    if (!object_class_dynamic_cast(klass, TYPE_OBJECT)) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
+                   "a QOM type");
         return NULL;
     }
 
-- 
2.26.2

Re: [PATCH 09/10] qom: Improve {qom,device}-list-properties error messages

[Paolo Bonzini]

On 13/11/20 09:26, Markus Armbruster wrote:
> qom-list-properties reports
> 
>      Parameter 'typename' expects device
> 
> when @typename exists, but isn't a TYPE_DEVICE.  Improve this to
> 
>      Parameter 'typename' expects a non-abstract device type
> 
> device-list-properties reports
> 
>      Parameter 'typename' expects object
> 
> when @typename exists, but isn't a TYPE_OBJECT.  Improve this to
> 
>      Parameter 'typename' expects a QOM type

Silly mistake: device-list-properties and qom-list-properties are 
exchanged in the commit message.  Can be fixed without reposting.

Paolo

Re: [PATCH 09/10] qom: Improve {qom,device}-list-properties error messages

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 13/11/20 09:26, Markus Armbruster wrote:
>> qom-list-properties reports
>>      Parameter 'typename' expects device
>> when @typename exists, but isn't a TYPE_DEVICE.  Improve this to
>>      Parameter 'typename' expects a non-abstract device type
>> device-list-properties reports
>>      Parameter 'typename' expects object
>> when @typename exists, but isn't a TYPE_OBJECT.  Improve this to
>>      Parameter 'typename' expects a QOM type
>
> Silly mistake: device-list-properties and qom-list-properties are
> exchanged in the commit message.  Can be fixed without reposting.

You're right.  Thanks!

[PATCH 10/10] Tweak a few "Parameter 'NAME' expects THING" error message

[Markus Armbruster]

Change to "expects a THING" where that's an obvious improvement

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/quorum.c                   | 2 +-
 blockdev.c                       | 2 +-
 chardev/char.c                   | 2 +-
 hw/core/qdev-properties-system.c | 2 +-
 softmmu/qdev-monitor.c           | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/quorum.c b/block/quorum.c
index e846a7e892..656a80f93a 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -856,7 +856,7 @@ static int quorum_valid_threshold(int threshold, int num_children, Error **errp)
 
     if (threshold < 1) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                   "vote-threshold", "value >= 1");
+                   "vote-threshold", "a value >= 1");
         return -ERANGE;
     }
 
diff --git a/blockdev.c b/blockdev.c
index d05a8740f4..6c7be7c522 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2991,7 +2991,7 @@ static void blockdev_mirror_common(const char *job_id, BlockDriverState *bs,
     }
     if (granularity & (granularity - 1)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "granularity",
-                   "power of 2");
+                   "a power of 2");
         return;
     }
 
diff --git a/chardev/char.c b/chardev/char.c
index aa4282164a..a9b8c5a9aa 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -521,7 +521,7 @@ static const ChardevClass *char_get_class(const char *driver, Error **errp)
 
     if (object_class_is_abstract(oc)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
-                   "abstract device type");
+                   "an abstract device type");
         return NULL;
     }
 
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index b81a4e8d14..93061b5bf1 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -776,7 +776,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, const char *name,
         }
         if (value < -1 || value > 255) {
             error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                       name ? name : "null", "pci_devfn");
+                       name ? name : "null", "a value between -1 and 255");
             return;
         }
         *ptr = value;
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index bcfb90a08f..08318c5d9d 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -237,7 +237,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
 
     if (object_class_is_abstract(oc)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
-                   "non-abstract device type");
+                   "a non-abstract device type");
         return NULL;
     }
 
@@ -245,7 +245,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp)
     if (!dc->user_creatable ||
         (qdev_hotplug && !dc->hotpluggable)) {
         error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver",
-                   "pluggable device type");
+                   "a pluggable device type");
         return NULL;
     }
 
-- 
2.26.2