Markus Armbruster wrote:
> Jan Kiszka <jan.kis...@siemens.com> writes:
>
>> This introduces device_show, a monitor command that saves the vmstate of
>> a qdev device and visualizes it. QMP is also supported. Buffers are cut
>> after 16 byte by default, but the full content can be requested via
>> '-f'. To pretty-print sub-arrays, vmstate is extended to store the start
>> index name.
>>
>> Signed-off-by: Jan Kiszka <jan.kis...@siemens.com>
>
> Neato! Comments inline.
>
>> ---
>> hw/hw.h | 2 +
>> hw/qdev.c | 287
>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> hw/qdev.h | 2 +
>> qemu-monitor.hx | 16 +++
>> 4 files changed, 307 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/hw.h b/hw/hw.h
>> index 328b704..1ff8e40 100644
>> --- a/hw/hw.h
>> +++ b/hw/hw.h
>> @@ -299,6 +299,7 @@ enum VMStateFlags {
>>
>> typedef struct {
>> const char *name;
>> + const char *start_index;
>> size_t offset;
>> size_t size;
>> size_t start;
>
> Why is start_index a string?
It can, actually should be a symbolic constant (from e1000:
"mac_reg[RA+03]: ...", ie. "RA" is the start index here).
>
>> @@ -414,6 +415,7 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>> .size = sizeof(_type), \
>> .flags = VMS_ARRAY, \
>> .offset = vmstate_offset_sub_array(_state, _field, _type, _start), \
>> + .start_index = (stringify(_start)), \
>> }
>>
>> #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info,
>> _type) {\
>> diff --git a/hw/qdev.c b/hw/qdev.c
>> index fe49e71..c989010 100644
>> --- a/hw/qdev.c
>> +++ b/hw/qdev.c
>> @@ -29,6 +29,9 @@
>> #include "qdev.h"
>> #include "sysemu.h"
>> #include "monitor.h"
>> +#include "qjson.h"
>> +#include "qint.h"
>> +#include "qbuffer.h"
>>
>> static int qdev_hotplug = 0;
>>
>> @@ -824,3 +827,287 @@ int do_device_del(Monitor *mon, const QDict *qdict,
>> QObject **ret_data)
>> }
>> return qdev_unplug(dev);
>> }
>> +
>> +static int print_array_index(Monitor *mon, const char *start, int index)
>> +{
>> + char index_str[32];
>> + int len;
>> +
>> + if (start) {
>> + len = snprintf(index_str, sizeof(index_str), "[%s+%02x]", start,
>> + index);
>> + } else {
>> + len = snprintf(index_str, sizeof(index_str), "[%02x]", index);
>> + }
>> + monitor_printf(mon, index_str);
>> + return len;
>
> You go via the snprintf() detour just because monitor_printf() doesn't
> return the number of characters printed. Wouldn't it be better to fix
> that?
Yeah, good point. Will address this.
>
>> +}
>> +
>> +#define NAME_COLUMN_WIDTH 23
>> +
>> +static void print_field(Monitor *mon, const QDict *qfield, int indent);
>> +
>> +static void print_elem(Monitor *mon, const QObject *qelem, size_t size,
>> + int column_pos, int indent)
>> +{
>> + int64_t data_size;
>> + const void *data;
>> + int n;
>> +
>> + if (qobject_type(qelem) == QTYPE_QDICT) {
>> + if (column_pos >= 0) {
>> + monitor_printf(mon, ".\n");
>> + }
>> + } else {
>> + monitor_printf(mon, ":");
>> + column_pos++;
>> + if (column_pos < NAME_COLUMN_WIDTH) {
>> + monitor_printf(mon, "%*c", NAME_COLUMN_WIDTH - column_pos, ' ');
>> + }
>> + }
>> +
>> + switch (qobject_type(qelem)) {
>> + case QTYPE_QDICT:
>> + print_field(mon, qobject_to_qdict(qelem), indent + 2);
>> + break;
>> + case QTYPE_QBUFFER:
>> + data = qbuffer_get_data(qobject_to_qbuffer(qelem));
>> + data_size = qbuffer_get_size(qobject_to_qbuffer(qelem));
>> + for (n = 0; n < data_size; ) {
>> + monitor_printf(mon, " %02x", *((uint8_t *)data+n));
>> + if (++n < size) {
>> + if (n % 16 == 0) {
>> + monitor_printf(mon, "\n%*c", NAME_COLUMN_WIDTH, ' ');
>> + } else if (n % 8 == 0) {
>> + monitor_printf(mon, " -");
>> + }
>> + }
>> + }
>> + if (data_size < size) {
>> + monitor_printf(mon, " ...");
>> + }
>> + monitor_printf(mon, "\n");
>> + break;
>> + case QTYPE_QINT:
>> + monitor_printf(mon, " %0*x\n", (int)size * 2,
>> + (int)qint_get_int(qobject_to_qint(qelem)));
>
> I'm confused. Doesn't casting qint_get_int() to int lose bits?
You are right, will fix!
>
>> + break;
>> + default:
>> + assert(0);
>> + }
>> +}
>> +
>> +static void print_field(Monitor *mon, const QDict *qfield, int indent)
>> +{
>> + const char *name = qdict_get_str(qfield, "name");
>> + const char *start = qdict_get_try_str(qfield, "start");
>> + int64_t size = qdict_get_int(qfield, "size");
>> + QList *qlist = qdict_get_qlist(qfield, "elems");
>> + QListEntry *entry, *sub_entry;
>> + QList *sub_list;
>> + int elem_no = 0;
>> +
>> + QLIST_FOREACH_ENTRY(qlist, entry) {
>> + QObject *qelem = qlist_entry_obj(entry);
>> + int pos = indent + strlen(name);
>> +
>> + if (qobject_type(qelem) == QTYPE_QLIST) {
>> + monitor_printf(mon, "%*c%s", indent, ' ', name);
>> + pos += print_array_index(mon, start, elem_no);
>> + sub_list = qobject_to_qlist(qelem);
>> + QLIST_FOREACH_ENTRY(sub_list, sub_entry) {
>> + print_elem(mon, qlist_entry_obj(sub_entry), size, pos,
>> + indent + 2);
>> + pos = -1;
>> + }
>> + } else {
>> + if (elem_no == 0) {
>> + monitor_printf(mon, "%*c%s", indent, ' ', name);
>> + } else {
>> + pos = -1;
>> + }
>> + print_elem(mon, qelem, size, pos, indent);
>> + }
>> + elem_no++;
>> + }
>> +}
>> +
>> +void device_user_print(Monitor *mon, const QObject *data)
>> +{
>> + QDict *qdict = qobject_to_qdict(data);
>> + QList *qlist = qdict_get_qlist(qdict, "fields");
>> + QListEntry *entry;
>> +
>> + monitor_printf(mon, "dev: %s, id \"%s\"\n",
>> + qdict_get_str(qdict, "device"),
>> + qdict_get_str(qdict, "id"));
>> +
>> + QLIST_FOREACH_ENTRY(qlist, entry) {
>> + print_field(mon, qobject_to_qdict(qlist_entry_obj(entry)), 2);
>> + }
>> +}
>> +
>> +static void parse_vmstate(const VMStateDescription *vmsd, void *opaque,
>> + QList *qlist, int full_buffers)
>> +{
>> + VMStateField *field = vmsd->fields;
>> +
>> + if (vmsd->pre_save) {
>> + vmsd->pre_save(opaque);
>> + }
>> + while(field->name) {
>> + if (!field->field_exists ||
>> + field->field_exists(opaque, vmsd->version_id)) {
>> + void *base_addr = opaque + field->offset;
>> + int i, n_elems = 1;
>> + int is_array = 1;
>> + size_t size = field->size;
>> + QDict *qfield = qdict_new();
>> + QList *qelems = qlist_new();
>> +
>> + qlist_append_obj(qlist, QOBJECT(qfield));
>> +
>> + qdict_put_obj(qfield, "name",
>> + QOBJECT(qstring_from_str(field->name)));
>> + qdict_put_obj(qfield, "elems", QOBJECT(qelems));
>> +
>> + if (field->flags & VMS_VBUFFER) {
>> + size = *(int32_t *)(opaque + field->size_offset);
>> + if (field->flags & VMS_MULTIPLY) {
>> + size *= field->size;
>> + }
>> + }
>> + qdict_put_obj(qfield, "size", QOBJECT(qint_from_int(size)));
>> + if (field->start_index) {
>> + qdict_put_obj(qfield, "start",
>> +
>> QOBJECT(qstring_from_str(field->start_index)));
>> + }
>> +
>> + if (field->flags & VMS_ARRAY) {
>> + n_elems = field->num;
>> + } else if (field->flags & VMS_VARRAY_INT32) {
>> + n_elems = *(int32_t *)(opaque + field->num_offset);
>> + } else if (field->flags & VMS_VARRAY_UINT16) {
>> + n_elems = *(uint16_t *)(opaque + field->num_offset);
>> + } else {
>> + is_array = 0;
>> + }
>> + if (field->flags & VMS_POINTER) {
>> + base_addr = *(void **)base_addr + field->start;
>> + }
>> + for (i = 0; i < n_elems; i++) {
>> + void *addr = base_addr + size * i;
>> + QList *sub_elems = qelems;
>> + int val;
>> +
>> + if (is_array) {
>> + sub_elems = qlist_new();
>> + qlist_append_obj(qelems, QOBJECT(sub_elems));
>> + }
>> + if (field->flags & VMS_ARRAY_OF_POINTER) {
>> + addr = *(void **)addr;
>> + }
>> + if (field->flags & VMS_STRUCT) {
>> + parse_vmstate(field->vmsd, addr, sub_elems,
>> full_buffers);
>> + } else {
>> + if (field->flags & (VMS_BUFFER | VMS_VBUFFER)) {
>> + if (!full_buffers && size > 16) {
>> + size = 16;
>> + }
>> + qlist_append_obj(sub_elems,
>> + QOBJECT(qbuffer_from_data(addr,
>> + size)));
>> + } else {
>> + switch (size) {
>> + case 1:
>> + val = *(uint8_t *)addr;
>> + break;
>> + case 2:
>> + val = *(uint16_t *)addr;
>> + break;
>> + case 4:
>> + val = *(uint32_t *)addr;
>> + break;
>> + case 8:
>> + val = *(uint64_t *)addr;
>> + break;
>> + default:
>> + assert(0);
>> + }
>> + qlist_append_obj(sub_elems,
>> + QOBJECT(qint_from_int(val)));
>> + }
>> + }
>> + }
>> + }
>> + field++;
>> + }
>> +}
>> +
>> +static DeviceState *qdev_find(const char *path)
>> +{
>> + const char *dev_name;
>> + DeviceState *dev;
>> + char *bus_path;
>> + BusState *bus;
>> +
>> + dev_name = strrchr(path, '/');
>> + if (!dev_name) {
>> + bus = main_system_bus;
>> + dev_name = path;
>> + } else {
>> + dev_name++;
>> + bus_path = qemu_strdup(path);
>> + bus_path[dev_name - path] = 0;
>> +
>> + bus = qbus_find(bus_path);
>> + qemu_free(bus_path);
>> +
>> + if (!bus) {
>> + /* qbus_find already reported the error */
>> + return NULL;
>> + }
>> + }
>> + dev = qbus_find_dev(bus, dev_name);
>> + if (!dev) {
>> + qerror_report(QERR_DEVICE_NOT_FOUND, dev_name);
>> + if (!monitor_cur_is_qmp()) {
>> + qbus_list_dev(bus);
>> + }
>> + }
>> + return dev;
>> +}
>> +
>> +int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
>
> Command documentation missing.
In the make (depends on QMP doc infrastructure).
>
>> +{
>> + const char *path = qdict_get_str(qdict, "path");
>> + const VMStateDescription *vmsd;
>> + DeviceState *dev;
>> + QList *qlist;
>> +
>> + dev = qdev_find_recursive(main_system_bus, path);
>> + if (!dev) {
>> + dev = qdev_find(path);
>> + if (!dev) {
>> + return -1;
>> + }
>> + }
>
> This is inconsistent with how do_device_del() looks up devices by ID.
> Let's agree on one lookup, put it into a function, and use it
> everywhere.
Will adjust device_del to the same scheme.
BTW, qdev path completion will also be available for both commands in my
next series.
>
>> +
>> + vmsd = dev->info->vmsd;
>> + if (!vmsd) {
>> + qerror_report(QERR_UNDEFINED_ERROR);
>> + if (!monitor_cur_is_qmp()) {
>> + error_printf_unless_qmp("Device '%s' does not support state "
>> + "dumping\n", path);
>> + }
>
> Please use a suitable QERR_. Define one if necessary.
Already fixed in my tree (forgot this bit for v1). Just this redundant
qmp check was still present.
>
>> + return -1;
>> + }
>> +
>> + *ret_data = qobject_from_jsonf("{ 'device': %s, 'id': %s }",
>> + dev->info->name, dev->id ? : "");
>> + qlist = qlist_new();
>> + parse_vmstate(vmsd, dev, qlist, qdict_get_int(qdict, "full"));
>> + qdict_put_obj(qobject_to_qdict(*ret_data), "fields", QOBJECT(qlist));
>> +
>> + return 0;
>> +}
>> diff --git a/hw/qdev.h b/hw/qdev.h
>> index d8fbc73..f8436ec 100644
>> --- a/hw/qdev.h
>> +++ b/hw/qdev.h
>> @@ -177,6 +177,8 @@ void do_info_qtree(Monitor *mon);
>> void do_info_qdm(Monitor *mon);
>> int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
>> int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
>> +void device_user_print(Monitor *mon, const QObject *data);
>> +int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data);
>>
>> /*** qdev-properties.c ***/
>>
>> diff --git a/qemu-monitor.hx b/qemu-monitor.hx
>> index a8f194c..449f012 100644
>> --- a/qemu-monitor.hx
>> +++ b/qemu-monitor.hx
>> @@ -599,6 +599,22 @@ Remove device @var{id}.
>> ETEXI
>>
>> {
>> + .name = "device_show",
>> + .args_type = "path:s,full:-f",
>> + .params = "device [-f]",
>> + .help = "show device state (specify -f for full buffer
>> dumping)",
>> + .user_print = device_user_print,
>> + .mhandler.cmd_new = do_device_show,
>> + },
>> +
>> +STEXI
>> +...@item device_show @var{id} [...@code{-f}]
>> +
>> +Show state of device @var{id} in a human-readable form. Buffers are cut
>> after
>> +16 bytes unless a full dump is requested via @code{-f}
>> +ETEXI
>> +
>> + {
>> .name = "cpu",
>> .args_type = "index:i",
>> .params = "index",
>
>
Thanks for the comments!
Jan
signature.asc
Description: OpenPGP digital signature
