[Qemu-devel] [PATCH qom-next 0/4] qom: HMP commands to replace info qtree

[Qemu-devel] [PATCH qom-next 0/4] qom: HMP commands to replace info qtree

[Andreas Färber]

Hello,

The main patch of this series is an HMP command "info qom-composition",
which displays the machine composition tree. This names all devices,
including those missing in "info qtree" for lack of bus.

To make it more like "info qtree" bus-wise, we could extend it to display
link<> properties as well.

Properties of devices can be listed with "qom-list", like in QMP.

Based on a suggestion from Paolo, I went on to implement "qom-get" and
"qom-set" for reading and setting them, not basing on their QMP counterparts
but reimplementing them to circumvent the complicated QObject -> string
conversion Hani tried [1].

As I replied there, I think we can have both views - with and without
properties. My question is, should they be differently named commands?
Or an argument to the command? Should it go into qdev-monitor.c alongside
the old qdev HMP commands or into hmp.c? Is it okay to not base HMP commands
on their QMP counterparts, or is there any other visitor-based solution?

Built on top of my pending qom-tree script [2] but shouldn't depend on it.
I still consider that useful, but not resending it since it didn't change.

Regards,
Andreas

[1] http://patchwork.ozlabs.org/patch/343136/
[2] http://patchwork.ozlabs.org/patch/317224/

$ ./x86_64-softmmu/qemu-system-x86_64 -S -display none -monitor stdio
QEMU 2.0.50 monitor - type 'help' for more information
(qemu) info qom-composition
/machine (pc-i440fx-2.1-machine)
  /peripheral-anon (container)
  /peripheral (container)
  /unattached (container)
    /sysbus (System)
    /device[0] (qemu64-x86_64-cpu)
      /apic (apic)
    /device[1] (kvmvapic)
    /device[2] (i440FX)
    /device[3] (PIIX3)
      /isa.0 (ISA)
    /device[4] (isa-i8259)
    /device[5] (isa-i8259)
    /device[6] (cirrus-vga)
    /device[7] (hpet)
    /device[8] (mc146818rtc)
    /device[9] (isa-pit)
    /device[10] (isa-pcspk)
    /device[11] (isa-serial)
    /device[12] (isa-parallel)
    /device[13] (i8042)
    /device[14] (vmport)
    /device[15] (vmmouse)
    /device[16] (port92)
    /device[17] (isa-fdc)
    /device[18] (e1000)
    /device[19] (piix3-ide)
      /ide.0 (IDE)
      /ide.1 (IDE)
    /device[20] (ide-cd)
    /device[21] (PIIX4_PM)
      /i2c (i2c-bus)
    /device[22] (smbus-eeprom)
    /device[23] (smbus-eeprom)
    /device[24] (smbus-eeprom)
    /device[25] (smbus-eeprom)
    /device[26] (smbus-eeprom)
    /device[27] (smbus-eeprom)
    /device[28] (smbus-eeprom)
    /device[29] (smbus-eeprom)
  /icc-bridge (icc-bridge)
    /icc (icc-bus)
  /fw_cfg (fw_cfg)
  /i440fx (i440FX-pcihost)
    /pci.0 (PCI)
    /ioapic (ioapic)
(qemu) qom-list
/
(qemu) qom-list /
backend (child<container>)
machine (child<pc-i440fx-2.1-machine>)
type (string)
(qemu) qom-list /machine
i440fx (child<i440FX-pcihost>)
fw_cfg (child<fw_cfg>)
icc-bridge (child<icc-bridge>)
unattached (child<container>)
peripheral (child<container>)
peripheral-anon (child<container>)
type (string)
(qemu) qom-get /machine type
"pc-i440fx-2.1-machine"
(qemu) qom-get /machine/unassigned/device[0] realized
Device '/machine/unassigned/device[0]' not found
(qemu) qom-get /machine/unattached/device[0] realized
true
(qemu) qom-set /machine/unattached/device[0] realized true
(qemu) qom-set /machine/unattached/device[0] realized false
(qemu) 

