* [Qemu-devel] [PATCH v2 00/15] Basic device state visualization
@ 2010-05-22 8:17 Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 01/15] Add dependency of JSON unit tests on config-host.h Jan Kiszka
` (15 more replies)
0 siblings, 16 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:17 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Avi Kivity, Juan Quintela, Markus Armbruster, Luiz Capitulino
Here is version 2 of the device_show patch series. It currently has some
dependencies on recently posted doc changes / enhancements, namely:
- http://thread.gmane.org/gmane.comp.emulators.qemu/70673
([PATCH v3 0/3]: QMP: Commands doc)
- http://thread.gmane.org/gmane.comp.emulators.qemu/70756
([PATCH 1/7] QMP: Add "Downstream extension of QMP" to spec)
Major changes in v2 are:
- command line completion for device tree paths
- introduced complex object classes ("__class__") and applied that on
buffers
- documentation
- applied new qdev path specification also on device_del
- proper qdev device/bus sorting via QTAILQ (instead of QLIST_INSERT_TAIL)
- added QERR_DEVICE_NO_STATE
- fixed various bugs
- <things I forgot>
For reference, the series is also available at
git://git.kiszka.org/qemu.git queues/device-show
Thanks for all comments so far!
Jan Kiszka (15):
Add dependency of JSON unit tests on config-host.h
qdev: Fix scanning across single-bus devices
qdev: Allow device addressing via 'driver.instance'
qdev: Convert device and bus lists to QTAILQ
qdev: Allow device specification by qtree path for device_del
qdev: Push QMP mode checks into qbus_list_bus/dev
monitor: Add completion for qdev paths
Add base64 encoder/decoder
QMP: Reserve namespace for complex object classes
Add QBuffer
monitor: return length of printed string via monitor_[v]printf
monitor: Add basic device state visualization
QMP: Teach basic capability negotiation to python example
QMP: Fix python helper /wrt long return strings
QMP: Add support for buffer class to qmp python helper
Makefile | 5 +-
Makefile.objs | 4 +-
QMP/qmp-shell | 1 +
QMP/qmp-spec.txt | 24 +++-
QMP/qmp.py | 29 +++-
QMP/vm-info | 1 +
base64.c | 202 +++++++++++++++++++++++
base64.h | 18 ++
check-qbuffer.c | 172 +++++++++++++++++++
configure | 2 +-
docs/qdev-device-use.txt | 16 ++-
hw/acpi_piix4.c | 2 +-
hw/hw.h | 2 +
hw/i2c.c | 2 +-
hw/pci-hotplug.c | 2 +-
hw/qdev.c | 408 +++++++++++++++++++++++++++++++++++++++++-----
hw/qdev.h | 12 +-
hw/ssi.c | 6 +-
monitor.c | 108 +++++++++++-
monitor.h | 4 +-
qbuffer.c | 116 +++++++++++++
qbuffer.h | 33 ++++
qemu-monitor.hx | 74 ++++++++-
qemu-tool.c | 6 +-
qerror.c | 4 +
qerror.h | 3 +
qjson.c | 15 ++
qobject.h | 1 +
28 files changed, 1193 insertions(+), 79 deletions(-)
create mode 100644 base64.c
create mode 100644 base64.h
create mode 100644 check-qbuffer.c
create mode 100644 qbuffer.c
create mode 100644 qbuffer.h
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 01/15] Add dependency of JSON unit tests on config-host.h
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
@ 2010-05-22 8:17 ` Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices Jan Kiszka
` (14 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:17 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 110698e..aa81d9b 100644
--- a/Makefile
+++ b/Makefile
@@ -144,6 +144,8 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobj
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@")
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o: $(GENERATED_HEADERS)
+
check-qint: check-qint.o qint.o qemu-malloc.o
check-qstring: check-qstring.o qstring.o qemu-malloc.o
check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qemu-malloc.o qlist.o
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 01/15] Add dependency of JSON unit tests on config-host.h Jan Kiszka
@ 2010-05-22 8:17 ` Jan Kiszka
2010-05-29 7:38 ` Markus Armbruster
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance' Jan Kiszka
` (13 subsequent siblings)
15 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:17 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to
make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as
there is only one child bus per device.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
docs/qdev-device-use.txt | 4 ++++
hw/qdev.c | 12 +++++++++++-
2 files changed, 15 insertions(+), 1 deletions(-)
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index f252c8e..9ac1fa1 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -20,6 +20,10 @@ bus named pci.0. To put a FOO device into its slot 4, use -device
FOO,bus=/i440FX-pcihost/pci.0,addr=4. The abbreviated form bus=pci.0
also works as long as the bus name is unique.
+Furthermore, if a device only hosts a single bus, the bus name can be
+omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates
+/i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings.
+
Note: the USB device address can't be controlled at this time.
=== Block Devices ===
diff --git a/hw/qdev.c b/hw/qdev.c
index aa2ce01..2e50531 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -557,7 +557,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)
static BusState *qbus_find(const char *path)
{
- DeviceState *dev;
+ DeviceState *dev, *next_dev;
BusState *bus;
char elem[128];
int pos, len;
@@ -603,6 +603,7 @@ static BusState *qbus_find(const char *path)
return NULL;
}
+search_dev_bus:
assert(path[pos] == '/' || !path[pos]);
while (path[pos] == '/') {
pos++;
@@ -633,6 +634,15 @@ static BusState *qbus_find(const char *path)
pos += len;
bus = qbus_find_bus(dev, elem);
if (!bus) {
+ if (dev->num_child_bus == 1) {
+ /* Last element might have been a short-cut to a device on
+ * the single child bus of the parent device. */
+ next_dev = qbus_find_dev(QTAILQ_FIRST(&dev->child_bus), elem);
+ if (next_dev) {
+ dev = next_dev;
+ goto search_dev_bus;
+ }
+ }
qerror_report(QERR_BUS_NOT_FOUND, elem);
if (!monitor_cur_is_qmp()) {
qbus_list_bus(dev);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance'
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 01/15] Add dependency of JSON unit tests on config-host.h Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-29 7:50 ` Markus Armbruster
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 04/15] qdev: Convert device and bus lists to QTAILQ Jan Kiszka
` (12 subsequent siblings)
15 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Extend qbus_find_dev to allow addressing of devices without an unique id
via an optional per-bus instance number. The new formats are
'driver.instance' and 'alias.instance'.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
docs/qdev-device-use.txt | 12 +++++++++++-
hw/qdev.c | 23 ++++++++++++++++++-----
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
index 9ac1fa1..5939481 100644
--- a/docs/qdev-device-use.txt
+++ b/docs/qdev-device-use.txt
@@ -1,6 +1,6 @@
= How to convert to -device & friends =
-=== Specifying Bus and Address on Bus ===
+=== Specifying Bus, Address on Bus, and Devices ===
In qdev, each device has a parent bus. Some devices provide one or
more buses for children. You can specify a device's parent bus with
@@ -24,6 +24,16 @@ Furthermore, if a device only hosts a single bus, the bus name can be
omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates
/i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings.
+Existing devices can be addressed either via a unique ID if it was
+assigned during creation or via the device tree path:
+
+/full_bus_address/driver_name[.instance_number]
+ or
+abbreviated_bus_address/driver_name[.instance_number]
+
+Example: /i440FX-pcihost/pci.0/e1000.2 addresses the second e1000
+adapter on the bus 'pci.0'.
+
Note: the USB device address can't be controlled at this time.
=== Block Devices ===
diff --git a/hw/qdev.c b/hw/qdev.c
index 2e50531..6b4a629 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -527,28 +527,41 @@ static BusState *qbus_find_bus(DeviceState *dev, char *elem)
return NULL;
}
-static DeviceState *qbus_find_dev(BusState *bus, char *elem)
+static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
{
DeviceState *dev;
+ int instance, n;
+ char buf[128];
/*
* try to match in order:
* (1) instance id, if present
- * (2) driver name
- * (3) driver alias, if present
+ * (2) driver name [.instance]
+ * (3) driver alias [.instance], if present
*/
QLIST_FOREACH(dev, &bus->children, sibling) {
if (dev->id && strcmp(dev->id, elem) == 0) {
return dev;
}
}
+
+ if (sscanf(elem, "%127[^.].%u", buf, &instance) == 2) {
+ elem = buf;
+ } else {
+ instance = 0;
+ }
+
+ n = 0;
QLIST_FOREACH(dev, &bus->children, sibling) {
- if (strcmp(dev->info->name, elem) == 0) {
+ if (strcmp(dev->info->name, elem) == 0 && n++ == instance) {
return dev;
}
}
+
+ n = 0;
QLIST_FOREACH(dev, &bus->children, sibling) {
- if (dev->info->alias && strcmp(dev->info->alias, elem) == 0) {
+ if (dev->info->alias && strcmp(dev->info->alias, elem) == 0 &&
+ n++ == instance) {
return dev;
}
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 04/15] qdev: Convert device and bus lists to QTAILQ
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (2 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance' Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 05/15] qdev: Allow device specification by qtree path for device_del Jan Kiszka
` (11 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Cosmetic change to align the instance number assignment with bus
ordering. The current ordering due to QLIST_INSERT_HEAD is a bit
annoying when you dump the qtree or address devices via
'driver.instance'.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/acpi_piix4.c | 2 +-
hw/i2c.c | 2 +-
hw/pci-hotplug.c | 2 +-
hw/qdev.c | 43 ++++++++++++++++++++++---------------------
hw/qdev.h | 8 ++++----
hw/ssi.c | 6 +++---
6 files changed, 32 insertions(+), 31 deletions(-)
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 0fce958..3cb3d11 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -536,7 +536,7 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
PCIDevice *dev;
int slot = ffs(val) - 1;
- QLIST_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
+ QTAILQ_FOREACH_SAFE(qdev, &bus->children, sibling, next) {
dev = DO_UPCAST(PCIDevice, qdev, qdev);
if (PCI_SLOT(dev->devfn) == slot) {
qdev_free(qdev);
diff --git a/hw/i2c.c b/hw/i2c.c
index bee8e88..61ab6fa 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -84,7 +84,7 @@ int i2c_start_transfer(i2c_bus *bus, uint8_t address, int recv)
DeviceState *qdev;
i2c_slave *slave = NULL;
- QLIST_FOREACH(qdev, &bus->qbus.children, sibling) {
+ QTAILQ_FOREACH(qdev, &bus->qbus.children, sibling) {
i2c_slave *candidate = I2C_SLAVE_FROM_QDEV(qdev);
if (candidate->address == address) {
slave = candidate;
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index cc45c50..a226d3c 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -77,7 +77,7 @@ static int scsi_hot_add(Monitor *mon, DeviceState *adapter,
SCSIBus *scsibus;
SCSIDevice *scsidev;
- scsibus = DO_UPCAST(SCSIBus, qbus, QLIST_FIRST(&adapter->child_bus));
+ scsibus = DO_UPCAST(SCSIBus, qbus, QTAILQ_FIRST(&adapter->child_bus));
if (!scsibus || strcmp(scsibus->qbus.info->name, "SCSI") != 0) {
error_report("Device is not a SCSI adapter");
return -1;
diff --git a/hw/qdev.c b/hw/qdev.c
index 6b4a629..6d55e50 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -85,10 +85,11 @@ static DeviceState *qdev_create_from_info(BusState *bus, DeviceInfo *info)
dev = qemu_mallocz(info->size);
dev->info = info;
dev->parent_bus = bus;
+ QTAILQ_INIT(&dev->child_bus);
qdev_prop_set_defaults(dev, dev->info->props);
qdev_prop_set_defaults(dev, dev->parent_bus->info->props);
qdev_prop_set_globals(dev);
- QLIST_INSERT_HEAD(&bus->children, dev, sibling);
+ QTAILQ_INSERT_TAIL(&bus->children, dev, sibling);
if (qdev_hotplug) {
assert(bus->allow_hotplug);
dev->hotplugged = 1;
@@ -337,7 +338,7 @@ void qdev_free(DeviceState *dev)
if (dev->state == DEV_STATE_INITIALIZED) {
while (dev->num_child_bus) {
- bus = QLIST_FIRST(&dev->child_bus);
+ bus = QTAILQ_FIRST(&dev->child_bus);
qbus_free(bus);
}
if (dev->info->vmsd)
@@ -348,7 +349,7 @@ void qdev_free(DeviceState *dev)
qemu_opts_del(dev->opts);
}
qemu_unregister_reset(qdev_reset, dev);
- QLIST_REMOVE(dev, sibling);
+ QTAILQ_REMOVE(&dev->parent_bus->children, dev, sibling);
qemu_free(dev);
}
@@ -432,7 +433,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name)
{
BusState *bus;
- QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(bus, &dev->child_bus, sibling) {
if (strcmp(name, bus->name) == 0) {
return bus;
}
@@ -457,8 +458,8 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,
return bus;
}
- QLIST_FOREACH(dev, &bus->children, sibling) {
- QLIST_FOREACH(child, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
ret = qbus_find_recursive(child, name, info);
if (ret) {
return ret;
@@ -473,10 +474,10 @@ static DeviceState *qdev_find_recursive(BusState *bus, const char *id)
DeviceState *dev, *ret;
BusState *child;
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
if (dev->id && strcmp(dev->id, id) == 0)
return dev;
- QLIST_FOREACH(child, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
ret = qdev_find_recursive(child, id);
if (ret) {
return ret;
@@ -493,7 +494,7 @@ static void qbus_list_bus(DeviceState *dev)
error_printf("child busses at \"%s\":",
dev->id ? dev->id : dev->info->name);
- QLIST_FOREACH(child, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
error_printf("%s\"%s\"", sep, child->name);
sep = ", ";
}
@@ -506,7 +507,7 @@ static void qbus_list_dev(BusState *bus)
const char *sep = " ";
error_printf("devices at \"%s\":", bus->name);
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
error_printf("%s\"%s\"", sep, dev->info->name);
if (dev->id)
error_printf("/\"%s\"", dev->id);
@@ -519,7 +520,7 @@ static BusState *qbus_find_bus(DeviceState *dev, char *elem)
{
BusState *child;
- QLIST_FOREACH(child, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
if (strcmp(child->name, elem) == 0) {
return child;
}
@@ -539,7 +540,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
* (2) driver name [.instance]
* (3) driver alias [.instance], if present
*/
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
if (dev->id && strcmp(dev->id, elem) == 0) {
return dev;
}
@@ -552,14 +553,14 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
}
n = 0;
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
if (strcmp(dev->info->name, elem) == 0 && n++ == instance) {
return dev;
}
}
n = 0;
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
if (dev->info->alias && strcmp(dev->info->alias, elem) == 0 &&
n++ == instance) {
return dev;
@@ -629,7 +630,7 @@ search_dev_bus:
qerror_report(QERR_DEVICE_NO_BUS, elem);
return NULL;
case 1:
- return QLIST_FIRST(&dev->child_bus);
+ return QTAILQ_FIRST(&dev->child_bus);
default:
qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
if (!monitor_cur_is_qmp()) {
@@ -694,9 +695,9 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
bus->name = buf;
}
- QLIST_INIT(&bus->children);
+ QTAILQ_INIT(&bus->children);
if (parent) {
- QLIST_INSERT_HEAD(&parent->child_bus, bus, sibling);
+ QTAILQ_INSERT_TAIL(&parent->child_bus, bus, sibling);
parent->num_child_bus++;
}
@@ -716,11 +717,11 @@ void qbus_free(BusState *bus)
{
DeviceState *dev;
- while ((dev = QLIST_FIRST(&bus->children)) != NULL) {
+ while ((dev = QTAILQ_FIRST(&bus->children)) != NULL) {
qdev_free(dev);
}
if (bus->parent) {
- QLIST_REMOVE(bus, sibling);
+ QTAILQ_REMOVE(&bus->parent->child_bus, bus, sibling);
bus->parent->num_child_bus--;
}
if (bus->qdev_allocated) {
@@ -769,7 +770,7 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
qdev_print_props(mon, dev, dev->parent_bus->info->props, "bus", indent);
if (dev->parent_bus->info->print_dev)
dev->parent_bus->info->print_dev(mon, dev, indent);
- QLIST_FOREACH(child, &dev->child_bus, sibling) {
+ QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
qbus_print(mon, child, indent);
}
}
@@ -781,7 +782,7 @@ static void qbus_print(Monitor *mon, BusState *bus, int indent)
qdev_printf("bus: %s\n", bus->name);
indent += 2;
qdev_printf("type %s\n", bus->info->name);
- QLIST_FOREACH(dev, &bus->children, sibling) {
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
qdev_print(mon, dev, indent);
}
}
diff --git a/hw/qdev.h b/hw/qdev.h
index a44060e..53f5565 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -41,9 +41,9 @@ struct DeviceState {
qemu_irq *gpio_out;
int num_gpio_in;
qemu_irq *gpio_in;
- QLIST_HEAD(, BusState) child_bus;
+ QTAILQ_HEAD(, BusState) child_bus;
int num_child_bus;
- QLIST_ENTRY(DeviceState) sibling;
+ QTAILQ_ENTRY(DeviceState) sibling;
int instance_id_alias;
int alias_required_for_version;
};
@@ -62,8 +62,8 @@ struct BusState {
const char *name;
int allow_hotplug;
int qdev_allocated;
- QLIST_HEAD(, DeviceState) children;
- QLIST_ENTRY(BusState) sibling;
+ QTAILQ_HEAD(, DeviceState) children;
+ QTAILQ_ENTRY(BusState) sibling;
};
struct Property {
diff --git a/hw/ssi.c b/hw/ssi.c
index cfe7c07..2c18436 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -25,8 +25,8 @@ static int ssi_slave_init(DeviceState *dev, DeviceInfo *base_info)
SSIBus *bus;
bus = FROM_QBUS(SSIBus, qdev_get_parent_bus(dev));
- if (QLIST_FIRST(&bus->qbus.children) != dev
- || QLIST_NEXT(dev, sibling) != NULL) {
+ if (QTAILQ_FIRST(&bus->qbus.children) != dev
+ || QTAILQ_NEXT(dev, sibling) != NULL) {
hw_error("Too many devices on SSI bus");
}
@@ -61,7 +61,7 @@ uint32_t ssi_transfer(SSIBus *bus, uint32_t val)
{
DeviceState *dev;
SSISlave *slave;
- dev = QLIST_FIRST(&bus->qbus.children);
+ dev = QTAILQ_FIRST(&bus->qbus.children);
if (!dev) {
return 0;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 05/15] qdev: Allow device specification by qtree path for device_del
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (3 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 04/15] qdev: Convert device and bus lists to QTAILQ Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 06/15] qdev: Push QMP mode checks into qbus_list_bus/dev Jan Kiszka
` (10 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Allow to specify the device to be removed via device_del not only by ID
but also by its full or abbreviated qtree path. For this purpose,
qdev_find is introduced which combines searching for device IDs with
walking the qtree when required.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/qdev.c | 46 ++++++++++++++++++++++++++++++++++++++++++----
qemu-monitor.hx | 10 +++++-----
2 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index 6d55e50..fa611a1 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -666,6 +666,44 @@ search_dev_bus:
}
}
+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 = qdev_find_recursive(bus, path);
+ if (dev) {
+ return dev;
+ }
+ 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;
+}
+
void qbus_create_inplace(BusState *bus, BusInfo *info,
DeviceState *parent, const char *name)
{
@@ -824,12 +862,12 @@ int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
{
- const char *id = qdict_get_str(qdict, "id");
+ const char *path = qdict_get_str(qdict, "path");
DeviceState *dev;
- dev = qdev_find_recursive(main_system_bus, id);
- if (NULL == dev) {
- qerror_report(QERR_DEVICE_NOT_FOUND, id);
+ dev = qdev_find(path);
+ if (!dev) {
+ qerror_report(QERR_DEVICE_NOT_FOUND, path);
return -1;
}
return qdev_unplug(dev);
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index c8f1789..754d71e 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -703,7 +703,7 @@ EQMP
{
.name = "device_del",
- .args_type = "id:s",
+ .args_type = "path:s",
.params = "device",
.help = "remove device",
.user_print = monitor_user_noop,
@@ -711,10 +711,10 @@ EQMP
},
STEXI
-@item device_del @var{id}
+@item device_del @var{path}
@findex device_del
-Remove device @var{id}.
+Remove device @var{path}.
ETEXI
SQMP
device_del
@@ -724,11 +724,11 @@ Remove a device.
Arguments:
-- "id": the device's ID (json-string)
+- "path": the device's qtree path or unique ID (json-string)
Example:
--> { "execute": "device_del", "arguments": { "id": "net1" } }
+-> { "execute": "device_del", "arguments": { "path": "net1" } }
<- { "return": {} }
EQMP
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 06/15] qdev: Push QMP mode checks into qbus_list_bus/dev
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (4 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 05/15] qdev: Allow device specification by qtree path for device_del Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 07/15] monitor: Add completion for qdev paths Jan Kiszka
` (9 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Simplifies the usage.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/qdev.c | 22 ++++++++++------------
1 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index fa611a1..db005ce 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -492,6 +492,9 @@ static void qbus_list_bus(DeviceState *dev)
BusState *child;
const char *sep = " ";
+ if (monitor_cur_is_qmp()) {
+ return;
+ }
error_printf("child busses at \"%s\":",
dev->id ? dev->id : dev->info->name);
QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
@@ -506,6 +509,9 @@ static void qbus_list_dev(BusState *bus)
DeviceState *dev;
const char *sep = " ";
+ if (monitor_cur_is_qmp()) {
+ return;
+ }
error_printf("devices at \"%s\":", bus->name);
QTAILQ_FOREACH(dev, &bus->children, sibling) {
error_printf("%s\"%s\"", sep, dev->info->name);
@@ -611,9 +617,7 @@ static BusState *qbus_find(const char *path)
dev = qbus_find_dev(bus, elem);
if (!dev) {
qerror_report(QERR_DEVICE_NOT_FOUND, elem);
- if (!monitor_cur_is_qmp()) {
- qbus_list_dev(bus);
- }
+ qbus_list_dev(bus);
return NULL;
}
@@ -633,9 +637,7 @@ search_dev_bus:
return QTAILQ_FIRST(&dev->child_bus);
default:
qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
- if (!monitor_cur_is_qmp()) {
- qbus_list_bus(dev);
- }
+ qbus_list_bus(dev);
return NULL;
}
}
@@ -658,9 +660,7 @@ search_dev_bus:
}
}
qerror_report(QERR_BUS_NOT_FOUND, elem);
- if (!monitor_cur_is_qmp()) {
- qbus_list_bus(dev);
- }
+ qbus_list_bus(dev);
return NULL;
}
}
@@ -697,9 +697,7 @@ static DeviceState *qdev_find(const char *path)
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);
- }
+ qbus_list_dev(bus);
}
return dev;
}
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 07/15] monitor: Add completion for qdev paths
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (5 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 06/15] qdev: Push QMP mode checks into qbus_list_bus/dev Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder Jan Kiszka
` (8 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Implement monitor command line completion for device tree paths. The
first user is device_del.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/qdev.c | 50 ++++++++++++++++++++++----------
hw/qdev.h | 2 +
monitor.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
qemu-monitor.hx | 2 +-
4 files changed, 123 insertions(+), 16 deletions(-)
diff --git a/hw/qdev.c b/hw/qdev.c
index db005ce..7db839f 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -39,7 +39,7 @@ DeviceInfo *device_info_list;
static BusState *qbus_find_recursive(BusState *bus, const char *name,
const BusInfo *info);
-static BusState *qbus_find(const char *path);
+static BusState *qbus_find_internal(const char *path, bool report_errors);
/* Register a new device type. */
void qdev_register(DeviceInfo *info)
@@ -217,7 +217,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
/* find bus */
path = qemu_opt_get(opts, "bus");
if (path != NULL) {
- bus = qbus_find(path);
+ bus = qbus_find_internal(path, true);
if (!bus) {
return NULL;
}
@@ -575,7 +575,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)
return NULL;
}
-static BusState *qbus_find(const char *path)
+static BusState *qbus_find_internal(const char *path, bool report_errors)
{
DeviceState *dev, *next_dev;
BusState *bus;
@@ -593,7 +593,9 @@ static BusState *qbus_find(const char *path)
}
bus = qbus_find_recursive(main_system_bus, elem, NULL);
if (!bus) {
- qerror_report(QERR_BUS_NOT_FOUND, elem);
+ if (report_errors) {
+ qerror_report(QERR_BUS_NOT_FOUND, elem);
+ }
return NULL;
}
pos = len;
@@ -616,8 +618,10 @@ static BusState *qbus_find(const char *path)
pos += len;
dev = qbus_find_dev(bus, elem);
if (!dev) {
- qerror_report(QERR_DEVICE_NOT_FOUND, elem);
- qbus_list_dev(bus);
+ if (report_errors) {
+ qerror_report(QERR_DEVICE_NOT_FOUND, elem);
+ qbus_list_dev(bus);
+ }
return NULL;
}
@@ -631,13 +635,17 @@ search_dev_bus:
* one child bus accept it nevertheless */
switch (dev->num_child_bus) {
case 0:
- qerror_report(QERR_DEVICE_NO_BUS, elem);
+ if (report_errors) {
+ qerror_report(QERR_DEVICE_NO_BUS, elem);
+ }
return NULL;
case 1:
return QTAILQ_FIRST(&dev->child_bus);
default:
- qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
- qbus_list_bus(dev);
+ if (report_errors) {
+ qerror_report(QERR_DEVICE_MULTIPLE_BUSSES, elem);
+ qbus_list_bus(dev);
+ }
return NULL;
}
}
@@ -659,14 +667,21 @@ search_dev_bus:
goto search_dev_bus;
}
}
- qerror_report(QERR_BUS_NOT_FOUND, elem);
- qbus_list_bus(dev);
+ if (report_errors) {
+ qerror_report(QERR_BUS_NOT_FOUND, elem);
+ qbus_list_bus(dev);
+ }
return NULL;
}
}
}
-static DeviceState *qdev_find(const char *path)
+BusState *qbus_find(const char *path)
+{
+ return qbus_find_internal(path, false);
+}
+
+static DeviceState *qdev_find_internal(const char *path, bool report_errors)
{
const char *dev_name;
DeviceState *dev;
@@ -686,7 +701,7 @@ static DeviceState *qdev_find(const char *path)
bus_path = qemu_strdup(path);
bus_path[dev_name - path] = 0;
- bus = qbus_find(bus_path);
+ bus = qbus_find_internal(bus_path, report_errors);
qemu_free(bus_path);
if (!bus) {
@@ -695,13 +710,18 @@ static DeviceState *qdev_find(const char *path)
}
}
dev = qbus_find_dev(bus, dev_name);
- if (!dev) {
+ if (!dev && report_errors) {
qerror_report(QERR_DEVICE_NOT_FOUND, dev_name);
qbus_list_dev(bus);
}
return dev;
}
+DeviceState *qdev_find(const char *path)
+{
+ return qdev_find_internal(path, false);
+}
+
void qbus_create_inplace(BusState *bus, BusInfo *info,
DeviceState *parent, const char *name)
{
@@ -863,7 +883,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
const char *path = qdict_get_str(qdict, "path");
DeviceState *dev;
- dev = qdev_find(path);
+ dev = qdev_find_internal(path, true);
if (!dev) {
qerror_report(QERR_DEVICE_NOT_FOUND, path);
return -1;
diff --git a/hw/qdev.h b/hw/qdev.h
index 53f5565..b27d33b 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -165,6 +165,7 @@ void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n);
CharDriverState *qdev_init_chardev(DeviceState *dev);
BusState *qdev_get_parent_bus(DeviceState *dev);
+DeviceState *qdev_find(const char *path);
/*** BUS API. ***/
@@ -172,6 +173,7 @@ void qbus_create_inplace(BusState *bus, BusInfo *info,
DeviceState *parent, const char *name);
BusState *qbus_create(BusInfo *info, DeviceState *parent, const char *name);
void qbus_free(BusState *bus);
+BusState *qbus_find(const char *path);
#define FROM_QBUS(type, dev) DO_UPCAST(type, qbus, dev)
diff --git a/monitor.c b/monitor.c
index 4c95d7b..64de10a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -64,6 +64,7 @@
*
* 'F' filename
* 'B' block device name
+ * 'Q' qdev device path
* 's' string (accept optional quote)
* 'O' option string of the form NAME=VALUE,...
* parsed according to QemuOptsList given by its name
@@ -3437,6 +3438,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
switch(c) {
case 'F':
case 'B':
+ case 'Q':
case 's':
{
int ret;
@@ -3461,6 +3463,10 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
monitor_printf(mon, "%s: block device name expected\n",
cmdname);
break;
+ case 'Q':
+ monitor_printf(mon, "%s: qdev device path expected\n",
+ cmdname);
+ break;
default:
monitor_printf(mon, "%s: string expected\n", cmdname);
break;
@@ -3947,6 +3953,79 @@ static void block_completion_it(void *opaque, BlockDriverState *bs)
}
}
+static void add_qdev_completion(const char *parent_path, const char *name,
+ bool trailing_slash)
+{
+ size_t parent_len = strlen(parent_path);
+ size_t name_len = strlen(name);
+ char *completion = qemu_malloc(parent_len + name_len + 2);
+
+ memcpy(completion, parent_path, parent_len);
+ memcpy(completion + parent_len, name, name_len);
+ if (trailing_slash) {
+ completion[parent_len + name_len] = '/';
+ completion[parent_len + name_len + 1] = 0;
+ } else {
+ completion[parent_len + name_len] = 0;
+ }
+ readline_add_completion(cur_mon->rs, completion);
+
+ qemu_free(completion);
+}
+
+static void qdev_completion(const char *input)
+{
+ size_t parent_len, name_len;
+ char *parent_path = NULL;
+ const char *p, *name;
+ DeviceState *dev;
+ BusState *bus;
+
+ p = strrchr(input, '/');
+ if (!p) {
+ if (*input == '\0') {
+ readline_add_completion(cur_mon->rs, "/");
+ }
+ return;
+ }
+
+ p++;
+ parent_path = qemu_strndup(input, p - input);
+ bus = qbus_find(parent_path);
+
+ if (bus) {
+ parent_len = strlen(parent_path);
+ name_len = strlen(bus->name);
+ if (bus->parent && strncmp(bus->name, p, strlen(p)) == 0 &&
+ (parent_len - 1 < name_len ||
+ strncmp(parent_path + parent_len - 1 - name_len, bus->name,
+ name_len) != 0)) {
+ add_qdev_completion(parent_path, bus->name, true);
+ }
+ QTAILQ_FOREACH(dev, &bus->children, sibling) {
+ name = dev->id ? : dev->info->name;
+ if (strncmp(name, p, strlen(p)) == 0) {
+ add_qdev_completion(parent_path, name, false);
+ if (!QTAILQ_EMPTY(&dev->child_bus)) {
+ add_qdev_completion(parent_path, name, true);
+ }
+ }
+ }
+ } else {
+ parent_path[strlen(parent_path) - 1] = 0;
+ dev = qdev_find(parent_path);
+ if (dev) {
+ parent_path[strlen(parent_path)] = '/';
+ QTAILQ_FOREACH(bus, &dev->child_bus, sibling) {
+ if (strncmp(bus->name, p, strlen(p)) == 0) {
+ add_qdev_completion(parent_path, bus->name, true);
+ }
+ }
+ }
+ }
+ qemu_free(parent_path);
+}
+
/* NOTE: this parser is an approximate form of the real command parser */
static void parse_cmdline(const char *cmdline,
int *pnb_args, char **args)
@@ -4044,6 +4123,11 @@ static void monitor_find_completion(const char *cmdline)
readline_set_completion_index(cur_mon->rs, strlen(str));
bdrv_iterate(block_completion_it, (void *)str);
break;
+ case 'Q':
+ /* qdev device path completion */
+ readline_set_completion_index(cur_mon->rs, strlen(str));
+ qdev_completion(str);
+ break;
case 's':
/* XXX: more generic ? */
if (!strcmp(cmd->name, "info")) {
@@ -4122,6 +4206,7 @@ static int check_arg(const CmdArgs *cmd_args, QDict *args)
switch (cmd_args->type) {
case 'F':
case 'B':
+ case 'Q':
case 's':
if (qobject_type(value) != QTYPE_QSTRING) {
qerror_report(QERR_INVALID_PARAMETER_TYPE, name, "string");
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 754d71e..eb257a6 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -703,7 +703,7 @@ EQMP
{
.name = "device_del",
- .args_type = "path:s",
+ .args_type = "path:Q",
.params = "device",
.help = "remove device",
.user_print = monitor_user_noop,
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (6 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 07/15] monitor: Add completion for qdev paths Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 13:59 ` Blue Swirl
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 09/15] QMP: Reserve namespace for complex object classes Jan Kiszka
` (7 subsequent siblings)
15 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Will be used by QBuffer.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile.objs | 2 +-
base64.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
base64.h | 18 +++++
3 files changed, 221 insertions(+), 1 deletions(-)
create mode 100644 base64.c
create mode 100644 base64.h
diff --git a/Makefile.objs b/Makefile.objs
index acbaf22..2c603b2 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,7 @@
# QObject
qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
-qobject-obj-y += qerror.o
+qobject-obj-y += qerror.o base64.o
#######################################################################
# block-obj-y is code used by both qemu system emulation and qemu-img
diff --git a/base64.c b/base64.c
new file mode 100644
index 0000000..543e8c6
--- /dev/null
+++ b/base64.c
@@ -0,0 +1,202 @@
+/*
+ * Base64 encoder/decoder conforming to RFC 4648
+ * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "inttypes.h"
+#include "base64.h"
+
+static const char base[] =
+ "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
+
+static void encode3to4(const char *src, char *dest)
+{
+ uint32_t b32 = 0;
+ int i, j = 18;
+
+ for (i = 0; i < 3; i++) {
+ b32 <<= 8;
+ b32 |= src[i];
+ }
+ for (i = 0; i < 4; i++) {
+ dest[i] = base[(b32 >> j) & 0x3F];
+ j -= 6;
+ }
+}
+
+static void encode2to4(const char *src, char *dest)
+{
+ dest[0] = base[(src[0] >> 2) & 0x3F];
+ dest[1] = base[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0F)];
+ dest[2] = base[(src[1] & 0x0F) << 2];
+ dest[3] = '=';
+}
+
+static void encode1to4(const char *src, char *dest)
+{
+ dest[0] = base[(src[0] >> 2) & 0x3F];
+ dest[1] = base[(src[0] & 0x03) << 4];
+ dest[2] = '=';
+ dest[3] = '=';
+}
+
+/*
+ * Encode data in 'src' of length 'srclen' to a base64 string, saving the
+ * null-terminated result in 'dest'. The size of the destition buffer must be
+ * at least ((srclen + 2) / 3) * 4 + 1.
+ */
+void base64_encode(const void *src, size_t srclen, char *dest)
+{
+ while (srclen >= 3) {
+ encode3to4(src, dest);
+ src += 3;
+ dest += 4;
+ srclen -= 3;
+ }
+ switch (srclen) {
+ case 2:
+ encode2to4(src, dest);
+ dest += 4;
+ break;
+ case 1:
+ encode1to4(src, dest);
+ dest += 4;
+ break;
+ case 0:
+ break;
+ }
+ dest[0] = 0;
+}
+
+static int32_t codetovalue(char c)
+{
+ if (c >= 'A' && c <= 'Z') {
+ return c - 'A';
+ } else if (c >= 'a' && c <= 'z') {
+ return c - 'a' + 26;
+ } else if (c >= '0' && c <= '9') {
+ return c - '0' + 52;
+ } else if (c == '+') {
+ return 62;
+ } else if ( c == '/') {
+ return 63;
+ } else {
+ return -1;
+ }
+}
+
+static int decode4to3 (const char *src, char *dest)
+{
+ uint32_t b32 = 0;
+ int32_t bits;
+ int i;
+
+ for (i = 0; i < 4; i++) {
+ bits = codetovalue(src[i]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 <<= 6;
+ b32 |= bits;
+ }
+ dest[0] = (b32 >> 16) & 0xFF;
+ dest[1] = (b32 >> 8) & 0xFF;
+ dest[2] = b32 & 0xFF;
+
+ return 0;
+}
+
+static int decode3to2(const char *src, char *dest)
+{
+ uint32_t b32 = 0;
+ int32_t bits;
+
+ bits = codetovalue(src[0]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 = (uint32_t)bits;
+ b32 <<= 6;
+
+ bits = codetovalue(src[1]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 |= (uint32_t)bits;
+ b32 <<= 4;
+
+ bits = codetovalue(src[2]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 |= ((uint32_t)bits) >> 2;
+
+ dest[0] = (b32 >> 8) & 0xFF;
+ dest[1] = b32 & 0xFF;
+
+ return 0;
+}
+
+static int decode2to1(const char *src, char *dest)
+{
+ uint32_t b32;
+ int32_t bits;
+
+ bits = codetovalue(src[0]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 = (uint32_t)bits << 2;
+
+ bits = codetovalue(src[1]);
+ if (bits < 0) {
+ return bits;
+ }
+ b32 |= ((uint32_t)bits) >> 4;
+
+ dest[0] = b32;
+
+ return 0;
+}
+
+/*
+ * Convert string 'src' of length 'srclen' from base64 to binary form,
+ * saving the result in 'dest'. The size of the destination buffer must be at
+ * least srclen * 3 / 4.
+ *
+ * Returns 0 on success, -1 on conversion error.
+ */
+int base64_decode(const char *src, size_t srclen, void *dest)
+{
+ int ret;
+
+ while (srclen >= 4) {
+ ret = decode4to3(src, dest);
+ if (ret < 0) {
+ return ret;
+ }
+ src += 4;
+ dest += 3;
+ srclen -= 4;
+ }
+
+ switch (srclen) {
+ case 3:
+ return decode3to2(src, dest);
+ case 2:
+ return decode2to1(src, dest);
+ case 1:
+ return -1;
+ default: /* 0 */
+ return 0;
+ }
+}
diff --git a/base64.h b/base64.h
new file mode 100644
index 0000000..9a0e03a
--- /dev/null
+++ b/base64.h
@@ -0,0 +1,18 @@
+/*
+ * Base64 encoder/decoder conforming to RFC 4648
+ * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include <unistd.h>
+
+void base64_encode(const void *src, size_t srclen, char *dest);
+int base64_decode(const char *src, size_t srclen, void *dest);
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 09/15] QMP: Reserve namespace for complex object classes
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (7 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 10/15] Add QBuffer Jan Kiszka
` (6 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
This reserves JSON objects that contain the key '__class__' for QMP-specific
complex objects. First user will be the buffer class.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
QMP/qmp-spec.txt | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index 9d30a8c..fa1dd62 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -146,6 +146,15 @@ The format is:
For a listing of supported asynchronous events, please, refer to the
qmp-events.txt file.
+2.6 Complex object classes
+--------------------------
+
+JSON objects that contain the key-value pair '"__class__": json-string' are
+reserved for QMP-specific complex object classes that. QMP specifies which
+further keys each of these objects include and how they are encoded.
+
+So far, no complex object class is specified.
+
3. QMP Examples
===============
@@ -229,9 +238,10 @@ avoid modifying QMP. Both upstream and downstream need to take care to
preserve long-term compatibility and interoperability.
To help with that, QMP reserves JSON object member names beginning with
-'__' (double underscore) for downstream use ("downstream names"). This
-means upstream will never use any downstream names for its commands,
-arguments, errors, asynchronous events, and so forth.
+'__' (double underscore) for downstream use ("downstream names"). Downstream
+names MUST NOT end with '__' as this pattern is reserved for QMP-defined JSON
+object classes. Upstream will never use any downstream names for its
+commands, arguments, errors, asynchronous events, and so forth.
Any new names downstream wishes to add must begin with '__'. To
ensure compatibility with other downstreams, it is strongly
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 10/15] Add QBuffer
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (8 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 09/15] QMP: Reserve namespace for complex object classes Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 11/15] monitor: return length of printed string via monitor_[v]printf Jan Kiszka
` (5 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
This introduces a buffer object for use with QMP. As a buffer is not
natively encodable in JSON, we encode it as a base64 string and
encapsulate the result in the new QMP object class "buffer".
The first use case for this is pushing the content of buffers that are
part of a device state into a qdict.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Makefile | 5 +-
Makefile.objs | 2 +-
QMP/qmp-spec.txt | 10 +++-
check-qbuffer.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
configure | 2 +-
qbuffer.c | 116 ++++++++++++++++++++++++++++++++++++
qbuffer.h | 33 ++++++++++
qjson.c | 15 +++++
qobject.h | 1 +
9 files changed, 351 insertions(+), 5 deletions(-)
create mode 100644 check-qbuffer.c
create mode 100644 qbuffer.c
create mode 100644 qbuffer.h
diff --git a/Makefile b/Makefile
index aa81d9b..9c226ae 100644
--- a/Makefile
+++ b/Makefile
@@ -144,14 +144,15 @@ qemu-io$(EXESUF): qemu-io.o cmd.o qemu-tool.o qemu-error.o $(block-obj-y) $(qobj
qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx
$(call quiet-command,sh $(SRC_PATH)/hxtool -h < $< > $@," GEN $@")
-check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o: $(GENERATED_HEADERS)
+check-qint.o check-qstring.o check-qdict.o check-qlist.o check-qfloat.o check-qjson.o check-qbuffer: $(GENERATED_HEADERS)
check-qint: check-qint.o qint.o qemu-malloc.o
check-qstring: check-qstring.o qstring.o qemu-malloc.o
check-qdict: check-qdict.o qdict.o qfloat.o qint.o qstring.o qbool.o qemu-malloc.o qlist.o
check-qlist: check-qlist.o qlist.o qint.o qemu-malloc.o
check-qfloat: check-qfloat.o qfloat.o qemu-malloc.o
-check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qjson.o json-streamer.o json-lexer.o json-parser.o qemu-malloc.o
+check-qjson: check-qjson.o qfloat.o qint.o qdict.o qstring.o qlist.o qbool.o qbuffer.o base64.o qjson.o json-streamer.o json-lexer.o json-parser.o qemu-malloc.o
+check-qbuffer: check-qbuffer.o qbuffer.o base64.o qstring.o qemu-malloc.o
clean:
# avoid old build problems by removing potentially incorrect old files
diff --git a/Makefile.objs b/Makefile.objs
index 2c603b2..d556806 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -1,6 +1,6 @@
#######################################################################
# QObject
-qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
+qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o qbuffer.o
qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
qobject-obj-y += qerror.o base64.o
diff --git a/QMP/qmp-spec.txt b/QMP/qmp-spec.txt
index fa1dd62..820e39d 100644
--- a/QMP/qmp-spec.txt
+++ b/QMP/qmp-spec.txt
@@ -153,7 +153,15 @@ JSON objects that contain the key-value pair '"__class__": json-string' are
reserved for QMP-specific complex object classes that. QMP specifies which
further keys each of these objects include and how they are encoded.
-So far, no complex object class is specified.
+2.6.1 Buffer class
+------------------
+
+This QMP object class allows to transport binary data. A buffer object
+consists of the following keys:
+
+{ "__class__": "buffer", "data": json-string }
+
+The data string is base64 encoded according to RFC 4648.
3. QMP Examples
===============
diff --git a/check-qbuffer.c b/check-qbuffer.c
new file mode 100644
index 0000000..b490230
--- /dev/null
+++ b/check-qbuffer.c
@@ -0,0 +1,172 @@
+/*
+ * QBuffer unit-tests.
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include <check.h>
+
+#include "qbuffer.h"
+#include "qemu-common.h"
+
+const char data[] = "some data";
+
+START_TEST(qbuffer_from_data_test)
+{
+ QBuffer *qbuffer;
+
+ qbuffer = qbuffer_from_data(data, sizeof(data));
+ fail_unless(qbuffer != NULL);
+ fail_unless(qbuffer->base.refcnt == 1);
+ fail_unless(memcmp(data, qbuffer->data, sizeof(data)) == 0);
+ fail_unless(qbuffer->size == sizeof(data));
+ fail_unless(qobject_type(QOBJECT(qbuffer)) == QTYPE_QBUFFER);
+
+ /* destroy doesn't exit yet */
+ qemu_free(qbuffer->data);
+ qemu_free(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_destroy_test)
+{
+ QBuffer *qbuffer = qbuffer_from_data(data, sizeof(data));
+
+ QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_data_test)
+{
+ QBuffer *qbuffer;
+ const void *ret_data;
+
+ qbuffer = qbuffer_from_data(data, sizeof(data));
+ ret_data = qbuffer_get_data(qbuffer);
+ fail_unless(memcmp(ret_data, data, sizeof(data)) == 0);
+
+ QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_get_size_test)
+{
+ QBuffer *qbuffer;
+
+ qbuffer = qbuffer_from_data(data, sizeof(data));
+ fail_unless(qbuffer_get_size(qbuffer) == sizeof(data));
+
+ QDECREF(qbuffer);
+}
+END_TEST
+
+START_TEST(qbuffer_from_qstring_test)
+{
+ const struct {
+ const char *encoded;
+ const char *decoded;
+ } pattern[3] = {
+ {
+ .encoded = "SGVsbG8sIFFCdWZmZXIhCg==",
+ .decoded = "Hello, QBuffer!",
+ },
+ {
+ .encoded = "SGVsbG8gUUJ1ZmZlcgo=",
+ .decoded = "Hello QBuffer",
+ },
+ {
+ .encoded = "SGVsbG8gUUJ1ZmZlciEK===",
+ .decoded = "Hello QBuffer!",
+ },
+ };
+ QBuffer *qbuffer;
+ QString *qstring;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+ qstring = qstring_from_str(pattern[i].encoded);
+ qbuffer = qbuffer_from_qstring(qstring);
+ QDECREF(qstring);
+
+ fail_unless(qbuffer != NULL);
+ fail_unless(memcmp(qbuffer_get_data(qbuffer), pattern[i].decoded,
+ sizeof(pattern[i].decoded)) == 0);
+
+ QDECREF(qbuffer);
+ }
+}
+END_TEST
+
+START_TEST(qbuffer_from_invalid_qstring_test)
+{
+ const char *pattern[] = {
+ "SGVsbG8sIFFCdWZmZXIhC",
+ "SGVsbG8gU=UJ1ZmZlcgo",
+ "SGVsbG8gUUJ1*ZmZlciEK",
+ };
+ QBuffer *qbuffer;
+ QString *qstring;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+ qstring = qstring_from_str(pattern[i]);
+ qbuffer = qbuffer_from_qstring(qstring);
+ QDECREF(qstring);
+
+ fail_unless(qbuffer == NULL);
+ }
+}
+END_TEST
+
+START_TEST(qobject_to_qbuffer_test)
+{
+ QBuffer *qbuffer;
+
+ qbuffer = qbuffer_from_data(data, sizeof(data));
+ fail_unless(qobject_to_qbuffer(QOBJECT(qbuffer)) == qbuffer);
+
+ QDECREF(qbuffer);
+}
+END_TEST
+
+static Suite *qbuffer_suite(void)
+{
+ Suite *s;
+ TCase *qbuffer_public_tcase;
+
+ s = suite_create("QBuffer test-suite");
+
+ qbuffer_public_tcase = tcase_create("Public Interface");
+ suite_add_tcase(s, qbuffer_public_tcase);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_from_data_test);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_destroy_test);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_get_data_test);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_get_size_test);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_from_qstring_test);
+ tcase_add_test(qbuffer_public_tcase, qbuffer_from_invalid_qstring_test);
+ tcase_add_test(qbuffer_public_tcase, qobject_to_qbuffer_test);
+
+ return s;
+}
+
+int main(void)
+{
+ int nf;
+ Suite *s;
+ SRunner *sr;
+
+ s = qbuffer_suite();
+ sr = srunner_create(s);
+
+ srunner_run_all(sr, CK_NORMAL);
+ nf = srunner_ntests_failed(sr);
+ srunner_free(sr);
+
+ return (nf == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}
diff --git a/configure b/configure
index 653c8d2..1c5c0f9 100755
--- a/configure
+++ b/configure
@@ -2290,7 +2290,7 @@ if test `expr "$target_list" : ".*softmmu.*"` != 0 ; then
if [ "$linux" = "yes" -o "$bsd" = "yes" -o "$solaris" = "yes" ] ; then
tools="qemu-nbd\$(EXESUF) $tools"
if [ "$check_utests" = "yes" ]; then
- tools="check-qint check-qstring check-qdict check-qlist $tools"
+ tools="check-qint check-qstring check-qdict check-qlist check-qbuffer $tools"
tools="check-qfloat check-qjson $tools"
fi
fi
diff --git a/qbuffer.c b/qbuffer.c
new file mode 100644
index 0000000..704d170
--- /dev/null
+++ b/qbuffer.c
@@ -0,0 +1,116 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qbuffer.h"
+#include "qobject.h"
+#include "qemu-common.h"
+#include "base64.h"
+
+static void qbuffer_destroy_obj(QObject *obj);
+
+static const QType qbuffer_type = {
+ .code = QTYPE_QBUFFER,
+ .destroy = qbuffer_destroy_obj,
+};
+
+/**
+ * qbuffer_from_data(): Create a new QBuffer from an existing data blob
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_data(const void *data, size_t size)
+{
+ QBuffer *qb;
+
+ qb = qemu_malloc(sizeof(*qb));
+ qb->data = qemu_malloc(size);
+ memcpy(qb->data, data, size);
+ qb->size = size;
+ QOBJECT_INIT(qb, &qbuffer_type);
+
+ return qb;
+}
+
+/**
+ * qbuffer_from_qstring(): Create a new QBuffer from a QString object that
+ * contains the data as a stream of hex-encoded bytes
+ *
+ * Returns strong reference.
+ */
+QBuffer *qbuffer_from_qstring(const QString *string)
+{
+ const char *str = qstring_get_str(string);
+ size_t str_len;
+ QBuffer *qb;
+
+ qb = qemu_malloc(sizeof(*qb));
+
+ str_len = strlen(str);
+ while (str_len > 0 && str[str_len - 1] == '=') {
+ str_len--;
+ }
+ qb->size = (str_len / 4) * 3 + ((str_len % 4) * 3) / 4;
+ qb->data = qemu_malloc(qb->size);
+
+ QOBJECT_INIT(qb, &qbuffer_type);
+
+ if (base64_decode(str, str_len, qb->data) < 0) {
+ qbuffer_destroy_obj(QOBJECT(qb));
+ return NULL;
+ }
+
+ return qb;
+}
+
+/**
+ * qbuffer_get_data(): Return pointer to stored data
+ *
+ * NOTE: Should be used with caution, if the object is deallocated
+ * this pointer becomes invalid.
+ */
+const void *qbuffer_get_data(const QBuffer *qb)
+{
+ return qb->data;
+}
+
+/**
+ * qbuffer_get_size(): Return size of stored data
+ */
+size_t qbuffer_get_size(const QBuffer *qb)
+{
+ return qb->size;
+}
+
+/**
+ * qobject_to_qbool(): Convert a QObject into a QBuffer
+ */
+QBuffer *qobject_to_qbuffer(const QObject *obj)
+{
+ if (qobject_type(obj) != QTYPE_QBUFFER)
+ return NULL;
+
+ return container_of(obj, QBuffer, base);
+}
+
+/**
+ * qbuffer_destroy_obj(): Free all memory allocated by a QBuffer object
+ */
+static void qbuffer_destroy_obj(QObject *obj)
+{
+ QBuffer *qb;
+
+ assert(obj != NULL);
+ qb = qobject_to_qbuffer(obj);
+ qemu_free(qb->data);
+ qemu_free(qb);
+}
diff --git a/qbuffer.h b/qbuffer.h
new file mode 100644
index 0000000..2e01078
--- /dev/null
+++ b/qbuffer.h
@@ -0,0 +1,33 @@
+/*
+ * QBuffer Module
+ *
+ * Copyright (C) 2010 Siemens AG
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef QBUFFER_H
+#define QBUFFER_H
+
+#include <stdint.h>
+#include "qobject.h"
+#include "qstring.h"
+
+typedef struct QBuffer {
+ QObject_HEAD;
+ void *data;
+ size_t size;
+} QBuffer;
+
+QBuffer *qbuffer_from_data(const void *data, size_t size);
+QBuffer *qbuffer_from_qstring(const QString *string);
+const void *qbuffer_get_data(const QBuffer *qb);
+size_t qbuffer_get_size(const QBuffer *qb);
+QBuffer *qobject_to_qbuffer(const QObject *obj);
+
+#endif /* QBUFFER_H */
diff --git a/qjson.c b/qjson.c
index 483c667..419b4e7 100644
--- a/qjson.c
+++ b/qjson.c
@@ -19,7 +19,9 @@
#include "qlist.h"
#include "qbool.h"
#include "qfloat.h"
+#include "qbuffer.h"
#include "qdict.h"
+#include "base64.h"
typedef struct JSONParsingState
{
@@ -235,6 +237,19 @@ static void to_json(const QObject *obj, QString *str)
}
break;
}
+ case QTYPE_QBUFFER: {
+ QBuffer *val = qobject_to_qbuffer(obj);
+ size_t data_size = qbuffer_get_size(val);
+ size_t str_len = ((data_size + 2) / 3) * 4;
+ char *buffer = qemu_malloc(str_len + 1);
+
+ base64_encode(qbuffer_get_data(val), data_size, buffer);
+ qstring_append(str, "{\"__class__\": \"buffer\", \"data\": \"");
+ qstring_append(str, buffer);
+ qstring_append(str, "\"}");
+ qemu_free(buffer);
+ break;
+ }
case QTYPE_QERROR:
/* XXX: should QError be emitted? */
case QTYPE_NONE:
diff --git a/qobject.h b/qobject.h
index 07de211..45c4fa0 100644
--- a/qobject.h
+++ b/qobject.h
@@ -44,6 +44,7 @@ typedef enum {
QTYPE_QFLOAT,
QTYPE_QBOOL,
QTYPE_QERROR,
+ QTYPE_QBUFFER,
} qtype_code;
struct QObject;
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 11/15] monitor: return length of printed string via monitor_[v]printf
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (9 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 10/15] Add QBuffer Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 12/15] monitor: Add basic device state visualization Jan Kiszka
` (4 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
This simply forwards the result of the internal vsnprintf to the callers
of monitor_printf and monitor_vprintf. When invoked over a QMP session
or in absence of an active monitor, -1 is returned.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
monitor.c | 23 +++++++++++++++--------
monitor.h | 4 ++--
qemu-tool.c | 6 ++++--
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/monitor.c b/monitor.c
index 64de10a..6766e49 100644
--- a/monitor.c
+++ b/monitor.c
@@ -258,29 +258,36 @@ static void monitor_puts(Monitor *mon, const char *str)
}
}
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
{
char buf[4096];
+ int ret;
- if (!mon)
- return;
-
+ if (!mon) {
+ return -1;
+ }
mon_print_count_inc(mon);
if (monitor_ctrl_mode(mon)) {
- return;
+ return -1;
}
- vsnprintf(buf, sizeof(buf), fmt, ap);
+ ret = vsnprintf(buf, sizeof(buf), fmt, ap);
monitor_puts(mon, buf);
+
+ return ret;
}
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
{
va_list ap;
+ int ret;
+
va_start(ap, fmt);
- monitor_vprintf(mon, fmt, ap);
+ ret = monitor_vprintf(mon, fmt, ap);
va_end(ap);
+
+ return ret;
}
void monitor_print_filename(Monitor *mon, const char *filename)
diff --git a/monitor.h b/monitor.h
index ea15469..32c0170 100644
--- a/monitor.h
+++ b/monitor.h
@@ -45,8 +45,8 @@ int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,
int monitor_get_fd(Monitor *mon, const char *fdname);
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap);
+int monitor_printf(Monitor *mon, const char *fmt, ...)
__attribute__ ((__format__ (__printf__, 2, 3)));
void monitor_print_filename(Monitor *mon, const char *filename);
void monitor_flush(Monitor *mon);
diff --git a/qemu-tool.c b/qemu-tool.c
index b39af86..f6ce6cd 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -43,12 +43,14 @@ void monitor_set_error(Monitor *mon, QError *qerror)
{
}
-void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
+int monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
{
+ return -1;
}
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+int monitor_printf(Monitor *mon, const char *fmt, ...)
{
+ return -1;
}
void monitor_print_filename(Monitor *mon, const char *filename)
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (10 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 11/15] monitor: return length of printed string via monitor_[v]printf Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 18:55 ` [Qemu-devel] " Avi Kivity
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 13/15] QMP: Teach basic capability negotiation to python example Jan Kiszka
` (3 subsequent siblings)
15 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
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. A new qerror is introduced to signal a missing vmstate. And
it comes with documentation.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
hw/hw.h | 2 +
hw/qdev.c | 244 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/qdev.h | 2 +
qemu-monitor.hx | 64 +++++++++++++++
qerror.c | 4 +
qerror.h | 3 +
6 files changed, 319 insertions(+), 0 deletions(-)
diff --git a/hw/hw.h b/hw/hw.h
index fc2d184..cc4bd5f 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;
@@ -413,6 +414,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 7db839f..a30ac56 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;
@@ -890,3 +893,244 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)
}
return qdev_unplug(dev);
}
+
+#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*" PRIx64 "\n", (int)size * 2,
+ qint_get_int(qobject_to_qint(qelem)));
+ 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);
+ if (start) {
+ pos += monitor_printf(mon, "[%s+%02x]", start, elem_no);
+ } else {
+ pos += monitor_printf(mon, "[%02x]", 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 size_t parse_vmstate(const VMStateDescription *vmsd, void *opaque,
+ QList *qlist, int full_buffers)
+{
+ VMStateField *field = vmsd->fields;
+ size_t overall_size = 0;
+
+ 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;
+ size_t real_size = 0;
+ size_t dump_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;
+ }
+ }
+ 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) {
+ real_size = parse_vmstate(field->vmsd, addr,
+ sub_elems, full_buffers);
+ } else {
+ real_size = size;
+ if (field->flags & (VMS_BUFFER | VMS_VBUFFER)) {
+ dump_size = (full_buffers || size <= 16) ? size : 16;
+ qlist_append_obj(sub_elems,
+ QOBJECT(qbuffer_from_data(addr, dump_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)));
+ }
+ }
+ overall_size += real_size;
+ }
+ qdict_put_obj(qfield, "size", QOBJECT(qint_from_int(real_size)));
+ }
+ field++;
+ }
+ return overall_size;
+}
+
+int do_device_show(Monitor *mon, const QDict *qdict, QObject **ret_data)
+{
+ 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_internal(path, true);
+ if (!dev) {
+ return -1;
+ }
+ }
+
+ vmsd = dev->info->vmsd;
+ if (!vmsd) {
+ qerror_report(QERR_DEVICE_NO_STATE, dev->info->name);
+ error_printf_unless_qmp("Note: device may simply lack complete qdev "
+ "conversion\n");
+ 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 b27d33b..447d13c 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -183,6 +183,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 eb257a6..cff16d7 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -734,6 +734,70 @@ Example:
EQMP
{
+ .name = "device_show",
+ .args_type = "path:Q,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{path} [@code{-f}]
+
+Show state of device @var{path} in a human-readable form. @var{path} can be
+an unique id specified during device creation or a full path in the device
+tree (see @code{info qtree}). Buffers are cut after 16 bytes unless a full
+dump is requested via @code{-f}.
+ETEXI
+SQMP
+device_show
+-----------
+
+Dump a snapshot of the device state. Buffers are cut after 16 bytes unless
+a full dump is requested.
+
+Arguments:
+
+- "path": the device's qtree path or unique ID (json-string)
+- "full": report full state (json-bool, optional)
+
+Schema of returned object:
+
+{ "device": json-string, "id": json-string, "fields" : [ field-objects ] }
+
+The field object array may be empty, otherwise it consists of
+
+{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
+
+"size" describes the real number of bytes required for a binary representation
+of a single field element in the array. The actually transfered amount may be
+smaller unless a full dump was requested.
+
+The element object array may be empty, otherwise it can contain
+
+- json-int objects
+- QMP buffer objects
+- field objects
+- arrays of json-ints, QMP buffers, or field objects
+
+Example:
+
+-> { "execute": "device_show", "arguments": { "path": "isa.0/i8042" } }
+<- { "return": { "device": "i8042", "id": "", "fields":
+ [ { "name": "kbd", "size": 4, "elems":
+ [ { "name": "write_cmd", "size": 1, "elems": [0] },
+ { "name": "status", "size": 1, "elems": [25] },
+ { "name": "mode", "size": 1, "elems": [3] },
+ { "name": "pending", "size": 1, "elems": [1] }
+ ] }
+ ]
+ }
+ }
+
+EQMP
+
+ {
.name = "cpu",
.args_type = "index:i",
.params = "index",
diff --git a/qerror.c b/qerror.c
index 034c7de..c50ff91 100644
--- a/qerror.c
+++ b/qerror.c
@@ -101,6 +101,10 @@ static const QErrorStringTable qerror_table[] = {
.desc = "Device '%(device)' has no child bus",
},
{
+ .error_fmt = QERR_DEVICE_NO_STATE,
+ .desc = "No state available for device '%(device)'",
+ },
+ {
.error_fmt = QERR_DUPLICATE_ID,
.desc = "Duplicate ID '%(id)' for %(object)",
},
diff --git a/qerror.h b/qerror.h
index c98c61a..e5de6dd 100644
--- a/qerror.h
+++ b/qerror.h
@@ -91,6 +91,9 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_DEVICE_NO_BUS \
"{ 'class': 'DeviceNoBus', 'data': { 'device': %s } }"
+#define QERR_DEVICE_NO_STATE \
+ "{ 'class': 'DeviceNoState', 'data': { 'device': %s } }"
+
#define QERR_DUPLICATE_ID \
"{ 'class': 'DuplicateId', 'data': { 'id': %s, 'object': %s } }"
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 13/15] QMP: Teach basic capability negotiation to python example
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (11 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 12/15] monitor: Add basic device state visualization Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 14/15] QMP: Fix python helper /wrt long return strings Jan Kiszka
` (2 subsequent siblings)
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
As sending "qmp_capabilities" on session start became mandatory, both
python examples were broken.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
QMP/qmp-shell | 1 +
QMP/vm-info | 1 +
2 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/QMP/qmp-shell b/QMP/qmp-shell
index f89b9af..a5b72d1 100755
--- a/QMP/qmp-shell
+++ b/QMP/qmp-shell
@@ -42,6 +42,7 @@ def main():
qemu = qmp.QEMUMonitorProtocol(argv[1])
qemu.connect()
+ qemu.send("qmp_capabilities")
print 'Connected!'
diff --git a/QMP/vm-info b/QMP/vm-info
index b150d82..d29e7f5 100755
--- a/QMP/vm-info
+++ b/QMP/vm-info
@@ -24,6 +24,7 @@ def main():
qemu = qmp.QEMUMonitorProtocol(argv[1])
qemu.connect()
+ qemu.send("qmp_capabilities")
for cmd in [ 'version', 'hpet', 'kvm', 'status', 'uuid', 'balloon' ]:
print cmd + ': ' + str(qemu.send('query-' + cmd))
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 14/15] QMP: Fix python helper /wrt long return strings
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (12 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 13/15] QMP: Teach basic capability negotiation to python example Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 15/15] QMP: Add support for buffer class to qmp python helper Jan Kiszka
2010-05-22 14:05 ` [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Blue Swirl
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
Remove the arbitrary limitation of 1024 characters per return string and
read complete lines instead. Required for device_show.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
QMP/qmp.py | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/QMP/qmp.py b/QMP/qmp.py
index d9da603..4062f84 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -63,10 +63,14 @@ class QEMUMonitorProtocol:
def __json_read(self):
try:
- return json.loads(self.sock.recv(1024))
+ while True:
+ line = json.loads(self.sockfile.readline())
+ if not 'event' in line:
+ return line
except ValueError:
return
def __init__(self, filename):
self.filename = filename
self.sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+ self.sockfile = self.sock.makefile()
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* [Qemu-devel] [PATCH v2 15/15] QMP: Add support for buffer class to qmp python helper
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (13 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 14/15] QMP: Fix python helper /wrt long return strings Jan Kiszka
@ 2010-05-22 8:18 ` Jan Kiszka
2010-05-22 14:05 ` [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Blue Swirl
15 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-22 8:18 UTC (permalink / raw)
To: qemu-devel, Anthony Liguori
Cc: Jan Kiszka, Avi Kivity, Juan Quintela, Markus Armbruster,
Luiz Capitulino
From: Jan Kiszka <jan.kiszka@siemens.com>
This demonstrates the conversion of QMP buffer objects and does some
minimalistic pretty-printing.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
QMP/qmp.py | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/QMP/qmp.py b/QMP/qmp.py
index 4062f84..67ca8b9 100644
--- a/QMP/qmp.py
+++ b/QMP/qmp.py
@@ -8,7 +8,7 @@
# This work is licensed under the terms of the GNU GPL, version 2. See
# the COPYING file in the top-level directory.
-import socket, json
+import socket, json, binascii
class QMPError(Exception):
pass
@@ -16,6 +16,18 @@ class QMPError(Exception):
class QMPConnectError(QMPError):
pass
+class QMPBuffer:
+ def __init__(self, data):
+ self.data = binascii.a2b_base64(data)
+
+ def __repr__(self):
+ str = ''
+ for i in range(0, len(self.data) - 1):
+ if i > 0:
+ str += ' '
+ str += binascii.b2a_hex(self.data[i])
+ return str
+
class QEMUMonitorProtocol:
def connect(self):
self.sock.connect(self.filename)
@@ -61,10 +73,19 @@ class QEMUMonitorProtocol:
# the Server won't read our input
self.sock.send(json.dumps(cmd) + ' ')
+ def __json_obj_hook(self, dct):
+ if '__class__' in dct:
+ if dct['__class__'] == 'buffer':
+ return QMPBuffer(dct['data'])
+ else:
+ return
+ return dct
+
def __json_read(self):
try:
while True:
- line = json.loads(self.sockfile.readline())
+ line = json.loads(self.sockfile.readline(),
+ object_hook=self.__json_obj_hook)
if not 'event' in line:
return line
except ValueError:
--
1.6.0.2
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder Jan Kiszka
@ 2010-05-22 13:59 ` Blue Swirl
2010-05-23 7:55 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Blue Swirl @ 2010-05-22 13:59 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Avi Kivity, Luiz Capitulino
On Sat, May 22, 2010 at 8:18 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Will be used by QBuffer.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> Makefile.objs | 2 +-
> base64.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> base64.h | 18 +++++
> 3 files changed, 221 insertions(+), 1 deletions(-)
> create mode 100644 base64.c
> create mode 100644 base64.h
>
> diff --git a/Makefile.objs b/Makefile.objs
> index acbaf22..2c603b2 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -2,7 +2,7 @@
> # QObject
> qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
> qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
> -qobject-obj-y += qerror.o
> +qobject-obj-y += qerror.o base64.o
>
> #######################################################################
> # block-obj-y is code used by both qemu system emulation and qemu-img
> diff --git a/base64.c b/base64.c
> new file mode 100644
> index 0000000..543e8c6
> --- /dev/null
> +++ b/base64.c
> @@ -0,0 +1,202 @@
> +/*
> + * Base64 encoder/decoder conforming to RFC 4648
> + * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
> + *
> + * Copyright (C) 2010 Siemens AG
> + *
> + * Authors:
> + * Jan Kiszka <jan.kiszka@siemens.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include "inttypes.h"
Why not <inttypes.h>?
> +#include "base64.h"
> +
> +static const char base[] =
> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
> +
> +static void encode3to4(const char *src, char *dest)
> +{
> + uint32_t b32 = 0;
> + int i, j = 18;
> +
> + for (i = 0; i < 3; i++) {
> + b32 <<= 8;
> + b32 |= src[i];
> + }
> + for (i = 0; i < 4; i++) {
> + dest[i] = base[(b32 >> j) & 0x3F];
> + j -= 6;
> + }
> +}
> +
> +static void encode2to4(const char *src, char *dest)
> +{
> + dest[0] = base[(src[0] >> 2) & 0x3F];
> + dest[1] = base[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0F)];
> + dest[2] = base[(src[1] & 0x0F) << 2];
> + dest[3] = '=';
> +}
> +
> +static void encode1to4(const char *src, char *dest)
> +{
> + dest[0] = base[(src[0] >> 2) & 0x3F];
> + dest[1] = base[(src[0] & 0x03) << 4];
> + dest[2] = '=';
> + dest[3] = '=';
> +}
> +
> +/*
> + * Encode data in 'src' of length 'srclen' to a base64 string, saving the
> + * null-terminated result in 'dest'. The size of the destition buffer must be
> + * at least ((srclen + 2) / 3) * 4 + 1.
> + */
> +void base64_encode(const void *src, size_t srclen, char *dest)
> +{
> + while (srclen >= 3) {
> + encode3to4(src, dest);
> + src += 3;
> + dest += 4;
> + srclen -= 3;
> + }
> + switch (srclen) {
> + case 2:
> + encode2to4(src, dest);
> + dest += 4;
> + break;
> + case 1:
> + encode1to4(src, dest);
> + dest += 4;
> + break;
> + case 0:
> + break;
> + }
> + dest[0] = 0;
> +}
> +
> +static int32_t codetovalue(char c)
> +{
> + if (c >= 'A' && c <= 'Z') {
> + return c - 'A';
> + } else if (c >= 'a' && c <= 'z') {
> + return c - 'a' + 26;
> + } else if (c >= '0' && c <= '9') {
> + return c - '0' + 52;
> + } else if (c == '+') {
> + return 62;
> + } else if ( c == '/') {
> + return 63;
> + } else {
> + return -1;
> + }
> +}
> +
> +static int decode4to3 (const char *src, char *dest)
> +{
> + uint32_t b32 = 0;
> + int32_t bits;
> + int i;
> +
> + for (i = 0; i < 4; i++) {
> + bits = codetovalue(src[i]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 <<= 6;
> + b32 |= bits;
> + }
> + dest[0] = (b32 >> 16) & 0xFF;
> + dest[1] = (b32 >> 8) & 0xFF;
> + dest[2] = b32 & 0xFF;
> +
> + return 0;
> +}
> +
> +static int decode3to2(const char *src, char *dest)
> +{
> + uint32_t b32 = 0;
> + int32_t bits;
> +
> + bits = codetovalue(src[0]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 = (uint32_t)bits;
> + b32 <<= 6;
> +
> + bits = codetovalue(src[1]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 |= (uint32_t)bits;
> + b32 <<= 4;
> +
> + bits = codetovalue(src[2]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 |= ((uint32_t)bits) >> 2;
> +
> + dest[0] = (b32 >> 8) & 0xFF;
> + dest[1] = b32 & 0xFF;
> +
> + return 0;
> +}
> +
> +static int decode2to1(const char *src, char *dest)
> +{
> + uint32_t b32;
> + int32_t bits;
> +
> + bits = codetovalue(src[0]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 = (uint32_t)bits << 2;
> +
> + bits = codetovalue(src[1]);
> + if (bits < 0) {
> + return bits;
> + }
> + b32 |= ((uint32_t)bits) >> 4;
> +
> + dest[0] = b32;
> +
> + return 0;
> +}
> +
> +/*
> + * Convert string 'src' of length 'srclen' from base64 to binary form,
> + * saving the result in 'dest'. The size of the destination buffer must be at
> + * least srclen * 3 / 4.
> + *
> + * Returns 0 on success, -1 on conversion error.
> + */
> +int base64_decode(const char *src, size_t srclen, void *dest)
I think dest should be char *, like all the functions where dest is passed to.
> +{
> + int ret;
> +
> + while (srclen >= 4) {
> + ret = decode4to3(src, dest);
> + if (ret < 0) {
> + return ret;
> + }
> + src += 4;
> + dest += 3;
> + srclen -= 4;
> + }
> +
> + switch (srclen) {
> + case 3:
> + return decode3to2(src, dest);
> + case 2:
> + return decode2to1(src, dest);
> + case 1:
> + return -1;
> + default: /* 0 */
> + return 0;
> + }
> +}
> diff --git a/base64.h b/base64.h
> new file mode 100644
> index 0000000..9a0e03a
> --- /dev/null
> +++ b/base64.h
> @@ -0,0 +1,18 @@
> +/*
> + * Base64 encoder/decoder conforming to RFC 4648
> + * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
> + *
> + * Copyright (C) 2010 Siemens AG
> + *
> + * Authors:
> + * Jan Kiszka <jan.kiszka@siemens.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
> + * See the COPYING.LIB file in the top-level directory.
> + *
> + */
> +
> +#include <unistd.h>
Maybe <stddef.h> instead, it's only for size_t?
> +
> +void base64_encode(const void *src, size_t srclen, char *dest);
> +int base64_decode(const char *src, size_t srclen, void *dest);
> --
> 1.6.0.2
>
>
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/15] Basic device state visualization
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
` (14 preceding siblings ...)
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 15/15] QMP: Add support for buffer class to qmp python helper Jan Kiszka
@ 2010-05-22 14:05 ` Blue Swirl
2010-05-23 7:55 ` Jan Kiszka
15 siblings, 1 reply; 53+ messages in thread
From: Blue Swirl @ 2010-05-22 14:05 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, qemu-devel, Markus Armbruster,
Luiz Capitulino, Avi Kivity
On Sat, May 22, 2010 at 8:17 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Here is version 2 of the device_show patch series. It currently has some
> dependencies on recently posted doc changes / enhancements, namely:
> - http://thread.gmane.org/gmane.comp.emulators.qemu/70673
> ([PATCH v3 0/3]: QMP: Commands doc)
> - http://thread.gmane.org/gmane.comp.emulators.qemu/70756
> ([PATCH 1/7] QMP: Add "Downstream extension of QMP" to spec)
I had minor comments to 8/15, otherwise looks good.
>
> Major changes in v2 are:
> - command line completion for device tree paths
> - introduced complex object classes ("__class__") and applied that on
> buffers
> - documentation
> - applied new qdev path specification also on device_del
> - proper qdev device/bus sorting via QTAILQ (instead of QLIST_INSERT_TAIL)
> - added QERR_DEVICE_NO_STATE
> - fixed various bugs
> - <things I forgot>
>
> For reference, the series is also available at
>
> git://git.kiszka.org/qemu.git queues/device-show
>
> Thanks for all comments so far!
>
> Jan Kiszka (15):
> Add dependency of JSON unit tests on config-host.h
> qdev: Fix scanning across single-bus devices
> qdev: Allow device addressing via 'driver.instance'
> qdev: Convert device and bus lists to QTAILQ
> qdev: Allow device specification by qtree path for device_del
> qdev: Push QMP mode checks into qbus_list_bus/dev
> monitor: Add completion for qdev paths
> Add base64 encoder/decoder
> QMP: Reserve namespace for complex object classes
> Add QBuffer
> monitor: return length of printed string via monitor_[v]printf
> monitor: Add basic device state visualization
> QMP: Teach basic capability negotiation to python example
> QMP: Fix python helper /wrt long return strings
> QMP: Add support for buffer class to qmp python helper
>
> Makefile | 5 +-
> Makefile.objs | 4 +-
> QMP/qmp-shell | 1 +
> QMP/qmp-spec.txt | 24 +++-
> QMP/qmp.py | 29 +++-
> QMP/vm-info | 1 +
> base64.c | 202 +++++++++++++++++++++++
> base64.h | 18 ++
> check-qbuffer.c | 172 +++++++++++++++++++
> configure | 2 +-
> docs/qdev-device-use.txt | 16 ++-
> hw/acpi_piix4.c | 2 +-
> hw/hw.h | 2 +
> hw/i2c.c | 2 +-
> hw/pci-hotplug.c | 2 +-
> hw/qdev.c | 408 +++++++++++++++++++++++++++++++++++++++++-----
> hw/qdev.h | 12 +-
> hw/ssi.c | 6 +-
> monitor.c | 108 +++++++++++-
> monitor.h | 4 +-
> qbuffer.c | 116 +++++++++++++
> qbuffer.h | 33 ++++
> qemu-monitor.hx | 74 ++++++++-
> qemu-tool.c | 6 +-
> qerror.c | 4 +
> qerror.h | 3 +
> qjson.c | 15 ++
> qobject.h | 1 +
> 28 files changed, 1193 insertions(+), 79 deletions(-)
> create mode 100644 base64.c
> create mode 100644 base64.h
> create mode 100644 check-qbuffer.c
> create mode 100644 qbuffer.c
> create mode 100644 qbuffer.h
>
>
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 12/15] monitor: Add basic device state visualization Jan Kiszka
@ 2010-05-22 18:55 ` Avi Kivity
2010-05-23 7:57 ` Jan Kiszka
2010-05-24 20:22 ` Anthony Liguori
0 siblings, 2 replies; 53+ messages in thread
From: Avi Kivity @ 2010-05-22 18:55 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Luiz Capitulino
On 05/22/2010 11:18 AM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> 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. A new qerror is introduced to signal a missing vmstate. And
> it comes with documentation.
>
> +
> +Dump a snapshot of the device state. Buffers are cut after 16 bytes unless
> +a full dump is requested.
> +
> +Arguments:
> +
> +- "path": the device's qtree path or unique ID (json-string)
>
This may be ambiguous.
> +- "full": report full state (json-bool, optional)
>
Is this needed for QMP? The client can always truncate it to any length.
> +
> +Schema of returned object:
> +
> +{ "device": json-string, "id": json-string, "fields" : [ field-objects ] }
> +
> +The field object array may be empty, otherwise it consists of
> +
> +{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
> +
> +"size" describes the real number of bytes required for a binary representation
> +of a single field element in the array. The actually transfered amount may be
> +smaller unless a full dump was requested.
>
This converts the entire qdev tree into an undocumented stable protocol
(the qdev paths were already in this state I believe). This really
worries me.
> +
> +The element object array may be empty, otherwise it can contain
> +
> +- json-int objects
> +- QMP buffer objects
> +- field objects
> +- arrays of json-ints, QMP buffers, or field objects
> +
> +Example:
> +
> +-> { "execute": "device_show", "arguments": { "path": "isa.0/i8042" } }
> +<- { "return": { "device": "i8042", "id": "", "fields":
> + [ { "name": "kbd", "size": 4, "elems":
> + [ { "name": "write_cmd", "size": 1, "elems": [0] },
> + { "name": "status", "size": 1, "elems": [25] },
> + { "name": "mode", "size": 1, "elems": [3] },
> + { "name": "pending", "size": 1, "elems": [1] }
> + ] }
> + ]
> + }
> + }
> +
> +EQMP
>
Looks good. I am only worried about long term stability and documentation.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder
2010-05-22 13:59 ` Blue Swirl
@ 2010-05-23 7:55 ` Jan Kiszka
2010-05-23 8:48 ` Avi Kivity
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-23 7:55 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Avi Kivity, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 7503 bytes --]
Blue Swirl wrote:
> On Sat, May 22, 2010 at 8:18 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Will be used by QBuffer.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> Makefile.objs | 2 +-
>> base64.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> base64.h | 18 +++++
>> 3 files changed, 221 insertions(+), 1 deletions(-)
>> create mode 100644 base64.c
>> create mode 100644 base64.h
>>
>> diff --git a/Makefile.objs b/Makefile.objs
>> index acbaf22..2c603b2 100644
>> --- a/Makefile.objs
>> +++ b/Makefile.objs
>> @@ -2,7 +2,7 @@
>> # QObject
>> qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
>> qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
>> -qobject-obj-y += qerror.o
>> +qobject-obj-y += qerror.o base64.o
>>
>> #######################################################################
>> # block-obj-y is code used by both qemu system emulation and qemu-img
>> diff --git a/base64.c b/base64.c
>> new file mode 100644
>> index 0000000..543e8c6
>> --- /dev/null
>> +++ b/base64.c
>> @@ -0,0 +1,202 @@
>> +/*
>> + * Base64 encoder/decoder conforming to RFC 4648
>> + * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
>> + *
>> + * Copyright (C) 2010 Siemens AG
>> + *
>> + * Authors:
>> + * Jan Kiszka <jan.kiszka@siemens.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "inttypes.h"
>
> Why not <inttypes.h>?
Oops, no intention.
>
>> +#include "base64.h"
>> +
>> +static const char base[] =
>> + "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";
>> +
>> +static void encode3to4(const char *src, char *dest)
>> +{
>> + uint32_t b32 = 0;
>> + int i, j = 18;
>> +
>> + for (i = 0; i < 3; i++) {
>> + b32 <<= 8;
>> + b32 |= src[i];
>> + }
>> + for (i = 0; i < 4; i++) {
>> + dest[i] = base[(b32 >> j) & 0x3F];
>> + j -= 6;
>> + }
>> +}
>> +
>> +static void encode2to4(const char *src, char *dest)
>> +{
>> + dest[0] = base[(src[0] >> 2) & 0x3F];
>> + dest[1] = base[((src[0] & 0x03) << 4) | ((src[1] >> 4) & 0x0F)];
>> + dest[2] = base[(src[1] & 0x0F) << 2];
>> + dest[3] = '=';
>> +}
>> +
>> +static void encode1to4(const char *src, char *dest)
>> +{
>> + dest[0] = base[(src[0] >> 2) & 0x3F];
>> + dest[1] = base[(src[0] & 0x03) << 4];
>> + dest[2] = '=';
>> + dest[3] = '=';
>> +}
>> +
>> +/*
>> + * Encode data in 'src' of length 'srclen' to a base64 string, saving the
>> + * null-terminated result in 'dest'. The size of the destition buffer must be
>> + * at least ((srclen + 2) / 3) * 4 + 1.
>> + */
>> +void base64_encode(const void *src, size_t srclen, char *dest)
>> +{
>> + while (srclen >= 3) {
>> + encode3to4(src, dest);
>> + src += 3;
>> + dest += 4;
>> + srclen -= 3;
>> + }
>> + switch (srclen) {
>> + case 2:
>> + encode2to4(src, dest);
>> + dest += 4;
>> + break;
>> + case 1:
>> + encode1to4(src, dest);
>> + dest += 4;
>> + break;
>> + case 0:
>> + break;
>> + }
>> + dest[0] = 0;
>> +}
>> +
>> +static int32_t codetovalue(char c)
>> +{
>> + if (c >= 'A' && c <= 'Z') {
>> + return c - 'A';
>> + } else if (c >= 'a' && c <= 'z') {
>> + return c - 'a' + 26;
>> + } else if (c >= '0' && c <= '9') {
>> + return c - '0' + 52;
>> + } else if (c == '+') {
>> + return 62;
>> + } else if ( c == '/') {
>> + return 63;
>> + } else {
>> + return -1;
>> + }
>> +}
>> +
>> +static int decode4to3 (const char *src, char *dest)
>> +{
>> + uint32_t b32 = 0;
>> + int32_t bits;
>> + int i;
>> +
>> + for (i = 0; i < 4; i++) {
>> + bits = codetovalue(src[i]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 <<= 6;
>> + b32 |= bits;
>> + }
>> + dest[0] = (b32 >> 16) & 0xFF;
>> + dest[1] = (b32 >> 8) & 0xFF;
>> + dest[2] = b32 & 0xFF;
>> +
>> + return 0;
>> +}
>> +
>> +static int decode3to2(const char *src, char *dest)
>> +{
>> + uint32_t b32 = 0;
>> + int32_t bits;
>> +
>> + bits = codetovalue(src[0]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 = (uint32_t)bits;
>> + b32 <<= 6;
>> +
>> + bits = codetovalue(src[1]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 |= (uint32_t)bits;
>> + b32 <<= 4;
>> +
>> + bits = codetovalue(src[2]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 |= ((uint32_t)bits) >> 2;
>> +
>> + dest[0] = (b32 >> 8) & 0xFF;
>> + dest[1] = b32 & 0xFF;
>> +
>> + return 0;
>> +}
>> +
>> +static int decode2to1(const char *src, char *dest)
>> +{
>> + uint32_t b32;
>> + int32_t bits;
>> +
>> + bits = codetovalue(src[0]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 = (uint32_t)bits << 2;
>> +
>> + bits = codetovalue(src[1]);
>> + if (bits < 0) {
>> + return bits;
>> + }
>> + b32 |= ((uint32_t)bits) >> 4;
>> +
>> + dest[0] = b32;
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Convert string 'src' of length 'srclen' from base64 to binary form,
>> + * saving the result in 'dest'. The size of the destination buffer must be at
>> + * least srclen * 3 / 4.
>> + *
>> + * Returns 0 on success, -1 on conversion error.
>> + */
>> +int base64_decode(const char *src, size_t srclen, void *dest)
>
> I think dest should be char *, like all the functions where dest is passed to.
The output may but need not be a string, it's binary data. And to avoid
needless warnings about signedness mismatches if unsigned char or
uint8_t buffers are passed, I chose void *.
>
>> +{
>> + int ret;
>> +
>> + while (srclen >= 4) {
>> + ret = decode4to3(src, dest);
>> + if (ret < 0) {
>> + return ret;
>> + }
>> + src += 4;
>> + dest += 3;
>> + srclen -= 4;
>> + }
>> +
>> + switch (srclen) {
>> + case 3:
>> + return decode3to2(src, dest);
>> + case 2:
>> + return decode2to1(src, dest);
>> + case 1:
>> + return -1;
>> + default: /* 0 */
>> + return 0;
>> + }
>> +}
>> diff --git a/base64.h b/base64.h
>> new file mode 100644
>> index 0000000..9a0e03a
>> --- /dev/null
>> +++ b/base64.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Base64 encoder/decoder conforming to RFC 4648
>> + * (based on Mozilla's nsprpub/lib/libc/src/base64.c)
>> + *
>> + * Copyright (C) 2010 Siemens AG
>> + *
>> + * Authors:
>> + * Jan Kiszka <jan.kiszka@siemens.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
>> + *
>> + */
>> +
>> +#include <unistd.h>
>
> Maybe <stddef.h> instead, it's only for size_t?
Makes sense.
>
>> +
>> +void base64_encode(const void *src, size_t srclen, char *dest);
>> +int base64_decode(const char *src, size_t srclen, void *dest);
>> --
>> 1.6.0.2
>>
>>
>>
Thanks,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 00/15] Basic device state visualization
2010-05-22 14:05 ` [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Blue Swirl
@ 2010-05-23 7:55 ` Jan Kiszka
0 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-23 7:55 UTC (permalink / raw)
To: Blue Swirl
Cc: Anthony Liguori, Juan Quintela, qemu-devel, Markus Armbruster,
Luiz Capitulino, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 691 bytes --]
Blue Swirl wrote:
> On Sat, May 22, 2010 at 8:17 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Here is version 2 of the device_show patch series. It currently has some
>> dependencies on recently posted doc changes / enhancements, namely:
>> - http://thread.gmane.org/gmane.comp.emulators.qemu/70673
>> ([PATCH v3 0/3]: QMP: Commands doc)
>> - http://thread.gmane.org/gmane.comp.emulators.qemu/70756
>> ([PATCH 1/7] QMP: Add "Downstream extension of QMP" to spec)
>
> I had minor comments to 8/15, otherwise looks good.
Great. I will follow up on that patch or incorporate those bits in a
potential third run.
Thanks again - and sorry for forgetting to CC you,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-22 18:55 ` [Qemu-devel] " Avi Kivity
@ 2010-05-23 7:57 ` Jan Kiszka
2010-05-23 8:44 ` Avi Kivity
2010-05-24 12:51 ` Luiz Capitulino
2010-05-24 20:22 ` Anthony Liguori
1 sibling, 2 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-23 7:57 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 3839 bytes --]
Avi Kivity wrote:
> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> 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. A new qerror is introduced to signal a missing vmstate. And
>> it comes with documentation.
>>
>> +
>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>> unless
>> +a full dump is requested.
>> +
>> +Arguments:
>> +
>> +- "path": the device's qtree path or unique ID (json-string)
>>
>
> This may be ambiguous.
Can your elaborate what precisely is ambiguous?
>
>> +- "full": report full state (json-bool, optional)
>>
>
> Is this needed for QMP? The client can always truncate it to any length.
The effect may not be needed for QMP, but I do need this channel from
the command line to the monitor pretty-printer. I could just stick
"full": json-bool back into the return dict, but that would look somehow
strange IMO.
>
>> +
>> +Schema of returned object:
>> +
>> +{ "device": json-string, "id": json-string, "fields" : [
>> field-objects ] }
>> +
>> +The field object array may be empty, otherwise it consists of
>> +
>> +{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
>> +
>> +"size" describes the real number of bytes required for a binary
>> representation
>> +of a single field element in the array. The actually transfered
>> amount may be
>> +smaller unless a full dump was requested.
>>
>
> This converts the entire qdev tree into an undocumented stable protocol
> (the qdev paths were already in this state I believe). This really
> worries me.
Being primarily a debugging tool, device_show exports the entire
(qdev'ified) vmstates via QMP. Unlike the migration protocol, it does
not provide something like backward compatibility. This would be
overkill for the intended purpose (though someone may find a different
use case one day).
I think we have the following options:
- disable device_show via QMP, limit it to the monitor console
- declare its output inherently unstable, maybe at least adding the
vmstate version to each device so that potential QMP consumers notice
that they may have to update their tools or switch to a different
processing function
Given that vmstate annotations will most probably require some work on
the output structure (and I don't have a QMP use case ATM anyway), I
would be fine with the first option for now. Still, I don't think we
will ever get beyond the second option because this service is tight to
some internals of QEMU we don't want to freeze.
>
>> +
>> +The element object array may be empty, otherwise it can contain
>> +
>> +- json-int objects
>> +- QMP buffer objects
>> +- field objects
>> +- arrays of json-ints, QMP buffers, or field objects
>> +
>> +Example:
>> +
>> +-> { "execute": "device_show", "arguments": { "path": "isa.0/i8042" } }
>> +<- { "return": { "device": "i8042", "id": "", "fields":
>> + [ { "name": "kbd", "size": 4, "elems":
>> + [ { "name": "write_cmd", "size": 1, "elems": [0] },
>> + { "name": "status", "size": 1, "elems": [25] },
>> + { "name": "mode", "size": 1, "elems": [3] },
>> + { "name": "pending", "size": 1, "elems": [1] }
>> + ] }
>> + ]
>> + }
>> + }
>> +
>> +EQMP
>>
>
> Looks good. I am only worried about long term stability and documentation.
>
Thanks,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-23 7:57 ` Jan Kiszka
@ 2010-05-23 8:44 ` Avi Kivity
2010-05-23 10:03 ` Jan Kiszka
2010-05-29 8:00 ` Markus Armbruster
2010-05-24 12:51 ` Luiz Capitulino
1 sibling, 2 replies; 53+ messages in thread
From: Avi Kivity @ 2010-05-23 8:44 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Luiz Capitulino
On 05/23/2010 10:57 AM, Jan Kiszka wrote:
> Avi Kivity wrote:
>
>> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>>
>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>
>>> 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. A new qerror is introduced to signal a missing vmstate. And
>>> it comes with documentation.
>>>
>>> +
>>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>>> unless
>>> +a full dump is requested.
>>> +
>>> +Arguments:
>>> +
>>> +- "path": the device's qtree path or unique ID (json-string)
>>>
>>>
>> This may be ambiguous.
>>
> Can your elaborate what precisely is ambiguous?
>
Can't the user choose the unique ID so that it aliases an unrelated
qtree path?
I prefer having mutually exclusive 'path' and 'ref' arguments.
>>> +- "full": report full state (json-bool, optional)
>>>
>>>
>> Is this needed for QMP? The client can always truncate it to any length.
>>
> The effect may not be needed for QMP, but I do need this channel from
> the command line to the monitor pretty-printer. I could just stick
> "full": json-bool back into the return dict, but that would look somehow
> strange IMO.
>
So we could disallow it as a QMP input, but allow it as an HMP input.
>>> +
>>> +Schema of returned object:
>>> +
>>> +{ "device": json-string, "id": json-string, "fields" : [
>>> field-objects ] }
>>> +
>>> +The field object array may be empty, otherwise it consists of
>>> +
>>> +{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
>>> +
>>> +"size" describes the real number of bytes required for a binary
>>> representation
>>> +of a single field element in the array. The actually transfered
>>> amount may be
>>> +smaller unless a full dump was requested.
>>>
>>>
>> This converts the entire qdev tree into an undocumented stable protocol
>> (the qdev paths were already in this state I believe). This really
>> worries me.
>>
> Being primarily a debugging tool, device_show exports the entire
> (qdev'ified) vmstates via QMP. Unlike the migration protocol, it does
> not provide something like backward compatibility.
Should be explicitly documented. All QMP commands should be backwards
and forwards compatible unless noted.
> This would be
> overkill for the intended purpose (though someone may find a different
> use case one day).
>
Even for simply showing things, a GUI may depend on the presence of
certain fields. If we document that the fields may change, a correctly
written GUI can fall back to a simpler display.
> I think we have the following options:
> - disable device_show via QMP, limit it to the monitor console
> - declare its output inherently unstable, maybe at least adding the
> vmstate version to each device so that potential QMP consumers notice
> that they may have to update their tools or switch to a different
> processing function
>
> Given that vmstate annotations will most probably require some work on
> the output structure (and I don't have a QMP use case ATM anyway), I
> would be fine with the first option for now. Still, I don't think we
> will ever get beyond the second option because this service is tight to
> some internals of QEMU we don't want to freeze.
>
I agree. This feature is very useful as a debugging aid, and as I don't
think we'll have debugging GUIs any time soon, it's better to defer the
problem until we really need to solve it.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder
2010-05-23 7:55 ` Jan Kiszka
@ 2010-05-23 8:48 ` Avi Kivity
2010-05-23 10:04 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Avi Kivity @ 2010-05-23 8:48 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Blue Swirl, Luiz Capitulino
On 05/23/2010 10:55 AM, Jan Kiszka wrote:
>>> +/*
>>> + * Convert string 'src' of length 'srclen' from base64 to binary form,
>>> + * saving the result in 'dest'. The size of the destination buffer must be at
>>> + * least srclen * 3 / 4.
>>> + *
>>> + * Returns 0 on success, -1 on conversion error.
>>> + */
>>> +int base64_decode(const char *src, size_t srclen, void *dest)
>>>
>> I think dest should be char *, like all the functions where dest is passed to.
>>
> The output may but need not be a string, it's binary data. And to avoid
> needless warnings about signedness mismatches if unsigned char or
> uint8_t buffers are passed, I chose void *.
>
I think qemu is pretty consistent in using uint8_t for binary, and void
* is a little dangerous as it allows passing any kind of data (anything
above a byte is subject to endianness issues for example).
But I don't feel strongly about this.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-23 8:44 ` Avi Kivity
@ 2010-05-23 10:03 ` Jan Kiszka
2010-05-23 10:42 ` Avi Kivity
2010-05-29 8:00 ` Markus Armbruster
1 sibling, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-23 10:03 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 4313 bytes --]
Avi Kivity wrote:
> On 05/23/2010 10:57 AM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>
>>> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>>>
>>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>>
>>>> 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. A new qerror is introduced to signal a missing vmstate. And
>>>> it comes with documentation.
>>>>
>>>> +
>>>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>>>> unless
>>>> +a full dump is requested.
>>>> +
>>>> +Arguments:
>>>> +
>>>> +- "path": the device's qtree path or unique ID (json-string)
>>>>
>>>>
>>> This may be ambiguous.
>>>
>> Can your elaborate what precisely is ambiguous?
>>
>
> Can't the user choose the unique ID so that it aliases an unrelated
> qtree path?
True. I'll swap the search order and document this. Qtree paths should
always rule.
>
> I prefer having mutually exclusive 'path' and 'ref' arguments.
That would be unhandy.
>
>>>> +- "full": report full state (json-bool, optional)
>>>>
>>>>
>>> Is this needed for QMP? The client can always truncate it to any
>>> length.
>>>
>> The effect may not be needed for QMP, but I do need this channel from
>> the command line to the monitor pretty-printer. I could just stick
>> "full": json-bool back into the return dict, but that would look somehow
>> strange IMO.
>>
>
> So we could disallow it as a QMP input, but allow it as an HMP input.
>
>>>> +
>>>> +Schema of returned object:
>>>> +
>>>> +{ "device": json-string, "id": json-string, "fields" : [
>>>> field-objects ] }
>>>> +
>>>> +The field object array may be empty, otherwise it consists of
>>>> +
>>>> +{ "name": json-string, "size": json-int, "elems": [ element-objects
>>>> ] }
>>>> +
>>>> +"size" describes the real number of bytes required for a binary
>>>> representation
>>>> +of a single field element in the array. The actually transfered
>>>> amount may be
>>>> +smaller unless a full dump was requested.
>>>>
>>>>
>>> This converts the entire qdev tree into an undocumented stable protocol
>>> (the qdev paths were already in this state I believe). This really
>>> worries me.
>>>
>> Being primarily a debugging tool, device_show exports the entire
>> (qdev'ified) vmstates via QMP. Unlike the migration protocol, it does
>> not provide something like backward compatibility.
>
> Should be explicitly documented. All QMP commands should be backwards
> and forwards compatible unless noted.
>
>> This would be
>> overkill for the intended purpose (though someone may find a different
>> use case one day).
>>
>
> Even for simply showing things, a GUI may depend on the presence of
> certain fields. If we document that the fields may change, a correctly
> written GUI can fall back to a simpler display.
>
>> I think we have the following options:
>> - disable device_show via QMP, limit it to the monitor console
>> - declare its output inherently unstable, maybe at least adding the
>> vmstate version to each device so that potential QMP consumers notice
>> that they may have to update their tools or switch to a different
>> processing function
>>
>> Given that vmstate annotations will most probably require some work on
>> the output structure (and I don't have a QMP use case ATM anyway), I
>> would be fine with the first option for now. Still, I don't think we
>> will ever get beyond the second option because this service is tight to
>> some internals of QEMU we don't want to freeze.
>>
>
> I agree. This feature is very useful as a debugging aid, and as I don't
> think we'll have debugging GUIs any time soon, it's better to defer the
> problem until we really need to solve it.
I introduced .user_only as a monitor command tag and applied it on
device_show. But I also added the vmstate version to the device output,
maybe already helpful for users. All this will come with v3.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder
2010-05-23 8:48 ` Avi Kivity
@ 2010-05-23 10:04 ` Jan Kiszka
0 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-23 10:04 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Blue Swirl, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1091 bytes --]
Avi Kivity wrote:
> On 05/23/2010 10:55 AM, Jan Kiszka wrote:
>>>> +/*
>>>> + * Convert string 'src' of length 'srclen' from base64 to binary form,
>>>> + * saving the result in 'dest'. The size of the destination buffer
>>>> must be at
>>>> + * least srclen * 3 / 4.
>>>> + *
>>>> + * Returns 0 on success, -1 on conversion error.
>>>> + */
>>>> +int base64_decode(const char *src, size_t srclen, void *dest)
>>>>
>>> I think dest should be char *, like all the functions where dest is
>>> passed to.
>>>
>> The output may but need not be a string, it's binary data. And to avoid
>> needless warnings about signedness mismatches if unsigned char or
>> uint8_t buffers are passed, I chose void *.
>>
>
> I think qemu is pretty consistent in using uint8_t for binary, and void
> * is a little dangerous as it allows passing any kind of data (anything
> above a byte is subject to endianness issues for example).
>
> But I don't feel strongly about this.
>
Let's go for consistency: I switched to uint8_t for the binary input/output.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-23 10:03 ` Jan Kiszka
@ 2010-05-23 10:42 ` Avi Kivity
0 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-05-23 10:42 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Luiz Capitulino
On 05/23/2010 01:03 PM, Jan Kiszka wrote:
>>>
>>> Can your elaborate what precisely is ambiguous?
>>>
>>>
>> Can't the user choose the unique ID so that it aliases an unrelated
>> qtree path?
>>
> True. I'll swap the search order and document this. Qtree paths should
> always rule.
>
Well, I guess the user could avoid ambiguity by avoiding /es.
>
>> I prefer having mutually exclusive 'path' and 'ref' arguments.
>>
> That would be unhandy.
>
Don't really see why.
>> I agree. This feature is very useful as a debugging aid, and as I don't
>> think we'll have debugging GUIs any time soon, it's better to defer the
>> problem until we really need to solve it.
>>
> I introduced .user_only as a monitor command tag and applied it on
> device_show. But I also added the vmstate version to the device output,
> maybe already helpful for users. All this will come with v3.
>
Thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-23 7:57 ` Jan Kiszka
2010-05-23 8:44 ` Avi Kivity
@ 2010-05-24 12:51 ` Luiz Capitulino
2010-05-24 20:22 ` Anthony Liguori
1 sibling, 1 reply; 53+ messages in thread
From: Luiz Capitulino @ 2010-05-24 12:51 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, Markus Armbruster,
qemu-devel, Avi Kivity
On Sun, 23 May 2010 09:57:43 +0200
Jan Kiszka <jan.kiszka@web.de> wrote:
> Avi Kivity wrote:
[...]
> >
> >> +- "full": report full state (json-bool, optional)
> >>
> >
> > Is this needed for QMP? The client can always truncate it to any length.
>
> The effect may not be needed for QMP, but I do need this channel from
> the command line to the monitor pretty-printer. I could just stick
> "full": json-bool back into the return dict, but that would look somehow
> strange IMO.
We could have a form of optional key which is only present internally,
we have that async commands.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 12:51 ` Luiz Capitulino
@ 2010-05-24 20:22 ` Anthony Liguori
0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-05-24 20:22 UTC (permalink / raw)
To: Luiz Capitulino
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Jan Kiszka, Avi Kivity
On 05/24/2010 07:51 AM, Luiz Capitulino wrote:
> On Sun, 23 May 2010 09:57:43 +0200
> Jan Kiszka<jan.kiszka@web.de> wrote:
>
>
>> Avi Kivity wrote:
>>
> [...]
>
>
>>>
>>>> +- "full": report full state (json-bool, optional)
>>>>
>>>>
>>> Is this needed for QMP? The client can always truncate it to any length.
>>>
>> The effect may not be needed for QMP, but I do need this channel from
>> the command line to the monitor pretty-printer. I could just stick
>> "full": json-bool back into the return dict, but that would look somehow
>> strange IMO.
>>
> We could have a form of optional key which is only present internally,
> we have that async commands.
>
I'd rather see two separate commands with an internal mechanism to make
direct QMP calls. The human monitor can be implemented in terms of the
QMP call but it should be in terms of the human monitor invoking the QMP
command such that it can munge the output as it sees appropriate.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-22 18:55 ` [Qemu-devel] " Avi Kivity
2010-05-23 7:57 ` Jan Kiszka
@ 2010-05-24 20:22 ` Anthony Liguori
2010-05-24 20:35 ` Jan Kiszka
2010-05-25 7:23 ` Avi Kivity
1 sibling, 2 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-05-24 20:22 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Jan Kiszka, Luiz Capitulino
On 05/22/2010 01:55 PM, Avi Kivity wrote:
> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> 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. A new qerror is introduced to signal a missing vmstate. And
>> it comes with documentation.
>>
>> +
>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>> unless
>> +a full dump is requested.
>> +
>> +Arguments:
>> +
>> +- "path": the device's qtree path or unique ID (json-string)
>
> This may be ambiguous.
>
>> +- "full": report full state (json-bool, optional)
>
> Is this needed for QMP? The client can always truncate it to any length.
>
>> +
>> +Schema of returned object:
>> +
>> +{ "device": json-string, "id": json-string, "fields" : [
>> field-objects ] }
>> +
>> +The field object array may be empty, otherwise it consists of
>> +
>> +{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
>> +
>> +"size" describes the real number of bytes required for a binary
>> representation
>> +of a single field element in the array. The actually transfered
>> amount may be
>> +smaller unless a full dump was requested.
>
> This converts the entire qdev tree into an undocumented stable
> protocol (the qdev paths were already in this state I believe). This
> really worries me.
N.B. the association with qdev is only in identifying the device. The
contents of the device's state are not part of qdev but rather part of
vmstate. vmstate is something that we already guarantee to be stable
since that's required for live migration compatibility.
I don't think that qdev device names and paths are something we have to
worry much about changing over time since they reflect logical bus
layout. They should remain static provided the devices remain static.
The qdev properties are a different matter entirely. A command like
'info qdm' would be potentially difficult to support as part of QMP but
the proposed command's output is actually already part of a backward
compatible interface (vmstate).
Regards,
Anthony Liguori
>> +
>> +The element object array may be empty, otherwise it can contain
>> +
>> +- json-int objects
>> +- QMP buffer objects
>> +- field objects
>> +- arrays of json-ints, QMP buffers, or field objects
>> +
>> +Example:
>> +
>> +-> { "execute": "device_show", "arguments": { "path": "isa.0/i8042"
>> } }
>> +<- { "return": { "device": "i8042", "id": "", "fields":
>> + [ { "name": "kbd", "size": 4, "elems":
>> + [ { "name": "write_cmd", "size": 1, "elems":
>> [0] },
>> + { "name": "status", "size": 1, "elems": [25] },
>> + { "name": "mode", "size": 1, "elems": [3] },
>> + { "name": "pending", "size": 1, "elems": [1] }
>> + ] }
>> + ]
>> + }
>> + }
>> +
>> +EQMP
>
> Looks good. I am only worried about long term stability and
> documentation.
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 20:22 ` Anthony Liguori
@ 2010-05-24 20:35 ` Jan Kiszka
2010-05-24 21:49 ` Anthony Liguori
2010-05-25 7:23 ` Avi Kivity
1 sibling, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-24 20:35 UTC (permalink / raw)
To: Anthony Liguori
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Avi Kivity, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 2387 bytes --]
Anthony Liguori wrote:
> On 05/22/2010 01:55 PM, Avi Kivity wrote:
>> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>
>>> 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. A new qerror is introduced to signal a missing vmstate. And
>>> it comes with documentation.
>>>
>>> +
>>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>>> unless
>>> +a full dump is requested.
>>> +
>>> +Arguments:
>>> +
>>> +- "path": the device's qtree path or unique ID (json-string)
>>
>> This may be ambiguous.
>>
>>> +- "full": report full state (json-bool, optional)
>>
>> Is this needed for QMP? The client can always truncate it to any length.
>>
>>> +
>>> +Schema of returned object:
>>> +
>>> +{ "device": json-string, "id": json-string, "fields" : [
>>> field-objects ] }
>>> +
>>> +The field object array may be empty, otherwise it consists of
>>> +
>>> +{ "name": json-string, "size": json-int, "elems": [ element-objects ] }
>>> +
>>> +"size" describes the real number of bytes required for a binary
>>> representation
>>> +of a single field element in the array. The actually transfered
>>> amount may be
>>> +smaller unless a full dump was requested.
>>
>> This converts the entire qdev tree into an undocumented stable
>> protocol (the qdev paths were already in this state I believe). This
>> really worries me.
>
> N.B. the association with qdev is only in identifying the device. The
> contents of the device's state are not part of qdev but rather part of
> vmstate. vmstate is something that we already guarantee to be stable
> since that's required for live migration compatibility.
In contrast to save/loadvm, device_show does not provide a
backward-compatible output. Not only field names can change (as a result
of internal refactoring), fields may even disappear, new ones may pop
up. vmstate makes this transparent for reading of old states, but not
for visualization.
That said, I see no use case yet that would justify the effort for
making state dumps as stable as vmstates.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 20:35 ` Jan Kiszka
@ 2010-05-24 21:49 ` Anthony Liguori
2010-05-24 22:12 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Anthony Liguori @ 2010-05-24 21:49 UTC (permalink / raw)
To: Jan Kiszka
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Avi Kivity, Markus Armbruster
On 05/24/2010 03:35 PM, Jan Kiszka wrote:
> In contrast to save/loadvm, device_show does not provide a
> backward-compatible output. Not only field names can change (as a result
> of internal refactoring),
Field names could change, but that seems unlikely and unnecessary.
> fields may even disappear,
That would break live migration. The output of device_show when
specifying -M pc-0.13 should always be the same. If it's not, I believe
that's a bug.
The output of device_show across multiple qemu versions without
specifying a -M could certainly be different but that's to be expected
since you have a different hardware model.
> new ones may pop
> up. vmstate makes this transparent for reading of old states, but not
> for visualization.
>
> That said, I see no use case yet that would justify the effort for
> making state dumps as stable as vmstates.
>
They already are. It's just a matter of whether we declare it as such
AFAICT.
Regards,
Anthony Liguori
> Jan
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 21:49 ` Anthony Liguori
@ 2010-05-24 22:12 ` Jan Kiszka
2010-05-24 22:27 ` Anthony Liguori
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-24 22:12 UTC (permalink / raw)
To: Anthony Liguori
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Avi Kivity, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 1147 bytes --]
Anthony Liguori wrote:
> On 05/24/2010 03:35 PM, Jan Kiszka wrote:
>> In contrast to save/loadvm, device_show does not provide a
>> backward-compatible output. Not only field names can change (as a result
>> of internal refactoring),
>
> Field names could change, but that seems unlikely and unnecessary.
Actually, this may happen to improve the output format for device_show,
either via renaming the fields or annotating it (once we have the latter
mechanism).
>
>> fields may even disappear,
>
> That would break live migration.
Nope. We can (and I think we did before) perfectly read some field up to
version X and convert it into the new format but skip that field for X+1
upward.
> The output of device_show when
> specifying -M pc-0.13 should always be the same. If it's not, I believe
> that's a bug.
>
> The output of device_show across multiple qemu versions without
> specifying a -M could certainly be different but that's to be expected
> since you have a different hardware model.
It will be different as soon as the version_id of a dumped vmstate
changes, even independent of -M.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 22:12 ` Jan Kiszka
@ 2010-05-24 22:27 ` Anthony Liguori
0 siblings, 0 replies; 53+ messages in thread
From: Anthony Liguori @ 2010-05-24 22:27 UTC (permalink / raw)
To: Jan Kiszka
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Avi Kivity, Markus Armbruster
On 05/24/2010 05:12 PM, Jan Kiszka wrote:
> Anthony Liguori wrote:
>
>> On 05/24/2010 03:35 PM, Jan Kiszka wrote:
>>
>>> In contrast to save/loadvm, device_show does not provide a
>>> backward-compatible output. Not only field names can change (as a result
>>> of internal refactoring),
>>>
>> Field names could change, but that seems unlikely and unnecessary.
>>
> Actually, this may happen to improve the output format for device_show,
> either via renaming the fields or annotating it (once we have the latter
> mechanism).
I don't think it's a huge win vs. stability.
>> The output of device_show when
>> specifying -M pc-0.13 should always be the same. If it's not, I believe
>> that's a bug.
>>
>> The output of device_show across multiple qemu versions without
>> specifying a -M could certainly be different but that's to be expected
>> since you have a different hardware model.
>>
> It will be different as soon as the version_id of a dumped vmstate
> changes, even independent of -M.
>
But QMP stability is only guaranteed for releases and -M pc-0.12 should
(in a perfect world) always dump out the same versions regardless of
whether it's qemu-0.12, qemu-0.13, etc.
Regards,
Anthony Liguori
> Jan
>
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-24 20:22 ` Anthony Liguori
2010-05-24 20:35 ` Jan Kiszka
@ 2010-05-25 7:23 ` Avi Kivity
2010-05-25 13:03 ` Anthony Liguori
1 sibling, 1 reply; 53+ messages in thread
From: Avi Kivity @ 2010-05-25 7:23 UTC (permalink / raw)
To: Anthony Liguori
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Markus Armbruster, Jan Kiszka, Luiz Capitulino
On 05/24/2010 11:22 PM, Anthony Liguori wrote:
>> This converts the entire qdev tree into an undocumented stable
>> protocol (the qdev paths were already in this state I believe). This
>> really worries me.
>
>
> N.B. the association with qdev is only in identifying the device. The
> contents of the device's state are not part of qdev but rather part of
> vmstate. vmstate is something that we already guarantee to be stable
> since that's required for live migration compatibility.
That removes out ability to deprecate older vmstate as time passes. Not
a blocker but something to consider.
> I don't think that qdev device names and paths are something we have
> to worry much about changing over time since they reflect logical bus
> layout. They should remain static provided the devices remain static.
Modulo mistakes. We already saw one (lack of pci domains). To reduce
the possibility of mistakes, we need reviewable documentation.
Note sysfs had similar assumptions and problems.
> The qdev properties are a different matter entirely. A command like
> 'info qdm' would be potentially difficult to support as part of QMP
> but the proposed command's output is actually already part of a
> backward compatible interface (vmstate).
That's all good. But documentation is critical for this. Not only to
improve quality, but also so that tool authors would have something to
code against instead of trial and error (which invariably misses some
corner cases).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-25 7:23 ` Avi Kivity
@ 2010-05-25 13:03 ` Anthony Liguori
2010-05-25 13:19 ` Avi Kivity
0 siblings, 1 reply; 53+ messages in thread
From: Anthony Liguori @ 2010-05-25 13:03 UTC (permalink / raw)
To: Avi Kivity
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Jan Kiszka, Markus Armbruster
On 05/25/2010 02:23 AM, Avi Kivity wrote:
> On 05/24/2010 11:22 PM, Anthony Liguori wrote:
>>> This converts the entire qdev tree into an undocumented stable
>>> protocol (the qdev paths were already in this state I believe).
>>> This really worries me.
>>
>>
>> N.B. the association with qdev is only in identifying the device.
>> The contents of the device's state are not part of qdev but rather
>> part of vmstate. vmstate is something that we already guarantee to
>> be stable since that's required for live migration compatibility.
>
> That removes out ability to deprecate older vmstate as time passes.
> Not a blocker but something to consider.
>
>> I don't think that qdev device names and paths are something we have
>> to worry much about changing over time since they reflect logical bus
>> layout. They should remain static provided the devices remain static.
>
> Modulo mistakes. We already saw one (lack of pci domains). To reduce
> the possibility of mistakes, we need reviewable documentation.
pci domains was only a mistake as a nice-to-have. We can add pci
domains in a backwards compatible way.
The arguments you're making about the importance of backwards
compatibility and what's needed to strongly guarantee it are equally
applicable to the live migration protocol. We really do need to
formally document the live migration protocol in such a way that it's
reviewable if we hope to truly make it compatible across versions.
Regards,
Anthony Liguori
> Note sysfs had similar assumptions and problems.
>
>> The qdev properties are a different matter entirely. A command like
>> 'info qdm' would be potentially difficult to support as part of QMP
>> but the proposed command's output is actually already part of a
>> backward compatible interface (vmstate).
>
> That's all good. But documentation is critical for this. Not only to
> improve quality, but also so that tool authors would have something to
> code against instead of trial and error (which invariably misses some
> corner cases).
>
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-25 13:03 ` Anthony Liguori
@ 2010-05-25 13:19 ` Avi Kivity
2010-05-25 13:31 ` Anthony Liguori
0 siblings, 1 reply; 53+ messages in thread
From: Avi Kivity @ 2010-05-25 13:19 UTC (permalink / raw)
To: Anthony Liguori
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Jan Kiszka, Markus Armbruster
On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>
>>> I don't think that qdev device names and paths are something we have
>>> to worry much about changing over time since they reflect logical
>>> bus layout. They should remain static provided the devices remain
>>> static.
>>
>> Modulo mistakes. We already saw one (lack of pci domains). To
>> reduce the possibility of mistakes, we need reviewable documentation.
>
>
> pci domains was only a mistake as a nice-to-have. We can add pci
> domains in a backwards compatible way.
It adds a new level to the qdev tree. Of course we can hide the new
level for older clients, and newer clients can drop the level for older
qemus, but it will be oh-so-painful.
>
> The arguments you're making about the importance of backwards
> compatibility and what's needed to strongly guarantee it are equally
> applicable to the live migration protocol. We really do need to
> formally document the live migration protocol in such a way that it's
> reviewable if we hope to truly make it compatible across versions.
Mostly agreed. I think live migration has a faster/easier deprecation
schedule (easier not to support migration from 0.n-k to 0.n than to
remove qmp support for a feature introduced in 0.n-k when releasing
0.n). But that's a minor concern, improving our externally visible
interface documentation is a good thing and badly needed.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-25 13:19 ` Avi Kivity
@ 2010-05-25 13:31 ` Anthony Liguori
2010-05-25 13:41 ` Avi Kivity
0 siblings, 1 reply; 53+ messages in thread
From: Anthony Liguori @ 2010-05-25 13:31 UTC (permalink / raw)
To: Avi Kivity
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Anthony Liguori, Jan Kiszka, Markus Armbruster
On 05/25/2010 08:19 AM, Avi Kivity wrote:
> On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>>
>>>> I don't think that qdev device names and paths are something we
>>>> have to worry much about changing over time since they reflect
>>>> logical bus layout. They should remain static provided the devices
>>>> remain static.
>>>
>>> Modulo mistakes. We already saw one (lack of pci domains). To
>>> reduce the possibility of mistakes, we need reviewable documentation.
>>
>>
>> pci domains was only a mistake as a nice-to-have. We can add pci
>> domains in a backwards compatible way.
>
> It adds a new level to the qdev tree.
The tree is not organized like that today. IOW, the PCI hierarchy is
not reflected in the qdev hierarchy. All PCI devices (regardless of
whether they're a function or a full slot) simply sit below the PCI bus.
>>
>> The arguments you're making about the importance of backwards
>> compatibility and what's needed to strongly guarantee it are equally
>> applicable to the live migration protocol. We really do need to
>> formally document the live migration protocol in such a way that it's
>> reviewable if we hope to truly make it compatible across versions.
>
> Mostly agreed. I think live migration has a faster/easier deprecation
> schedule (easier not to support migration from 0.n-k to 0.n than to
> remove qmp support for a feature introduced in 0.n-k when releasing
> 0.n). But that's a minor concern, improving our externally visible
> interface documentation is a good thing and badly needed.
>
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-25 13:31 ` Anthony Liguori
@ 2010-05-25 13:41 ` Avi Kivity
0 siblings, 0 replies; 53+ messages in thread
From: Avi Kivity @ 2010-05-25 13:41 UTC (permalink / raw)
To: Anthony Liguori
Cc: Juan Quintela, Jan Kiszka, qemu-devel, Luiz Capitulino,
Anthony Liguori, Jan Kiszka, Markus Armbruster
On 05/25/2010 04:31 PM, Anthony Liguori wrote:
> On 05/25/2010 08:19 AM, Avi Kivity wrote:
>> On 05/25/2010 04:03 PM, Anthony Liguori wrote:
>>>>
>>>>> I don't think that qdev device names and paths are something we
>>>>> have to worry much about changing over time since they reflect
>>>>> logical bus layout. They should remain static provided the
>>>>> devices remain static.
>>>>
>>>> Modulo mistakes. We already saw one (lack of pci domains). To
>>>> reduce the possibility of mistakes, we need reviewable documentation.
>>>
>>>
>>> pci domains was only a mistake as a nice-to-have. We can add pci
>>> domains in a backwards compatible way.
>>
>> It adds a new level to the qdev tree.
>
> The tree is not organized like that today. IOW, the PCI hierarchy is
> not reflected in the qdev hierarchy. All PCI devices (regardless of
> whether they're a function or a full slot) simply sit below the PCI bus.
That's a bug IMO, but regardless, s/qdev tree/pci device component of
the qdev path/.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices Jan Kiszka
@ 2010-05-29 7:38 ` Markus Armbruster
2010-05-29 7:56 ` Jan Kiszka
2010-05-31 9:45 ` Gerd Hoffmann
0 siblings, 2 replies; 53+ messages in thread
From: Markus Armbruster @ 2010-05-29 7:38 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity, Gerd Hoffmann
[cc: kraxel]
Jan Kiszka <jan.kiszka@web.de> writes:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to
> make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as
> there is only one child bus per device.
We auto-root a path not starting with '/' via convention "first
component is ID then" (I like that). We auto-complete a path ending
with a device when we want a bus (a bit too clever). Auto-inserting bus
names in the middle is too clever by half!
Such cleverness can be convenient in the human monitor, but all it adds
to QMP is complexity.
I'm worried about possibly ambigous interpretation of paths. Would be
bad if a path could name different node after we add something to the
tree. I *think* your fine print makes that impossible, but it's not
exactly obvious. Heck, I could be wrong.
Wouldn't it be better to put the convenience into the interactive
completion function? That way we keep it out of QMP.
Subject is misleading, it's a feature, not a bug fix.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance'
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance' Jan Kiszka
@ 2010-05-29 7:50 ` Markus Armbruster
2010-05-29 8:09 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Markus Armbruster @ 2010-05-29 7:50 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity
Jan Kiszka <jan.kiszka@web.de> writes:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> Extend qbus_find_dev to allow addressing of devices without an unique id
> via an optional per-bus instance number. The new formats are
> 'driver.instance' and 'alias.instance'.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> docs/qdev-device-use.txt | 12 +++++++++++-
> hw/qdev.c | 23 ++++++++++++++++++-----
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
> index 9ac1fa1..5939481 100644
> --- a/docs/qdev-device-use.txt
> +++ b/docs/qdev-device-use.txt
> @@ -1,6 +1,6 @@
> = How to convert to -device & friends =
>
> -=== Specifying Bus and Address on Bus ===
> +=== Specifying Bus, Address on Bus, and Devices ===
>
> In qdev, each device has a parent bus. Some devices provide one or
> more buses for children. You can specify a device's parent bus with
> @@ -24,6 +24,16 @@ Furthermore, if a device only hosts a single bus, the bus name can be
> omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates
> /i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings.
>
> +Existing devices can be addressed either via a unique ID if it was
> +assigned during creation or via the device tree path:
> +
> +/full_bus_address/driver_name[.instance_number]
> + or
> +abbreviated_bus_address/driver_name[.instance_number]
> +
> +Example: /i440FX-pcihost/pci.0/e1000.2 addresses the second e1000
> +adapter on the bus 'pci.0'.
> +
> Note: the USB device address can't be controlled at this time.
"instance number" isn't defined in this document.
I understand the problem you're trying to solve; I've had it myself many
times. But is inventing an instance number the right solution?
The two e1000 devices already have a perfectly fine unique identifier on
their bus: their bus address. What about recognizing bus addresses in
paths? Requires a suitable restriction on device names and IDs to avoid
ambiguity, say "start with letter".
qdev currently doesn't abstract bus addresses, but that's fixable.
[...]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices
2010-05-29 7:38 ` Markus Armbruster
@ 2010-05-29 7:56 ` Jan Kiszka
2010-05-31 9:45 ` Gerd Hoffmann
1 sibling, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-29 7:56 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 1948 bytes --]
Markus Armbruster wrote:
> [cc: kraxel]
>
> Jan Kiszka <jan.kiszka@web.de> writes:
>
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to
>> make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as
>> there is only one child bus per device.
>
> We auto-root a path not starting with '/' via convention "first
> component is ID then" (I like that). We auto-complete a path ending
> with a device when we want a bus (a bit too clever). Auto-inserting bus
> names in the middle is too clever by half!
>
> Such cleverness can be convenient in the human monitor, but all it adds
> to QMP is complexity.
>
> I'm worried about possibly ambigous interpretation of paths. Would be
> bad if a path could name different node after we add something to the
> tree. I *think* your fine print makes that impossible, but it's not
> exactly obvious. Heck, I could be wrong.
For the above example, that would mean a bus called 'dev2' would have to
be added to dev1. If that makes sense is one question. The other is why
this should be more broken than the case that this happens to a bus
specified at the end of some path?
>
> Wouldn't it be better to put the convenience into the interactive
> completion function? That way we keep it out of QMP.
That would leave user interfaces over QMP broken behind, or at least
complicate their implementations as they would have to expand the qtree
path on their own to achieve consistency.
>
> Subject is misleading, it's a feature, not a bug fix.
Depends on the POV. For me it is a highly confusing inconsistency in the
way you can specify qtree paths. IMHO, we either have to drop this path
abbreviation or apply it to the whole path. As it's intention is
(according to my understanding) to ease the usage, an unintuitive
construction role is fairly counterproductive.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-23 8:44 ` Avi Kivity
2010-05-23 10:03 ` Jan Kiszka
@ 2010-05-29 8:00 ` Markus Armbruster
2010-05-29 8:14 ` Jan Kiszka
1 sibling, 1 reply; 53+ messages in thread
From: Markus Armbruster @ 2010-05-29 8:00 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Jan Kiszka
Avi Kivity <avi@redhat.com> writes:
> On 05/23/2010 10:57 AM, Jan Kiszka wrote:
>> Avi Kivity wrote:
>>
>>> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>>>
>>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>>
>>>> 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. A new qerror is introduced to signal a missing vmstate. And
>>>> it comes with documentation.
>>>>
>>>> +
>>>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>>>> unless
>>>> +a full dump is requested.
>>>> +
>>>> +Arguments:
>>>> +
>>>> +- "path": the device's qtree path or unique ID (json-string)
>>>>
>>>>
>>> This may be ambiguous.
>>>
>> Can your elaborate what precisely is ambiguous?
>>
>
> Can't the user choose the unique ID so that it aliases an unrelated
> qtree path?
>
> I prefer having mutually exclusive 'path' and 'ref' arguments.
Don't think that's necessary. If the string starts with '/', it's an
absolute path. Else, it's a relative path rooted at the node with the
ID equal to the first component.
Currently breaks down when IDs contain '/', but permitting that is a
bug. There may be more problems; the path lookup code is way too
clever.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance'
2010-05-29 7:50 ` Markus Armbruster
@ 2010-05-29 8:09 ` Jan Kiszka
2010-05-31 8:43 ` Markus Armbruster
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-29 8:09 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 2582 bytes --]
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@web.de> writes:
>
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Extend qbus_find_dev to allow addressing of devices without an unique id
>> via an optional per-bus instance number. The new formats are
>> 'driver.instance' and 'alias.instance'.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> docs/qdev-device-use.txt | 12 +++++++++++-
>> hw/qdev.c | 23 ++++++++++++++++++-----
>> 2 files changed, 29 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
>> index 9ac1fa1..5939481 100644
>> --- a/docs/qdev-device-use.txt
>> +++ b/docs/qdev-device-use.txt
>> @@ -1,6 +1,6 @@
>> = How to convert to -device & friends =
>>
>> -=== Specifying Bus and Address on Bus ===
>> +=== Specifying Bus, Address on Bus, and Devices ===
>>
>> In qdev, each device has a parent bus. Some devices provide one or
>> more buses for children. You can specify a device's parent bus with
>> @@ -24,6 +24,16 @@ Furthermore, if a device only hosts a single bus, the bus name can be
>> omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates
>> /i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings.
>>
>> +Existing devices can be addressed either via a unique ID if it was
>> +assigned during creation or via the device tree path:
>> +
>> +/full_bus_address/driver_name[.instance_number]
>> + or
>> +abbreviated_bus_address/driver_name[.instance_number]
>> +
>> +Example: /i440FX-pcihost/pci.0/e1000.2 addresses the second e1000
>> +adapter on the bus 'pci.0'.
>> +
>> Note: the USB device address can't be controlled at this time.
>
> "instance number" isn't defined in this document.
True, only implicitly via the example.
>
> I understand the problem you're trying to solve; I've had it myself many
> times. But is inventing an instance number the right solution?
>
> The two e1000 devices already have a perfectly fine unique identifier on
> their bus: their bus address. What about recognizing bus addresses in
> paths? Requires a suitable restriction on device names and IDs to avoid
> ambiguity, say "start with letter".
>
> qdev currently doesn't abstract bus addresses, but that's fixable.
You would also have to specify unique addressing scheme to those buses
that do not have it yet. E.g. what should be the address of a ISA bus
device? The base of its first ioport range? But if it does not have any
as it only injects ISA IRQs?
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-29 8:00 ` Markus Armbruster
@ 2010-05-29 8:14 ` Jan Kiszka
2010-05-30 8:26 ` Avi Kivity
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-29 8:14 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity
[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]
Markus Armbruster wrote:
> Avi Kivity <avi@redhat.com> writes:
>
>> On 05/23/2010 10:57 AM, Jan Kiszka wrote:
>>> Avi Kivity wrote:
>>>
>>>> On 05/22/2010 11:18 AM, Jan Kiszka wrote:
>>>>
>>>>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>>>>
>>>>> 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. A new qerror is introduced to signal a missing vmstate. And
>>>>> it comes with documentation.
>>>>>
>>>>> +
>>>>> +Dump a snapshot of the device state. Buffers are cut after 16 bytes
>>>>> unless
>>>>> +a full dump is requested.
>>>>> +
>>>>> +Arguments:
>>>>> +
>>>>> +- "path": the device's qtree path or unique ID (json-string)
>>>>>
>>>>>
>>>> This may be ambiguous.
>>>>
>>> Can your elaborate what precisely is ambiguous?
>>>
>> Can't the user choose the unique ID so that it aliases an unrelated
>> qtree path?
>>
>> I prefer having mutually exclusive 'path' and 'ref' arguments.
>
> Don't think that's necessary. If the string starts with '/', it's an
> absolute path. Else, it's a relative path rooted at the node with the
> ID equal to the first component.
'pci.0' can be both a (badly chosen) ID as well as an abbreviated qtree
path.
>
> Currently breaks down when IDs contain '/', but permitting that is a
> bug. There may be more problems; the path lookup code is way too
> clever.
Indeed. Less can sometimes be more. My impression is that some of the
cleverness was motivated by lacking qtree completion for the HMP.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-29 8:14 ` Jan Kiszka
@ 2010-05-30 8:26 ` Avi Kivity
2010-05-30 12:36 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Avi Kivity @ 2010-05-30 8:26 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Markus Armbruster
On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>> Currently breaks down when IDs contain '/', but permitting that is a
>> bug. There may be more problems; the path lookup code is way too
>> clever.
>>
> Indeed. Less can sometimes be more. My impression is that some of the
> cleverness was motivated by lacking qtree completion for the HMP.
>
Can we disable abbreviations for QMP and only allow them for HMP?
We can support that by adding a hidden argument to commands specifying
whether the input comes from a human or machine. Abbreviations are
dangerous if they become ambiguous; a human can recover while a machine
can't.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-30 8:26 ` Avi Kivity
@ 2010-05-30 12:36 ` Jan Kiszka
2010-05-31 8:46 ` Markus Armbruster
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-30 12:36 UTC (permalink / raw)
To: Avi Kivity
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Markus Armbruster
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]
Avi Kivity wrote:
> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>> Currently breaks down when IDs contain '/', but permitting that is a
>>> bug. There may be more problems; the path lookup code is way too
>>> clever.
>>>
>> Indeed. Less can sometimes be more. My impression is that some of the
>> cleverness was motivated by lacking qtree completion for the HMP.
>>
>
> Can we disable abbreviations for QMP and only allow them for HMP?
>
> We can support that by adding a hidden argument to commands specifying
> whether the input comes from a human or machine. Abbreviations are
> dangerous if they become ambiguous; a human can recover while a machine
> can't.
>
Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
schemes. Therefore, I'm more and more in favor of [1]. We just need to
add command line completion for option values, something that would be
beneficial for 'drive_add file=...' as well.
Jan
[1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance'
2010-05-29 8:09 ` Jan Kiszka
@ 2010-05-31 8:43 ` Markus Armbruster
0 siblings, 0 replies; 53+ messages in thread
From: Markus Armbruster @ 2010-05-31 8:43 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity
Jan Kiszka <jan.kiszka@web.de> writes:
> Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@web.de> writes:
>>
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Extend qbus_find_dev to allow addressing of devices without an unique id
>>> via an optional per-bus instance number. The new formats are
>>> 'driver.instance' and 'alias.instance'.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> docs/qdev-device-use.txt | 12 +++++++++++-
>>> hw/qdev.c | 23 ++++++++++++++++++-----
>>> 2 files changed, 29 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt
>>> index 9ac1fa1..5939481 100644
>>> --- a/docs/qdev-device-use.txt
>>> +++ b/docs/qdev-device-use.txt
>>> @@ -1,6 +1,6 @@
>>> = How to convert to -device & friends =
>>>
>>> -=== Specifying Bus and Address on Bus ===
>>> +=== Specifying Bus, Address on Bus, and Devices ===
>>>
>>> In qdev, each device has a parent bus. Some devices provide one or
>>> more buses for children. You can specify a device's parent bus with
>>> @@ -24,6 +24,16 @@ Furthermore, if a device only hosts a single bus, the bus name can be
>>> omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates
>>> /i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings.
>>>
>>> +Existing devices can be addressed either via a unique ID if it was
>>> +assigned during creation or via the device tree path:
>>> +
>>> +/full_bus_address/driver_name[.instance_number]
>>> + or
>>> +abbreviated_bus_address/driver_name[.instance_number]
>>> +
>>> +Example: /i440FX-pcihost/pci.0/e1000.2 addresses the second e1000
>>> +adapter on the bus 'pci.0'.
>>> +
>>> Note: the USB device address can't be controlled at this time.
>>
>> "instance number" isn't defined in this document.
>
> True, only implicitly via the example.
>
>>
>> I understand the problem you're trying to solve; I've had it myself many
>> times. But is inventing an instance number the right solution?
>>
>> The two e1000 devices already have a perfectly fine unique identifier on
>> their bus: their bus address. What about recognizing bus addresses in
>> paths? Requires a suitable restriction on device names and IDs to avoid
>> ambiguity, say "start with letter".
>>
>> qdev currently doesn't abstract bus addresses, but that's fixable.
>
> You would also have to specify unique addressing scheme to those buses
> that do not have it yet. E.g. what should be the address of a ISA bus
> device? The base of its first ioport range? But if it does not have any
> as it only injects ISA IRQs?
Hmm, instance numbers are the lesser evil in this case.
Define them properly, and make info qtree show them, and we'll be okay,
I guess.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-30 12:36 ` Jan Kiszka
@ 2010-05-31 8:46 ` Markus Armbruster
2010-05-31 8:58 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Markus Armbruster @ 2010-05-31 8:46 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Avi Kivity
Jan Kiszka <jan.kiszka@web.de> writes:
> Avi Kivity wrote:
>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>> bug. There may be more problems; the path lookup code is way too
>>>> clever.
>>>>
>>> Indeed. Less can sometimes be more. My impression is that some of the
>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>
>>
>> Can we disable abbreviations for QMP and only allow them for HMP?
>>
>> We can support that by adding a hidden argument to commands specifying
>> whether the input comes from a human or machine. Abbreviations are
>> dangerous if they become ambiguous; a human can recover while a machine
>> can't.
>>
>
> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
> schemes. Therefore, I'm more and more in favor of [1]. We just need to
> add command line completion for option values, something that would be
> beneficial for 'drive_add file=...' as well.
>
> Jan
>
> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
[1] = abolish the clever abbreviations in path lookup. I agree we
should do that.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-31 8:46 ` Markus Armbruster
@ 2010-05-31 8:58 ` Jan Kiszka
2010-05-31 11:07 ` Markus Armbruster
0 siblings, 1 reply; 53+ messages in thread
From: Jan Kiszka @ 2010-05-31 8:58 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, qemu-devel, Luiz Capitulino,
Jan Kiszka, Avi Kivity
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@web.de> writes:
>
>> Avi Kivity wrote:
>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>> bug. There may be more problems; the path lookup code is way too
>>>>> clever.
>>>>>
>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>
>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>
>>> We can support that by adding a hidden argument to commands specifying
>>> whether the input comes from a human or machine. Abbreviations are
>>> dangerous if they become ambiguous; a human can recover while a machine
>>> can't.
>>>
>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>> add command line completion for option values, something that would be
>> beneficial for 'drive_add file=...' as well.
>>
>> Jan
>>
>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
>
> [1] = abolish the clever abbreviations in path lookup. I agree we
> should do that.
Fine. No concerns regarding overlaying IDs and path specifications as well?
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices
2010-05-29 7:38 ` Markus Armbruster
2010-05-29 7:56 ` Jan Kiszka
@ 2010-05-31 9:45 ` Gerd Hoffmann
1 sibling, 0 replies; 53+ messages in thread
From: Gerd Hoffmann @ 2010-05-31 9:45 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, Jan Kiszka, qemu-devel,
Luiz Capitulino, Jan Kiszka, Avi Kivity
On 05/29/10 09:38, Markus Armbruster wrote:
> [cc: kraxel]
>
> Jan Kiszka<jan.kiszka@web.de> writes:
>
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to
>> make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as
>> there is only one child bus per device.
>
> We auto-root a path not starting with '/' via convention "first
> component is ID then" (I like that). We auto-complete a path ending
> with a device when we want a bus (a bit too clever).
... only if there is a single child bus only (i.e. it isn't ambiguous).
Which is the common case though.
> Auto-inserting bus
> names in the middle is too clever by half!
Yea. I have to agree here. It can only work reliably if you make sure
all your devices have unique ids. In which case there is no need to
reference dev1 at all, you can just use the "first component is ID then"
mechanism for dev2.
cheers,
Gerd
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-31 8:58 ` Jan Kiszka
@ 2010-05-31 11:07 ` Markus Armbruster
2010-05-31 11:11 ` Jan Kiszka
0 siblings, 1 reply; 53+ messages in thread
From: Markus Armbruster @ 2010-05-31 11:07 UTC (permalink / raw)
To: Jan Kiszka
Cc: Anthony Liguori, Juan Quintela, qemu-devel, Luiz Capitulino,
Jan Kiszka, Avi Kivity
Jan Kiszka <jan.kiszka@siemens.com> writes:
> Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@web.de> writes:
>>
>>> Avi Kivity wrote:
>>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>>> bug. There may be more problems; the path lookup code is way too
>>>>>> clever.
>>>>>>
>>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>>
>>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>>
>>>> We can support that by adding a hidden argument to commands specifying
>>>> whether the input comes from a human or machine. Abbreviations are
>>>> dangerous if they become ambiguous; a human can recover while a machine
>>>> can't.
>>>>
>>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>>> add command line completion for option values, something that would be
>>> beneficial for 'drive_add file=...' as well.
>>>
>>> Jan
>>>
>>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
>>
>> [1] = abolish the clever abbreviations in path lookup. I agree we
>> should do that.
>
> Fine. No concerns regarding overlaying IDs and path specifications as well?
I'm fine with that.
Calling the overlayed argument "id" is not so nice. We got a bunch of
other not-so-nice names in QMP, maybe we'll have a flag day to clean
them all up.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: [Qemu-devel] Re: [PATCH v2 12/15] monitor: Add basic device state visualization
2010-05-31 11:07 ` Markus Armbruster
@ 2010-05-31 11:11 ` Jan Kiszka
0 siblings, 0 replies; 53+ messages in thread
From: Jan Kiszka @ 2010-05-31 11:11 UTC (permalink / raw)
To: Markus Armbruster
Cc: Anthony Liguori, Juan Quintela, qemu-devel, Luiz Capitulino,
Jan Kiszka, Avi Kivity
Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
>
>> Markus Armbruster wrote:
>>> Jan Kiszka <jan.kiszka@web.de> writes:
>>>
>>>> Avi Kivity wrote:
>>>>> On 05/29/2010 11:14 AM, Jan Kiszka wrote:
>>>>>>> Currently breaks down when IDs contain '/', but permitting that is a
>>>>>>> bug. There may be more problems; the path lookup code is way too
>>>>>>> clever.
>>>>>>>
>>>>>> Indeed. Less can sometimes be more. My impression is that some of the
>>>>>> cleverness was motivated by lacking qtree completion for the HMP.
>>>>>>
>>>>> Can we disable abbreviations for QMP and only allow them for HMP?
>>>>>
>>>>> We can support that by adding a hidden argument to commands specifying
>>>>> whether the input comes from a human or machine. Abbreviations are
>>>>> dangerous if they become ambiguous; a human can recover while a machine
>>>>> can't.
>>>>>
>>>> Both QMP _and_ HMP suffer from ambitious and inconsistent addressing
>>>> schemes. Therefore, I'm more and more in favor of [1]. We just need to
>>>> add command line completion for option values, something that would be
>>>> beneficial for 'drive_add file=...' as well.
>>>>
>>>> Jan
>>>>
>>>> [1] http://article.gmane.org/gmane.comp.emulators.qemu/72152
>>> [1] = abolish the clever abbreviations in path lookup. I agree we
>>> should do that.
>> Fine. No concerns regarding overlaying IDs and path specifications as well?
>
> I'm fine with that.
>
> Calling the overlayed argument "id" is not so nice. We got a bunch of
> other not-so-nice names in QMP, maybe we'll have a flag day to clean
> them all up.
For this case (device_del and device_show), I think we should simply
call it 'device'.
Jan
--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2010-05-31 11:11 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-22 8:17 [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 01/15] Add dependency of JSON unit tests on config-host.h Jan Kiszka
2010-05-22 8:17 ` [Qemu-devel] [PATCH v2 02/15] qdev: Fix scanning across single-bus devices Jan Kiszka
2010-05-29 7:38 ` Markus Armbruster
2010-05-29 7:56 ` Jan Kiszka
2010-05-31 9:45 ` Gerd Hoffmann
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 03/15] qdev: Allow device addressing via 'driver.instance' Jan Kiszka
2010-05-29 7:50 ` Markus Armbruster
2010-05-29 8:09 ` Jan Kiszka
2010-05-31 8:43 ` Markus Armbruster
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 04/15] qdev: Convert device and bus lists to QTAILQ Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 05/15] qdev: Allow device specification by qtree path for device_del Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 06/15] qdev: Push QMP mode checks into qbus_list_bus/dev Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 07/15] monitor: Add completion for qdev paths Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 08/15] Add base64 encoder/decoder Jan Kiszka
2010-05-22 13:59 ` Blue Swirl
2010-05-23 7:55 ` Jan Kiszka
2010-05-23 8:48 ` Avi Kivity
2010-05-23 10:04 ` Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 09/15] QMP: Reserve namespace for complex object classes Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 10/15] Add QBuffer Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 11/15] monitor: return length of printed string via monitor_[v]printf Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 12/15] monitor: Add basic device state visualization Jan Kiszka
2010-05-22 18:55 ` [Qemu-devel] " Avi Kivity
2010-05-23 7:57 ` Jan Kiszka
2010-05-23 8:44 ` Avi Kivity
2010-05-23 10:03 ` Jan Kiszka
2010-05-23 10:42 ` Avi Kivity
2010-05-29 8:00 ` Markus Armbruster
2010-05-29 8:14 ` Jan Kiszka
2010-05-30 8:26 ` Avi Kivity
2010-05-30 12:36 ` Jan Kiszka
2010-05-31 8:46 ` Markus Armbruster
2010-05-31 8:58 ` Jan Kiszka
2010-05-31 11:07 ` Markus Armbruster
2010-05-31 11:11 ` Jan Kiszka
2010-05-24 12:51 ` Luiz Capitulino
2010-05-24 20:22 ` Anthony Liguori
2010-05-24 20:22 ` Anthony Liguori
2010-05-24 20:35 ` Jan Kiszka
2010-05-24 21:49 ` Anthony Liguori
2010-05-24 22:12 ` Jan Kiszka
2010-05-24 22:27 ` Anthony Liguori
2010-05-25 7:23 ` Avi Kivity
2010-05-25 13:03 ` Anthony Liguori
2010-05-25 13:19 ` Avi Kivity
2010-05-25 13:31 ` Anthony Liguori
2010-05-25 13:41 ` Avi Kivity
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 13/15] QMP: Teach basic capability negotiation to python example Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 14/15] QMP: Fix python helper /wrt long return strings Jan Kiszka
2010-05-22 8:18 ` [Qemu-devel] [PATCH v2 15/15] QMP: Add support for buffer class to qmp python helper Jan Kiszka
2010-05-22 14:05 ` [Qemu-devel] [PATCH v2 00/15] Basic device state visualization Blue Swirl
2010-05-23 7:55 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.