diff mbox

qmp: set accurate parameter name for error msg of device-list-properties

Message ID 20170124105332.15992-1-lma@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lin Ma Jan. 24, 2017, 10:53 a.m. UTC
Signed-off-by: Lin Ma <lma@suse.com>
---
 qmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eric Blake Jan. 24, 2017, 4:57 p.m. UTC | #1
On 01/24/2017 04:53 AM, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@suse.com>
> ---

The commit message is rather sparse; is there an easy formula for
triggering the error message, to give more context about the change?

>  qmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qmp.c b/qmp.c
> index 0028f0b..812db6c 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -531,7 +531,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>  
>      klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
>      if (klass == NULL) {
> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name", TYPE_DEVICE);
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", TYPE_DEVICE);
>          return NULL;
>      }
>  
>
Markus Armbruster Jan. 24, 2017, 5:34 p.m. UTC | #2
Eric Blake <eblake@redhat.com> writes:

> On 01/24/2017 04:53 AM, Lin Ma wrote:
>> Signed-off-by: Lin Ma <lma@suse.com>
>> ---
>
> The commit message is rather sparse; is there an easy formula for
> triggering the error message, to give more context about the change?

Try this:
-> { "execute": "device-list-properties", "arguments": { "typename": "accel" } }
<- {"error": {"class": "GenericError", "desc": "Parameter 'name' expects device"}}

Type "accel" exists, but it's not a subtype of TYPE_DEVICE.

Perhaps:

    qmp: Fix argument name in error message of device-list-properties

    The argument is called "typename", not "name".

>>  qmp.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/qmp.c b/qmp.c
>> index 0028f0b..812db6c 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -531,7 +531,7 @@ DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
>>  
>>      klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
>>      if (klass == NULL) {
>> -        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name", TYPE_DEVICE);
>> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", TYPE_DEVICE);
>>          return NULL;
>>      }

Incomplete, the next one needs the same change:

        if (object_class_is_abstract(klass)) {
            error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name",
                       "non-abstract device type");
            return NULL;
        }

Reproducer:
-> { "execute": "device-list-properties", "arguments": { "typename": "pci-qxl" } }
<- {"error": {"class": "GenericError", "desc": "Parameter 'name' expects non-abstract device type"}}

Please fix this one, too.
diff mbox

Patch

diff --git a/qmp.c b/qmp.c
index 0028f0b..812db6c 100644
--- a/qmp.c
+++ b/qmp.c
@@ -531,7 +531,7 @@  DevicePropertyInfoList *qmp_device_list_properties(const char *typename,
 
     klass = object_class_dynamic_cast(klass, TYPE_DEVICE);
     if (klass == NULL) {
-        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "name", TYPE_DEVICE);
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", TYPE_DEVICE);
         return NULL;
     }