Cc: Hani Benhabiles <hani@linux.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Andreas Färber (4):
  qom: Implement info qom-composition HMP command
  qom: Implement qom-list HMP command
  qom: Implement qom-get HMP command
  qom: Implement qom-set HMP command

 hmp-commands.hx        | 39 +++++++++++++++++++++++++++++
 hmp.c                  | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
 hmp.h                  |  3 +++
 include/monitor/qdev.h |  1 +
 monitor.c              |  7 ++++++
 qdev-monitor.c         | 35 ++++++++++++++++++++++++++
 6 files changed, 152 insertions(+)

-- 
1.8.4.5

[Qemu-devel] [PATCH qom-next 1/4] qom: Implement info qom-composition HMP command

[Andreas Färber]

To complement qdev's bus-oriented info qtree, info qom-composition
prints a hierarchical view of the machine composition tree.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 include/monitor/qdev.h |  1 +
 monitor.c              |  7 +++++++
 qdev-monitor.c         | 35 +++++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
index 8d16e11..cb5c8ee 100644
--- a/include/monitor/qdev.h
+++ b/include/monitor/qdev.h
@@ -8,6 +8,7 @@
 
 void do_info_qtree(Monitor *mon, const QDict *qdict);
 void do_info_qdm(Monitor *mon, const QDict *qdict);
+void do_info_qom_composition(Monitor *mon, const QDict *dict);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int qdev_device_help(QemuOpts *opts);
 DeviceState *qdev_device_add(QemuOpts *opts);
diff --git a/monitor.c b/monitor.c
index 1266ba0..c2b9315 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2920,6 +2920,13 @@ static mon_cmd_t info_cmds[] = {
         .mhandler.cmd = do_info_qdm,
     },
     {
+        .name       = "qom-composition",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show QOM machine composition tree",
+        .mhandler.cmd = do_info_qom_composition,
+    },
+    {
         .name       = "roms",
         .args_type  = "",
         .params     = "",
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 02cbe43..744144a 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -658,6 +658,41 @@ void do_info_qdm(Monitor *mon, const QDict *qdict)
     qdev_print_devinfos(true);
 }
 
+typedef struct QOMCompositionState {
+    Monitor *mon;
+    int indent;
+} QOMCompositionState;
+
+static void print_qom_composition(Monitor *mon, Object *obj, int indent);
+
+static int print_qom_composition_child(Object *obj, void *opaque)
+{
+    QOMCompositionState *s = opaque;
+
+    print_qom_composition(s->mon, obj, s->indent);
+
+    return 0;
+}
+
+static void print_qom_composition(Monitor *mon, Object *obj, int indent)
+{
+    QOMCompositionState s = {
+        .mon = mon,
+        .indent = indent + 2,
+    };
+    char *name = object_get_canonical_path_component(obj);
+
+    monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name,
+                   object_get_typename(obj));
+    g_free(name);
+    object_child_foreach(obj, print_qom_composition_child, &s);
+}
+
+void do_info_qom_composition(Monitor *mon, const QDict *dict)
+{
+    print_qom_composition(mon, qdev_get_machine(), 0);
+}
+
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     Error *local_err = NULL;
-- 
1.8.4.5

[Qemu-devel] [PATCH qom-next 2/4] qom: Implement qom-list HMP command

[Andreas Färber]

