[PATCH v1] hmp: Make json format optional for qom-set

David Hildenbrand posted 1 patch 3 years, 11 months ago
Test docker-mingw@fedora passed
Test checkpatch passed
Test asan passed
Test docker-quick@centos7 passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200608140650.91517-1-david@redhat.com
There is a newer version of this series
hmp-commands.hx    |  7 ++++---
qom/qom-hmp-cmds.c | 20 ++++++++++++++++----
2 files changed, 20 insertions(+), 7 deletions(-)
[PATCH v1] hmp: Make json format optional for qom-set
Posted by David Hildenbrand 3 years, 11 months ago
Commit 7d2ef6dcc1cf ("hmp: Simplify qom-set") switched to the json
parser, making it possible to specify complex types. However, with this
change it is no longer possible to specify proper sizes (e.g., 2G, 128M),
turning the interface harder to use for properties that consume sizes.

Let's switch back to the previous handling and allow to specify passing
json via the "-j" parameter.

Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---

An alternative would be to teach the json parser about sizes. But I am not
completely sure if that is desirable.

I make excessive use of qom-set with size properties in virtio-mem.

---
 hmp-commands.hx    |  7 ++++---
 qom/qom-hmp-cmds.c | 20 ++++++++++++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 28256209b5..b44f220c3e 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1806,9 +1806,10 @@ ERST
 
     {
         .name       = "qom-set",
-        .args_type  = "path:s,property:s,value:S",
-        .params     = "path property value",
-        .help       = "set QOM property",
+        .args_type  = "json:-j,path:s,property:s,value:s",
+        .params     = "[-j] path property value",
+        .help       = "set QOM property.\n\t\t\t"
+                      "-j: the property is specified in json format.",
         .cmd        = hmp_qom_set,
         .flags      = "p",
     },
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index f704b6949a..a794e62f0b 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -44,15 +44,27 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
 
 void hmp_qom_set(Monitor *mon, const QDict *qdict)
 {
+    const bool json = qdict_get_try_bool(qdict, "json", false);
     const char *path = qdict_get_str(qdict, "path");
     const char *property = qdict_get_str(qdict, "property");
     const char *value = qdict_get_str(qdict, "value");
     Error *err = NULL;
-    QObject *obj;
 
-    obj = qobject_from_json(value, &err);
-    if (err == NULL) {
-        qmp_qom_set(path, property, obj, &err);
+    if (!json) {
+        Object *obj = object_resolve_path(path, NULL);
+
+        if (!obj) {
+            error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND,
+                      "Device '%s' not found", path);
+        } else {
+            object_property_parse(obj, value, property, &err);
+        }
+    } else {
+        QObject *obj = qobject_from_json(value, &err);
+
+        if (!err) {
+            qmp_qom_set(path, property, obj, &err);
+        }
     }
 
     hmp_handle_error(mon, err);
-- 
2.26.2


Re: [PATCH v1] hmp: Make json format optional for qom-set
Posted by Paolo Bonzini 3 years, 11 months ago
On 08/06/20 16:06, David Hildenbrand wrote:
>      {
>          .name       = "qom-set",
> -        .args_type  = "path:s,property:s,value:S",
> -        .params     = "path property value",
> -        .help       = "set QOM property",
> +        .args_type  = "json:-j,path:s,property:s,value:s",

I think this should remain an uppercase "S", so that it grabs the rest
of the input rather than stop at the first space.

Paolo

> +        .params     = "[-j] path property value",
> +        .help       = "set QOM property.\n\t\t\t"
> +                      "-j: the property is specified in json format.",
>          .cmd        = hmp_qom_set,
>          .flags      = "p",
>      },
> diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c


Re: [PATCH v1] hmp: Make json format optional for qom-set
Posted by David Hildenbrand 3 years, 11 months ago

> Am 08.06.2020 um 16:16 schrieb Paolo Bonzini <pbonzini@redhat.com>:
> 
> On 08/06/20 16:06, David Hildenbrand wrote:
>>     {
>>         .name       = "qom-set",
>> -        .args_type  = "path:s,property:s,value:S",
>> -        .params     = "path property value",
>> -        .help       = "set QOM property",
>> +        .args_type  = "json:-j,path:s,property:s,value:s",
> 
> I think this should remain an uppercase "S", so that it grabs the rest
> of the input rather than stop at the first space.
> 

Yes, changed by mistake (while working on a revert). Thanks!

> Paolo
> 
>> +        .params     = "[-j] path property value",
>> +        .help       = "set QOM property.\n\t\t\t"
>> +                      "-j: the property is specified in json format.",
>>         .cmd        = hmp_qom_set,
>>         .flags      = "p",
>>     },
>> diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
>