"Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> writes:
> From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > > Reimplement it based on qmp_qom_get() to avoid converting QObjects back > to strings. It's not using qmp_qom_get(), though. That's a sign our abstractions aren't quite right. HMP command handlers should be implemented as mostly trivial wrappers around the QMP handlers, or the two handlers should be mostly trivial wrappers around a common helper function. For a more detailed explanation, see below. Hmm, existing hmp_qom_set() has the same issue. I'm not demanding you clean this up. > Inspired-by: Paolo Bonzini <pbonz...@redhat.com> > Signed-off-by: Andreas Färber <afaer...@suse.de> > > Slight fix for bit-rot: > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > --- > hmp-commands.hx | 13 +++++++++++++ > hmp.c | 23 +++++++++++++++++++++++ > hmp.h | 1 + > 3 files changed, 37 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 848efee..73f0372 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1736,6 +1736,19 @@ Print QOM properties of object at location @var{path} > ETEXI > > { > + .name = "qom-get", > + .args_type = "path:s,property:s", > + .params = "path property", > + .help = "print QOM property", > + .mhandler.cmd = hmp_qom_get, > + }, > + > +STEXI > +@item qom-get @var{path} @var{property} > +Print QOM property @var{property} of object at location @var{path} This is a bit terse, but existing qom-list and qom-set are no less terse. The QAPI schema has more detail, perhaps we can adapt it for the manual. Not a blocker for me. > +ETEXI > + > + { > .name = "qom-set", > .args_type = "path:s,property:s,value:s", > .params = "path property value", > diff --git a/hmp.c b/hmp.c > index cc2056e..17e6b06 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -2064,6 +2064,29 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict) > hmp_handle_error(mon, &err); > } > > +void hmp_qom_get(Monitor *mon, const QDict *qdict) > +{ > + const char *path = qdict_get_str(qdict, "path"); > + const char *property = qdict_get_str(qdict, "property"); > + Error *err = NULL; > + Object *obj; > + char *value; > + > + obj = object_resolve_path(path, NULL); > + if (obj == NULL) { > + error_set(&err, ERROR_CLASS_DEVICE_NOT_FOUND, > + "Device '%s' not found", path); > + hmp_handle_error(mon, &err); > + return; > + } > + value = object_property_print(obj, property, true, &err); > + if (err == NULL) { > + monitor_printf(mon, "%s\n", value); > + g_free(value); > + } > + hmp_handle_error(mon, &err); > +} > + Compare: QObject *qmp_qom_get(const char *path, const char *property, Error **errp) { Object *obj; obj = object_resolve_path(path, NULL); if (!obj) { error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND, "Device '%s' not found", path); return NULL; } return object_property_get_qobject(obj, property, errp); } The missing piece seems to be a function that prints the result of object_property_get_qobject(). Since converting to QObject just for printing is stupid, a common helper function seems more appropriate. qmp_qom_get() would pass it the QMP output visitor to get a QObject, and hmp_qom_get() would pass it the string output visitor to get a string. > void hmp_qom_set(Monitor *mon, const QDict *qdict) > { > const char *path = qdict_get_str(qdict, "path"); > diff --git a/hmp.h b/hmp.h > index 0876ec0..882f339 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -103,6 +103,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict); > void hmp_info_memdev(Monitor *mon, const QDict *qdict); > void hmp_info_memory_devices(Monitor *mon, const QDict *qdict); > void hmp_qom_list(Monitor *mon, const QDict *qdict); > +void hmp_qom_get(Monitor *mon, const QDict *qdict); > void hmp_qom_set(Monitor *mon, const QDict *qdict); > void object_add_completion(ReadLineState *rs, int nb_args, const char *str); > void object_del_completion(ReadLineState *rs, int nb_args, const char *str);