Implement it as a wrapper for QMP qom-list, but mimic the behavior of
scripts/qmp/qom-list in making the path argument optional and listing
the root if absent, to hint users what kind of path to pass.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hmp-commands.hx | 13 +++++++++++++
 hmp.c           | 27 +++++++++++++++++++++++++++
 hmp.h           |  1 +
 3 files changed, 41 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 8971f1b..a0166af 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1678,6 +1678,19 @@ Add CPU with id @var{id}
 ETEXI
 
     {
+        .name       = "qom-list",
+        .args_type  = "path:s?",
+        .params     = "path",
+        .help       = "list QOM properties",
+        .mhandler.cmd  = hmp_qom_list,
+    },
+
+STEXI
+@item qom-list [@var{path}]
+Print QOM properties of object at location @var{path}
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index ca869ba..1c29c8a 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1666,3 +1666,30 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
     qmp_object_del(id, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_qom_list(Monitor *mon, const QDict *qdict)
+{
+    const char *path = qdict_get_try_str(qdict, "path");
+    ObjectPropertyInfoList *list;
+    Error *err = NULL;
+
+    if (path == NULL) {
+        monitor_printf(mon, "/\n");
+        return;
+    }
+    list = qmp_qom_list(path, &err);
+    if (err == NULL) {
+        while (list != NULL) {
+            ObjectPropertyInfoList *next = list->next;
+
+            monitor_printf(mon, "%s (%s)\n",
+                           list->value->name, list->value->type);
+            g_free(list->value->name);
+            g_free(list->value->type);
+            g_free(list->value);
+            g_free(list);
+            list = next;
+        }
+    }
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 20ef454..7ab969e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -93,6 +93,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict);
 void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
+void hmp_qom_list(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);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-- 
1.8.4.5

[Qemu-devel] [PATCH qom-next 3/4] qom: Implement qom-get HMP command

[Andreas Färber]

Reimplement it based on qmp_qom_get() to avoid converting QObjects back
to strings.

Inspired-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hmp-commands.hx | 13 +++++++++++++
 hmp.c           | 22 ++++++++++++++++++++++
 hmp.h           |  1 +
 3 files changed, 36 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index a0166af..c0603e9 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1691,6 +1691,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}
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 1c29c8a..d7d7a98 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1693,3 +1693,25 @@ 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, QERR_DEVICE_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);
+}
diff --git a/hmp.h b/hmp.h
index 7ab969e..269d99e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
 void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(Monitor *mon, const QDict *qdict);
 void hmp_qom_list(Monitor *mon, const QDict *qdict);
+void hmp_qom_get(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);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-- 
1.8.4.5

[Qemu-devel] [PATCH qom-next 4/4] qom: Implement qom-set HMP command

[Andreas Färber]

Re-implemented based on qmp_qom_set() to facilitate argument parsing.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 hmp-commands.hx | 13 +++++++++++++
 hmp.c           | 18 ++++++++++++++++++
 hmp.h           |  1 +
 3 files changed, 32 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index c0603e9..73145b7 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1704,6 +1704,19 @@ Print QOM property @var{property} of object at location @var{path}
 ETEXI
 
     {
+        .name       = "qom-set",
+        .args_type  = "path:s,property:s,value:s",
+        .params     = "path property value",
+        .help       = "set QOM property",
+        .mhandler.cmd  = hmp_qom_set,
+    },
+
+STEXI
+@item qom-set @var{path} @var{property} @var{value}
+Set QOM property @var{property} of object at location @var{path} to value @var{value}
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index d7d7a98..1cc2e60 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1715,3 +1715,21 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict)
     }
     hmp_handle_error(mon, &err);
 }
+
+void hmp_qom_set(Monitor *mon, const QDict *qdict)
+{
+    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;
+    Object *obj;
+
+    obj = object_resolve_path(path, NULL);
+    if (obj == NULL) {
+        error_set(&err, QERR_DEVICE_NOT_FOUND, path);
+        hmp_handle_error(mon, &err);
+        return;
+    }
+    object_property_parse(obj, value, property, &err);
+    hmp_handle_error(mon, &err);
+}
diff --git a/hmp.h b/hmp.h
index 269d99e..6b7b1da 100644
--- a/hmp.h
+++ b/hmp.h
@@ -95,6 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
 void hmp_object_del(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);
 void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
-- 
1.8.4.5

Re: [Qemu-devel] [PATCH qom-next 1/4] qom: Implement info qom-composition HMP command

[Hani Benhabiles]

On Thu, May 08, 2014 at 04:21:14PM +0200, Andreas Färber wrote:
> To complement qdev's bus-oriented info qtree, info qom-composition
> prints a hierarchical view of the machine composition tree.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  include/monitor/qdev.h |  1 +
>  monitor.c              |  7 +++++++
>  qdev-monitor.c         | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 43 insertions(+)
> 
> diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
> index 8d16e11..cb5c8ee 100644
> --- a/include/monitor/qdev.h
> +++ b/include/monitor/qdev.h
> @@ -8,6 +8,7 @@
>  
>  void do_info_qtree(Monitor *mon, const QDict *qdict);
>  void do_info_qdm(Monitor *mon, const QDict *qdict);
> +void do_info_qom_composition(Monitor *mon, const QDict *dict);
>  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
>  int qdev_device_help(QemuOpts *opts);
>  DeviceState *qdev_device_add(QemuOpts *opts);
> diff --git a/monitor.c b/monitor.c
> index 1266ba0..c2b9315 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2920,6 +2920,13 @@ static mon_cmd_t info_cmds[] = {
>          .mhandler.cmd = do_info_qdm,
>      },
>      {
> +        .name       = "qom-composition",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show QOM machine composition tree",
> +        .mhandler.cmd = do_info_qom_composition,
> +    },
> +    {
>          .name       = "roms",
>          .args_type  = "",
>          .params     = "",
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index 02cbe43..744144a 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -658,6 +658,41 @@ void do_info_qdm(Monitor *mon, const QDict *qdict)
>      qdev_print_devinfos(true);
>  }
>  
> +typedef struct QOMCompositionState {
> +    Monitor *mon;
> +    int indent;
> +} QOMCompositionState;
> +
> +static void print_qom_composition(Monitor *mon, Object *obj, int indent);
> +
> +static int print_qom_composition_child(Object *obj, void *opaque)
> +{
> +    QOMCompositionState *s = opaque;
> +
> +    print_qom_composition(s->mon, obj, s->indent);
> +
> +    return 0;
> +}
> +
> +static void print_qom_composition(Monitor *mon, Object *obj, int indent)
> +{
> +    QOMCompositionState s = {
> +        .mon = mon,
> +        .indent = indent + 2,
> +    };
> +    char *name = object_get_canonical_path_component(obj);
> +
> +    monitor_printf(mon, "%*s/%s (%s)\n", indent, "", name,
> +                   object_get_typename(obj));
> +    g_free(name);
> +    object_child_foreach(obj, print_qom_composition_child, &s);
> +}
> +
> +void do_info_qom_composition(Monitor *mon, const QDict *dict)
> +{
> +    print_qom_composition(mon, qdev_get_machine(), 0);
> +}
> +
>  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
>  {
>      Error *local_err = NULL;

Reviewed-by: Hani Benhabiles <hani@linux.com>

Re: [Qemu-devel] [PATCH qom-next 2/4] qom: Implement qom-list HMP command

[Hani Benhabiles]

On Thu, May 08, 2014 at 04:21:15PM +0200, Andreas Färber wrote:
> Implement it as a wrapper for QMP qom-list, but mimic the behavior of
> scripts/qmp/qom-list in making the path argument optional and listing
> the root if absent, to hint users what kind of path to pass.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hmp-commands.hx | 13 +++++++++++++
>  hmp.c           | 27 +++++++++++++++++++++++++++
>  hmp.h           |  1 +
>  3 files changed, 41 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 8971f1b..a0166af 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1678,6 +1678,19 @@ Add CPU with id @var{id}
>  ETEXI
>  
>      {
> +        .name       = "qom-list",
> +        .args_type  = "path:s?",
> +        .params     = "path",
> +        .help       = "list QOM properties",
> +        .mhandler.cmd  = hmp_qom_list,
> +    },
> +
> +STEXI
> +@item qom-list [@var{path}]
> +Print QOM properties of object at location @var{path}
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index ca869ba..1c29c8a 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1666,3 +1666,30 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
>      qmp_object_del(id, &err);
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_qom_list(Monitor *mon, const QDict *qdict)
> +{
> +    const char *path = qdict_get_try_str(qdict, "path");
> +    ObjectPropertyInfoList *list;
> +    Error *err = NULL;
> +
> +    if (path == NULL) {
> +        monitor_printf(mon, "/\n");
> +        return;
> +    }
> +    list = qmp_qom_list(path, &err);
> +    if (err == NULL) {

This:

> +        while (list != NULL) {
> +            ObjectPropertyInfoList *next = list->next;
> +
> +            monitor_printf(mon, "%s (%s)\n",
> +                           list->value->name, list->value->type);
> +            g_free(list->value->name);
> +            g_free(list->value->type);
> +            g_free(list->value);
> +            g_free(list);
> +            list = next;
> +        }

could be simplified to:
        ObjectPropertyInfoList *start = list;
        while (list) {
            ObjectPropertyInfo *value = list->value;
            monitor_printf(mon, "%s (%s)\n", value->name, value->type);
            list = list->next;
        }
        qapi_free_ObjectPropertyInfoList(start);

Other than that, the patch seems fine to me.

> +    }
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 20ef454..7ab969e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -93,6 +93,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict);
>  void hmp_cpu_add(Monitor *mon, const QDict *qdict);
>  void hmp_object_add(Monitor *mon, const QDict *qdict);
>  void hmp_object_del(Monitor *mon, const QDict *qdict);
> +void hmp_qom_list(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);
>  void device_add_completion(ReadLineState *rs, int nb_args, const char *str);

Re: [Qemu-devel] [PATCH qom-next 3/4] qom: Implement qom-get HMP command

[Hani Benhabiles]

On Thu, May 08, 2014 at 04:21:16PM +0200, Andreas Färber wrote:
> Reimplement it based on qmp_qom_get() to avoid converting QObjects back
> to strings.
> 
> Inspired-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hmp-commands.hx | 13 +++++++++++++
>  hmp.c           | 22 ++++++++++++++++++++++
>  hmp.h           |  1 +
>  3 files changed, 36 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a0166af..c0603e9 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1691,6 +1691,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}
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index 1c29c8a..d7d7a98 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1693,3 +1693,25 @@ 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, QERR_DEVICE_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);
> +}
> diff --git a/hmp.h b/hmp.h
> index 7ab969e..269d99e 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -94,6 +94,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict);
>  void hmp_object_add(Monitor *mon, const QDict *qdict);
>  void hmp_object_del(Monitor *mon, const QDict *qdict);
>  void hmp_qom_list(Monitor *mon, const QDict *qdict);
> +void hmp_qom_get(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);
>  void device_add_completion(ReadLineState *rs, int nb_args, const char *str);

This segfaults:
(qemu) qom-get /machine/unattached/device[9] date
Segmentation fault (core dumped)

And so does
(qemu) qom-get /machine/unattached/device[0] filtered-features
Segmentation fault (core dumped)

See: https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01975.html

Re: [Qemu-devel] [PATCH qom-next 4/4] qom: Implement qom-set HMP command

[Hani Benhabiles]

On Thu, May 08, 2014 at 04:21:17PM +0200, Andreas Färber wrote:
> Re-implemented based on qmp_qom_set() to facilitate argument parsing.
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  hmp-commands.hx | 13 +++++++++++++
>  hmp.c           | 18 ++++++++++++++++++
>  hmp.h           |  1 +
>  3 files changed, 32 insertions(+)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index c0603e9..73145b7 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1704,6 +1704,19 @@ Print QOM property @var{property} of object at location @var{path}
>  ETEXI
>  
>      {
> +        .name       = "qom-set",
> +        .args_type  = "path:s,property:s,value:s",
> +        .params     = "path property value",
> +        .help       = "set QOM property",
> +        .mhandler.cmd  = hmp_qom_set,
> +    },
> +
> +STEXI
> +@item qom-set @var{path} @var{property} @var{value}
> +Set QOM property @var{property} of object at location @var{path} to value @var{value}
> +ETEXI
> +
> +    {
>          .name       = "info",
>          .args_type  = "item:s?",
>          .params     = "[subcommand]",
> diff --git a/hmp.c b/hmp.c
> index d7d7a98..1cc2e60 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1715,3 +1715,21 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict)
>      }
>      hmp_handle_error(mon, &err);
>  }
> +
> +void hmp_qom_set(Monitor *mon, const QDict *qdict)
> +{
> +    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;
> +    Object *obj;
> +
> +    obj = object_resolve_path(path, NULL);

Is there any consensus on whether to check for path ambiguity ?

It seems to me that it would be more important to do so here than in
qmp_qom_list() for example.

Other than that, patch looks fine for me.

> +    if (obj == NULL) {
> +        error_set(&err, QERR_DEVICE_NOT_FOUND, path);
> +        hmp_handle_error(mon, &err);
> +        return;
> +    }
> +    object_property_parse(obj, value, property, &err);
> +    hmp_handle_error(mon, &err);
> +}
> diff --git a/hmp.h b/hmp.h
> index 269d99e..6b7b1da 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -95,6 +95,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict);
>  void hmp_object_del(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);
>  void device_add_completion(ReadLineState *rs, int nb_args, const char *str);
> -- 
> 1.8.4.5
> 
> 

Re: [Qemu-devel] [PATCH qom-next 0/4] qom: HMP commands to replace info qtree

[Peter Crosthwaite]

On Fri, May 9, 2014 at 12:21 AM, Andreas Färber <afaerber@suse.de> wrote:
> Hello,
>
> The main patch of this series is an HMP command "info qom-composition",

qom-composition is a mouthful. Can it be shortened? Although I suggest
alternative that obsoletes it anyways further down.

> which displays the machine composition tree. This names all devices,
> including those missing in "info qtree" for lack of bus.
>
> To make it more like "info qtree" bus-wise, we could extend it to display
> link<> properties as well.
>
> Properties of devices can be listed with "qom-list", like in QMP.
>

So this has some ease-of-use challenges. "info qom-comp" doesnt give
you copy-pastable full canonical paths, yet you need to have a full
path to get the options of a particular device.

Some possibilities to make this more usable might include:

1: Full canonical paths in your output rather than indentation.
Probably costs readability.
2: More flexibility in the information provided by qom-comp.
3: Tab autocomplete of args to functions like qom-list (probably rather hard)

> Based on a suggestion from Paolo, I went on to implement "qom-get" and
> "qom-set" for reading and setting them, not basing on their QMP counterparts
> but reimplementing them to circumvent the complicated QObject -> string
> conversion Hani tried [1].
>
> As I replied there, I think we can have both views - with and without
> properties. My question is, should they be differently named commands?
> Or an argument to the command?

Elaborating idea #2 a bit, I think qom-composition as it stands, is a
little bit "all or nothing". Should it perhaps accept an argument
(much like qom-list) and then it shows the composition tree starting
from that node?

Once you add a with-properties mode the output for each node, the
output is going to be similar to qom-list for each subnode. To that
end can all the problems be solved with only a single qom-list command
where:

1: We add a recursive mode to qom-list where instead of showing a
child link prop we show qom-list output of that child (indentation++).
2: We allow filtering property types for qom-list output (all, none,
only links etc).

This allows you to do tree views or either entire machine. or a
subsystem (or just a single device), with or without property
information solving all display-of-information desires. Depending on
how sophisticated idea #2 is implemented you could also do you
qom-composition-with-just-links output.

> Should it go into qdev-monitor.c alongside
> the old qdev HMP commands or into hmp.c? Is it okay to not base HMP commands
> on their QMP counterparts, or is there any other visitor-based solution?
>
> Built on top of my pending qom-tree script [2] but shouldn't depend on it.
> I still consider that useful, but not resending it since it didn't change.
>
> Regards,
> Andreas
>
> [1] http://patchwork.ozlabs.org/patch/343136/
> [2] http://patchwork.ozlabs.org/patch/317224/
>
> $ ./x86_64-softmmu/qemu-system-x86_64 -S -display none -monitor stdio
> QEMU 2.0.50 monitor - type 'help' for more information
> (qemu) info qom-composition
> /machine (pc-i440fx-2.1-machine)
>   /peripheral-anon (container)
>   /peripheral (container)
>   /unattached (container)
>     /sysbus (System)
>     /device[0] (qemu64-x86_64-cpu)
>       /apic (apic)
>     /device[1] (kvmvapic)
>     /device[2] (i440FX)
>     /device[3] (PIIX3)
>       /isa.0 (ISA)
>     /device[4] (isa-i8259)
>     /device[5] (isa-i8259)
>     /device[6] (cirrus-vga)
>     /device[7] (hpet)
>     /device[8] (mc146818rtc)
>     /device[9] (isa-pit)
>     /device[10] (isa-pcspk)
>     /device[11] (isa-serial)
>     /device[12] (isa-parallel)
>     /device[13] (i8042)
>     /device[14] (vmport)
>     /device[15] (vmmouse)
>     /device[16] (port92)
>     /device[17] (isa-fdc)
>     /device[18] (e1000)
>     /device[19] (piix3-ide)
>       /ide.0 (IDE)
>       /ide.1 (IDE)
>     /device[20] (ide-cd)
>     /device[21] (PIIX4_PM)
>       /i2c (i2c-bus)

Quite a tangential problem (and way out of scope of this series), but
should I2C devices be parented to their bus? I know TYPE_BUS is evil,
but TYPE_I2C_BUS probably need to stay even after full de-BUSification
(probably just reparented to TYPE_DEVICE). Then all the actual I2C
devices are children of that i2c-bus DEVICE. I2C bus itself implements
non-trivial functionality (much unlike sysbus).

>     /device[22] (smbus-eeprom)
>     /device[23] (smbus-eeprom)
>     /device[24] (smbus-eeprom)
>     /device[25] (smbus-eeprom)
>     /device[26] (smbus-eeprom)
>     /device[27] (smbus-eeprom)
>     /device[28] (smbus-eeprom)
>     /device[29] (smbus-eeprom)
>   /icc-bridge (icc-bridge)
>     /icc (icc-bus)
>   /fw_cfg (fw_cfg)
>   /i440fx (i440FX-pcihost)
>     /pci.0 (PCI)
>     /ioapic (ioapic)
> (qemu) qom-list
> /
> (qemu) qom-list /
> backend (child<container>)
> machine (child<pc-i440fx-2.1-machine>)
> type (string)
> (qemu) qom-list /machine
> i440fx (child<i440FX-pcihost>)
> fw_cfg (child<fw_cfg>)
> icc-bridge (child<icc-bridge>)
> unattached (child<container>)
> peripheral (child<container>)
> peripheral-anon (child<container>)
> type (string)
> (qemu) qom-get /machine type
> "pc-i440fx-2.1-machine"
> (qemu) qom-get /machine/unassigned/device[0] realized
> Device '/machine/unassigned/device[0]' not found
> (qemu) qom-get /machine/unattached/device[0] realized
> true
> (qemu) qom-set /machine/unattached/device[0] realized true
> (qemu) qom-set /machine/unattached/device[0] realized false
> (qemu)
>
> Cc: Hani Benhabiles <hani@linux.com>
> Cc: Luiz Capitulino <lcapitulino@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Anthony Liguori <anthony@codemonkey.ws>
> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>
> Andreas Färber (4):
>   qom: Implement info qom-composition HMP command
>   qom: Implement qom-list HMP command

>   qom: Implement qom-get HMP command
>   qom: Implement qom-set HMP command

These two seem a little out of scope. They don't really add anything
to the "lets get rid of qtree" problem. They perhaps just stand in
their own right.

Regards,
Peter

>
>  hmp-commands.hx        | 39 +++++++++++++++++++++++++++++
>  hmp.c                  | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  hmp.h                  |  3 +++
>  include/monitor/qdev.h |  1 +
>  monitor.c              |  7 ++++++
>  qdev-monitor.c         | 35 ++++++++++++++++++++++++++
>  6 files changed, 152 insertions(+)
>
> --
> 1.8.4.5
>
>