qdev path reworks & basic device state visualization
v4 cover qdev path reworks & basic device state visualization
This is v4 of this series. Besides small fixes, it's main focus is on the groundwork for the visualization command: qdev path usability.
The significant changes are:
- drop many of the inconsistent or ambiguous qtree abbreviations
- devices can now be address via unique ID (no '/' allowed here) or an absolute path (must start with '/')
- buses remain addressable via their ambiguous local name (mostly to avoid libvirt breakages) or their absolute path
- the per-bus instance numbers introduced in this series are now printed consistently
- monitor command completion inside option lists is added (allows to expand "device_add ...,bus=" and "drive=")
- HMP commands can now have their own argument set (not yet urgently needed for device_show but likely already useful for other commands)
Git url as usual:
git://git.kiszka.org/qemu.git queues/device-show
Thanks once again for comments. Hope we can soon agree on a mergable version for 0.13.
Jan Kiszka (23):
qdev: Rework qtree path abbreviations
qdev: Restrict direct bus addressing via its name
qdev: Drop ID matching from qtree paths
qdev: Allow device addressing via 'driver.instance'
qdev: Convert device and bus lists to QTAILQ
qdev: Push QMP mode checks into qbus_list_bus/dev
qdev: Allow device specification by qtree path for device_del
qdev: Introduce qdev_iterate_recursive
monitor: Fix leakage during completion processing
monitor: Fix command completion vs. boolean switches
monitor: Add completion support for option lists
monitor: Add completion for qdev paths
monitor: Allow to specify HMP-specifc command arguments
monitor: return length of printed string via monitor_[v]printf
monitor: Establish cmd flags and convert the async tag
monitor: Allow to exclude commands from QMP
Add base64 encoder/decoder
QMP: Reserve namespace for complex object classes
QMP: Add QBuffer
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 | 19 ++ check-qbuffer.c | 172 ++++++++++++++++ configure | 2 +- docs/qdev-device-use.txt | 13 ++- hw/acpi_piix4.c | 2 +- hw/hw.h | 2 + hw/i2c.c | 2 +- hw/pci-hotplug.c | 2 +- hw/qdev.c | 488 ++++++++++++++++++++++++++++++++++++++-------- hw/qdev.h | 16 ++- hw/ssi.c | 6 +- monitor.c | 258 +++++++++++++++++++++---- monitor.h | 8 +- qbuffer.c | 116 +++++++++++ qbuffer.h | 33 +++ qemu-monitor.hx | 34 +++- qemu-tool.c | 6 +- qerror.c | 8 +- qerror.h | 6 +- qjson.c | 15 ++ qobject.h | 1 + 28 files changed, 1315 insertions(+), 160 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
PATCH v4 1/23 qdev: Rework qtree path abbreviations
From: Jan Kiszka <[email protected]>
Path abbreviations suffer from the inconsistency that bus names can only be omitted at the end of a path. Drop this special rule, and also remove the now obsolete QERR_DEVICE_MULTIPLE_BUSSES.
--- hw/qdev.c | 22 ++++------------------ qerror.c | 4 ---- qerror.h | 3 --- 3 files changed, 4 insertions(+), 25 deletions(-)
hw/qdev.c +4 -18
@@ -608,32 +608,18 @@ static BusState *qbus_find(const char *path) }return NULL ; } +if (dev->num_child_bus ==0 ) { +qerror_report (QERR_DEVICE_NO_BUS , elem); +return NULL ; + }assert (path[pos] =='/' || !path[pos]);while (path[pos] =='/' ) { pos++; } -if (path[pos] =='\0' ) { -/* last specified element is a device. If it has exactly -* one child bus accept it nevertheless */ -switch (dev->num_child_bus) { -case 0 : -qerror_report (QERR_DEVICE_NO_BUS , elem); -return NULL ; -case 1 : -return QLIST_FIRST (&dev->child_bus); -default : -qerror_report (QERR_DEVICE_MULTIPLE_BUSSES , elem); -if (!monitor_cur_is_qmp ()) { -qbus_list_bus (dev); - } -return NULL ; - } - }/* find bus */ if (sscanf (path+pos,"%127[^/]%n" , elem, &len) !=1 ) { -assert (0 ); elem[0 ] = len =0 ; } pos += len;
qerror.c +0 -4
@@ -77,10 +77,6 @@ static const QErrorStringTable qerror_table[] = { .desc = "Device '%(device)' is locked", }, { - .error_fmt = QERR_DEVICE_MULTIPLE_BUSSES, - .desc = "Device '%(device)' has multiple child busses", - }, - { .error_fmt = QERR_DEVICE_NOT_ACTIVE, .desc = "Device '%(device)' has not been activated by the guest", },
qerror.h +0 -3
@@ -73,9 +73,6 @@ QError *qobject_to_qerror(const QObject *obj);#define QERR_DEVICE_LOCKED \ "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }" -#define QERR_DEVICE_MULTIPLE_BUSSES \ - "{ 'class': 'DeviceMultipleBusses', 'data': { 'device': %s } }" -#define QERR_DEVICE_NOT_ACTIVE \ "{ 'class': 'DeviceNotActive', 'data': { 'device': %s } }"
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 01/23] qdev: Rework qtree path abbreviations
[cc: kraxel]
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Path abbreviations suffer from the inconsistency that bus names can only be omitted at the end of a path. Drop this special rule, and also remove the now obsolete
QERR_DEVICE_MULTIPLE_BUSSES.Signed-off-by Jan Kiszka <[email protected]>
Affects only -device and device_add option bus. I support this.
Quoted 11 lines
--- hw/qdev.c | 22 ++++------------------ qerror.c | 4 ---- qerror.h | 3 --- 3 files changed, 4 insertions(+), 25 deletions(-)hw/qdev.c +0
-0
hw/qdev.c static BusState *qbus_find(const char *path) · line 614
Quoted 7 lines
hw/qdev.c +4
-0@@ -608,32 +608,18 @@ static BusState *qbus_find(const char *path) }return NULL ; } +if (dev->num_child_bus ==0 ) { +qerror_report (QERR_DEVICE_NO_BUS , elem); +return NULL ; + }
I'd kill this check as well. The QERR_BUS_NOT_FOUND further down suffices.
Quoted 29 lines
hw/qdev.c +0
-18assert (path[pos] =='/' || !path[pos]);while (path[pos] =='/' ) { pos++; } -if (path[pos] =='\0' ) { -/* last specified element is a device. If it has exactly -* one child bus accept it nevertheless */ -switch (dev->num_child_bus) { -case 0 : -qerror_report (QERR_DEVICE_NO_BUS , elem); -return NULL ; -case 1 : -return QLIST_FIRST (&dev->child_bus); -default : -qerror_report (QERR_DEVICE_MULTIPLE_BUSSES , elem); -if (!monitor_cur_is_qmp ()) { -qbus_list_bus (dev); - } -return NULL ; - } - }/* find bus */ if (sscanf (path+pos,"%127[^/]%n" , elem, &len) !=1 ) { -assert (0 ); elem[0 ] = len =0 ; } pos += len;
[...]
Re: [Qemu-devel] [PATCH v4 01/23] qdev: Rework qtree path abbreviations
Hmm, what happens if the path denotes a device instead of a bus?
qemu-system-x86_64: -device e1000,bus=/i440FX-pcihost: Bus '' not found
Not so nice.
PATCH v4 2/23 qdev: Restrict direct bus addressing via its name
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
We allow to address a bus only using its local name. This is ambiguous but unfortunately so handy that people (specifically libvirt) will likely complain if bus=pci.0 needs to be replaced with bus=/i440FX-pcihost/pci.0 all over the place. So keep this for now but drop at least support for starting a qtree walks with an abbreviated bus name.
--- hw/qdev.c | 22 +++++++--------------- 1 files changed, 7 insertions(+), 15 deletions(-)
hw/qdev.c +7 -15
@@ -564,25 +564,17 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)static BusState *qbus_find (const char *path) {DeviceState *dev; -BusState *bus; +BusState *bus = main_system_bus;char elem[128 ]; -int pos, len; +int len, pos =0 ; -/* find start element */ -if (path[0 ] =='/' ) { - bus = main_system_bus; - pos =0 ; - }else { -if (sscanf (path,"%127[^/]%n" , elem, &len) !=1 ) { -assert (!path[0 ]); - elem[0 ] = len =0 ; - } - bus =qbus_find_recursive (main_system_bus, elem,NULL ); +/* search for bus name recursively if path is not absolute */ +if (path[0 ] !='/' ) { + bus =qbus_find_recursive (bus, path,NULL );if (!bus) { -qerror_report (QERR_BUS_NOT_FOUND , elem); -return NULL ; +qerror_report (QERR_BUS_NOT_FOUND , path); } - pos = len; +return bus; }for (;;) {
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 02/23] qdev: Restrict direct bus addressing via its name
Quoted 10 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
We allow to address a bus only using its local name. This is ambiguous but unfortunately so handy that people (specifically libvirt) will likely complain if bus=pci.0 needs to be replaced with bus=/i440FX-pcihost/pci.0 all over the place. So keep this for now but drop at least support for starting a qtree walks with an abbreviated bus name.
Signed-off-by Jan Kiszka <[email protected]>
Again, affects only -device and device_add option bus.
Before, an argument started either with '/' (path anchored at root) or a bus name (path anchored at that bus).
Now, an argument started either with '/' (path anchored at root) or it is a bus name.
If we decide to add sane relative paths to qdev, i.e. paths anchored at unique ID, we get a slight anomaly here:
bus=a bus name, not a relative path
bus=a/b relative path anchored at node with unique ID a.
Works for me.
Re: [Qemu-devel] [PATCH v4 02/23] qdev: Restrict direct bus addressing via its name
Quoted 21 lines · Markus Armbruster
Quoted 10 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
We allow to address a bus only using its local name. This is ambiguous but unfortunately so handy that people (specifically libvirt) will likely complain if bus=pci.0 needs to be replaced with bus=/i440FX-pcihost/pci.0 all over the place. So keep this for now but drop at least support for starting a qtree walks with an abbreviated bus name.
Signed-off-by Jan Kiszka <[email protected]>Again, affects only -device and
device_addoption bus.Before, an argument started either with '/' (path anchored at root) or a bus name (path anchored at that bus).
Now, an argument started either with '/' (path anchored at root) or it is a bus name.
Quoted 7 lines
If we decide to add sane relative paths to qdev, i.e. paths anchored at unique ID, we get a slight anomaly here:
bus=a bus name, not a relative path
bus=a/b relative path anchored at node with unique ID a.Works for me.
If we allow ID-anchoring, we are in ambiguous land again unless we reject IDs that happen to match any bus name in the system. Even then, the problem will be that the aliases based on bus names need to be auto-generated (for backward compatibility) while IDs are user-assigned. If the user comes first, auto-generation will innocently produce an ugly conflict and we will either have to reject bus instantiation or live with the conflict.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v4 02/23] qdev: Restrict direct bus addressing via its name
Quoted 30 lines · Jan Kiszka
Quoted 21 lines · Markus Armbruster
Quoted 10 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
We allow to address a bus only using its local name. This is ambiguous but unfortunately so handy that people (specifically libvirt) will likely complain if bus=pci.0 needs to be replaced with bus=/i440FX-pcihost/pci.0 all over the place. So keep this for now but drop at least support for starting a qtree walks with an abbreviated bus name.
Signed-off-by Jan Kiszka <[email protected]>Again, affects only -device and
device_addoption bus.Before, an argument started either with '/' (path anchored at root) or a bus name (path anchored at that bus).
Now, an argument started either with '/' (path anchored at root) or it is a bus name.
Quoted 7 lines
If we decide to add sane relative paths to qdev, i.e. paths anchored at unique ID, we get a slight anomaly here:
bus=a bus name, not a relative path
bus=a/b relative path anchored at node with unique ID a.Works for me.
Quoted 7 lines
If we allow ID-anchoring, we are in ambiguous land again unless we reject IDs that happen to match any bus name in the system. Even then, the problem will be that the aliases based on bus names need to be auto-generated (for backward compatibility) while IDs are user-assigned. If the user comes first, auto-generation will innocently produce an ugly conflict and we will either have to reject bus instantiation or live with the conflict.
No.
For backward compatibility, we need bus=a to be interpreted as bus name, not as qdev path. In other words, we overload bus to be either bus name or qdev path. It's a wart, but unavoidable unless we restrict bus to bus names and add a separate option for paths.
As long as we require all qdev paths to be absolute, the wart is relatively small. The rule to resolve the overloading is simple: if the value starts with '/', then it's a qdev path, else it's a bus name.
With relative qdev paths, the wart becomes more a bit more prominent. The rule to resolve the overloading becomes: if the value contains '/', it's a qdev path, else it's a bus name.
I can't see ambiguity here.
PATCH v4 3/23 qdev: Drop ID matching from qtree paths
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Device IDs may conflict with device names or aliases. From now on we only accept them outside qtree paths. This also makes dumping IDs in qbus_list_dev/bus obsolete.
--- hw/qdev.c | 16 ++++------------ 1 files changed, 4 insertions(+), 12 deletions(-)
hw/qdev.c +4 -12
@@ -497,8 +497,7 @@ static void qbus_list_bus(DeviceState *dev)BusState *child;const char *sep =" " ; -error_printf ("child busses at \"%s\":" , - dev->id ? dev->id : dev->info->name); +error_printf ("child busses at \"%s\":" , dev->info->name); QLIST_FOREACH(child, &dev->child_bus, sibling) {error_printf ("%s\"%s\"" , sep, child->name); sep =", " ;
@@ -514,8 +513,6 @@ static void qbus_list_dev(BusState *bus)error_printf ("devices at \"%s\":" , bus->name); QLIST_FOREACH(dev, &bus->children, sibling) {error_printf ("%s\"%s\"" , sep, dev->info->name); -if (dev->id) -error_printf ("/\"%s\"" , dev->id); sep =", " ; }error_printf ("\n" );
@@ -539,15 +536,10 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem)/* * try to match in order: -* (1) instance id, if present -* (2) driver name -* (3) driver alias, if present +* (1) driver name +* (2) driver alias, if present */ - QLIST_FOREACH(dev, &bus->children, sibling) { -if (dev->id &&strcmp (dev->id, elem) ==0 ) { -return dev; - } - } + QLIST_FOREACH(dev, &bus->children, sibling) {if (strcmp (dev->info->name, elem) ==0 ) {return dev;
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 03/23] qdev: Drop ID matching from qtree paths
[cc: kraxel]
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Device IDs may conflict with device names or aliases. From now on we only accept them outside qtree paths. This also makes dumping IDs in qbus_list_dev/bus obsolete.
Signed-off-by Jan Kiszka <[email protected]>
I don't like this at all.
1. Specific problem:
With the current code, multiple devices with the same driver work only if I take care: addressing by driver name gets me only the first, so I better set suitable IDs.
With your patch, multiple devices with the same driver don't work, no matter what I do.
2. General principle:
When I set an ID, I want the system to accept that ID in all contexts where it makes sense. Ambiguity created by badly chosen IDs is *my* problem.
Re: [Qemu-devel] [PATCH v4 03/23] qdev: Drop ID matching from qtree paths
Quoted 14 lines · Markus Armbruster
[cc: kraxel]
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Device IDs may conflict with device names or aliases. From now on we only accept them outside qtree paths. This also makes dumping IDs in qbus_list_dev/bus obsolete.
Signed-off-by Jan Kiszka <[email protected]>I don't like this at all.
Quoted 8 lines
1. Specific problem:
With the current code, multiple devices with the same driver work only if I take care: addressing by driver name gets me only the first, so I better set suitable IDs.
With your patch, multiple devices with the same driver don't work, no matter what I do.
[ you already found how this is resolved ]
Quoted 6 lines
2. General principle:
When I set an ID, I want the system to accept that ID in all contexts where it makes sense. Ambiguity created by badly chosen IDs is *my* problem.
I disagree. IMO, QEMU should support the user to avoid such subtle shadowing.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v4 03/23] qdev: Drop ID matching from qtree paths
Quoted 17 lines · Jan Kiszka
Quoted 16 lines · Markus Armbruster
[cc: kraxel]
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Device IDs may conflict with device names or aliases. From now on we only accept them outside qtree paths. This also makes dumping IDs in qbus_list_dev/bus obsolete.
Signed-off-by Jan Kiszka <[email protected]>I don't like this at all.
1. Specific problem:
Quoted 8 lines
Quoted 6 lines
With the current code, multiple devices with the same driver work only if I take care: addressing by driver name gets me only the first, so I better set suitable IDs.
With your patch, multiple devices with the same driver don't work, no matter what I do.
[ you already found how this is resolved ]
Yes, but I don't like that the intermediate state is broken, and I don't like the final state.
Quoted 8 lines
Quoted 5 lines
2. General principle:
When I set an ID, I want the system to accept that ID in all contexts where it makes sense. Ambiguity created by badly chosen IDs is *my* problem.
I disagree. IMO, QEMU should support the user to avoid such subtle shadowing.
IDs let me refer to my devices without having to look up ever-changing instance numbers. This is especially relevant when I want to do refer to buses without using (possibly ambigious) bus names.
I want to be able to say bus=scsi-ctrl.2/scsi.0 instead of /i440FX-pcihost/pci.0/lsi53c895a.<ever-changing-number>/scsi.0
That's much more valuable to me than a well-meant attempt at protecting me from choosing IDs badly.
Throw in multiple PCI buses, and a "no IDs in qdev paths" law becomes even more annoying.
Re: [Qemu-devel] [PATCH v4 03/23] qdev: Drop ID matching from qtree paths
Quoted 37 lines · Markus Armbruster
Quoted 16 lines · Jan Kiszka
Quoted 15 lines · Markus Armbruster
[cc: kraxel]
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Device IDs may conflict with device names or aliases. From now on we only accept them outside qtree paths. This also makes dumping IDs in qbus_list_dev/bus obsolete.
Signed-off-by Jan Kiszka <[email protected]>I don't like this at all.
1. Specific problem:
Quoted 7 lines
Quoted 6 lines
With the current code, multiple devices with the same driver work only if I take care: addressing by driver name gets me only the first, so I better set suitable IDs.
With your patch, multiple devices with the same driver don't work, no matter what I do.
[ you already found how this is resolved ]
Yes, but I don't like that the intermediate state is broken, and I don't like the final state.
Quoted 7 lines
Quoted 5 lines
2. General principle:
When I set an ID, I want the system to accept that ID in all contexts where it makes sense. Ambiguity created by badly chosen IDs is *my* problem.
I disagree. IMO, QEMU should support the user to avoid such subtle shadowing.
Quoted 6 lines
IDs let me refer to my devices without having to look up ever-changing instance numbers. This is especially relevant when I want to do refer to buses without using (possibly ambigious) bus names.
I want to be able to say bus=scsi-ctrl.2/scsi.0 instead of /i440FX-pcihost/pci.0/lsi53c895a.<ever-changing-number>/scsi.0
It would once be something like /i440FX-pcihost/pci.0/@04.0/scsi.0 (with optional extension of '@04.0' to '@04.0:lsi53c895a.0' for more verbose interactive use), so at least no volatile naming here. But letting a qdev path start with an ID can of course shorten things, specifically on the command line where we have no completion.
Quoted 6 lines
That's much more valuable to me than a well-meant attempt at protecting me from choosing IDs badly.
Throw in multiple PCI buses, and a "no IDs in qdev paths" law becomes even more annoying.
No IDs _inside_ qdev paths, please. Letting them start is imaginable if we find proper rules how to resolve the possible conflicts with auto-generated bus aliases ('pci.0' = '/i440FX-pcihost/pci.0').
Let's look at it as Paul suggested: relative qdev path beginnings are aliases of the corresponding absolute bits. When registering a device with an ID, we also register an alias 'ID' = '/absolute/path/to/device'. Here we can (and already do) reject the registration if 'ID' is not unique. When registering a bus, we would add an alias 'busname' = '/absolute/path/to/bus'. This can already fail due to the fact that bus names need not be unique. But it could also fail if some silly guy registered a device with the same ID before.
Now what to do? Register the alias nevertheless and select whatever comes first in the alias list, possibly creating instable paths again? Or reject the bus registration (i.e. the registration of the parent device? Or silently skip the failing alias registration (which would mean having no such alias at all once the other device gets hot-removed)?
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
PATCH v4 4/23 qdev: Allow device addressing via 'driver.instance'
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
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'. Attach this name extension whenever an instantiated device is printed.
--- docs/qdev-device-use.txt | 13 +++++++++++- hw/qdev.c | 48 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 10 deletions(-)
docs/qdev-device-use.txt +12 -1
@@ -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
@@ -20,6 +20,17 @@ 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. +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] + +The instance number counts devices managed by the same driver on a +specifc bus. It is zero-based. + +Example: /i440FX-pcihost/pci.0/e1000.1 addresses the second e1000 +adapter on the bus 'pci.0'. + Note: the USB device address can't be controlled at this time. === Block Devices ===
hw/qdev.c +39 -9
89 lines not on this page · raw message
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 04/23] qdev: Allow device addressing via 'driver.instance'
[cc: kraxel]
Quoted 8 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Extend
qbus_find_devto allow addressing of devices without an unique id via an optional per-bus instance number. The new formats are 'driver.instance' and 'alias.instance'. Attach this name extension whenever an instantiated device is printed.Signed-off-by Jan Kiszka <[email protected]>
The instance number repairs the specific problem created by the previous patch. My objection to it on general principles stands.
That said, I'm neutral on the instance number feature. Instance numbers are not suitable where address stability is needed (see debate on canonical addresses), but they might be convenient for interactive use.
There is some functional overlap to the [driver-name]@bus-address syntax proposed elsewhere. Especially visible for buses where the bus-address is a small integer. Could be confusing.
There is some conceptual overlap with the .NUMBER in default bus names (see qbus_create_inplace()). Instance numbers count for each name independently. Numbers in default bus names count for all names. But I think we agree the latter sucks. Paul wants us to get rid of it.
Quoted 18 lines
--- docs/qdev-device-use.txt | 13 +++++++++++- hw/qdev.c | 48 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 10 deletions(-)docs/qdev-device-use.txt +1
-1@@ -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
docs/qdev-device-use.txt bus named pci.0. To put a FOO device into its slot 4, use -device · line 29
Quoted 10 lines
docs/qdev-device-use.txt +7
-0@@ -20,6 +20,17 @@ 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. +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] + +The instance number counts devices managed by the same driver on a +specifc bus. It is zero-based.
~~~~~~~ typo
Quoted 7 lines
docs/qdev-device-use.txt +4
-0+ +Example: /i440FX-pcihost/pci.0/e1000.1 addresses the second e1000 +adapter on the bus 'pci.0'. + Note: the USB device address can't be controlled at this time. === Block Devices ===
[...]
PATCH v4 5/23 qdev: Convert device and bus lists to QTAILQ
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
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'.
--- hw/acpi_piix4.c | 2 +- hw/i2c.c | 2 +- hw/pci-hotplug.c | 2 +- hw/qdev.c | 41 +++++++++++++++++++++-------------------- hw/qdev.h | 8 ++++---- hw/ssi.c | 6 +++--- 6 files changed, 31 insertions(+), 30 deletions(-)
hw/acpi_piix4.c +1 -1
@@ -564,7 +564,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);
hw/i2c.c +1 -1
@@ -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;
hw/pci-hotplug.c +1 -1
@@ -74,7 +74,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 ;
hw/qdev.c +21 -20
146 lines not on this page · raw message
hw/qdev.h +4 -4
@@ -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 {
hw/ssi.c +3 -3
@@ -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 ; }
Signature · 1 line
1.6.0.2
PATCH v4 6/23 qdev: Push QMP mode checks into qbus_list_bus/dev
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Simplifies the usage.
--- hw/qdev.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
hw/qdev.c +8 -6
@@ -514,6 +514,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.%d\":" , dev->info->name,qdev_instance_no (dev)); QTAILQ_FOREACH(child, &dev->child_bus, sibling) {
@@ -528,6 +531,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.%d\"", sep, dev->info->name,
@@ -618,9 +624,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 ; }if (dev->num_child_bus ==0 ) {
@@ -641,9 +645,7 @@ static BusState *qbus_find(const char *path) bus =qbus_find_bus (dev, elem);if (!bus) {qerror_report (QERR_BUS_NOT_FOUND , elem); -if (!monitor_cur_is_qmp ()) { -qbus_list_bus (dev); - } +qbus_list_bus (dev);return NULL ; } }
Signature · 1 line
1.6.0.2
PATCH v4 7/23 qdev: Allow device specification by qtree path for device_del
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
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 walking the qtree with searching for device IDs if required.
--- hw/qdev.c | 75 ++++++++++++++++++++++++++++++++++++++++++++----------- qemu-monitor.hx | 10 +++--- 2 files changed, 65 insertions(+), 20 deletions(-)
hw/qdev.c +60 -15
@@ -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 (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 (path, true);if (!bus) {return NULL ; }
@@ -475,7 +475,7 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,return NULL ; } -static DeviceState *qdev_find_recursive (BusState *bus,const char *id) +static DeviceState *qdev_find_id_recursive (BusState *bus,const char *id) {DeviceState *dev, *ret;BusState *child;
@@ -484,7 +484,7 @@ static DeviceState *qdev_find_recursive(BusState *bus, const char *id)if (dev->id &&strcmp (dev->id, id) ==0 )return dev; QTAILQ_FOREACH(child, &dev->child_bus, sibling) { - ret =qdev_find_recursive (child, id); + ret =qdev_find_id_recursive (child, id);if (ret) {return ret; }
@@ -590,7 +590,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)return NULL ; } -static BusState *qbus_find (const char *path) +static BusState *qbus_find (const char *path,bool report_errors) {DeviceState *dev;BusState *bus = main_system_bus;
@@ -600,7 +600,7 @@ static BusState *qbus_find(const char *path)/* search for bus name recursively if path is not absolute */ if (path[0 ] !='/' ) { bus =qbus_find_recursive (bus, path,NULL ); -if (!bus) { +if (!bus && report_errors) {qerror_report (QERR_BUS_NOT_FOUND , path); }return bus;
@@ -623,12 +623,16 @@ 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 ; }if (dev->num_child_bus ==0 ) { -qerror_report (QERR_DEVICE_NO_BUS , elem); +if (report_errors) { +qerror_report (QERR_DEVICE_NO_BUS , elem); + }return NULL ; }
@@ -644,13 +648,55 @@ static BusState *qbus_find(const char *path) pos += len; bus =qbus_find_bus (dev, elem);if (!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) +{ +const char *dev_name; +DeviceState *dev; +char *bus_path; +BusState *bus; + +/* search for unique ID recursively if path is not absolute */ +if (path[0 ] !='/' ) { + dev =qdev_find_id_recursive (main_system_bus, path); +if (!dev) { +qerror_report (QERR_DEVICE_NOT_FOUND , path); + } +return dev; + } + + dev_name =strrchr (path,'/' ) +1 ; + + bus_path =qemu_strdup (path); + bus_path[dev_name - path] =0 ; + + bus =qbus_find (bus_path, false); +qemu_free (bus_path); +if (!bus) { +/* retry with full path to generate correct error message */ + bus =qbus_find (path, true); +if (!bus) { +return NULL ; + } + dev_name ="" ; + } + + dev =qbus_find_dev (bus, dev_name); +if (!dev) { +qerror_report (QERR_DEVICE_NOT_FOUND , dev_name); +qbus_list_dev (bus); + } +return dev; +} +void qbus_create_inplace (BusState *bus,BusInfo *info,DeviceState *parent,const char *name) {
@@ -810,12 +856,11 @@ 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,"device" );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) {return -1 ; }return qdev_unplug (dev);
qemu-monitor.hx +5 -5
@@ -703,7 +703,7 @@ EQMP { .name = "device_del", - .args_type = "id:s", + .args_type = "device: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{device} @findex device_del -Remove device @var{id}. +Remove @var{device}, specified via its qtree path or unique ID. ETEXI SQMP device_del
@@ -724,11 +724,11 @@ Remove a device. Arguments: -- "id": the device's ID (json-string) +- "device": the device's qtree path or unique ID (json-string) Example: --> { "execute": "device_del", "arguments": { "id": "net1" } } +-> { "execute": "device_del", "arguments": { "device": "net1" } } <- { "return": {} } EQMP
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 07/23] qdev: Allow device specification by qtree path for device_del
[cc: kraxel]
Quoted 8 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Allow to specify the device to be removed via
device_delnot only by ID but also by its full or abbreviated qtree path. For this purpose,qdev_findis introduced which combines walking the qtree with searching for device IDs if required.Signed-off-by Jan Kiszka <[email protected]>
I like accepting paths there.
Your previous decision to abolish ID in qdev paths makes plain ID a special case here instead of a relative path that happens to be short.
Quoted 119 lines
--- hw/qdev.c | 75 ++++++++++++++++++++++++++++++++++++++++++++----------- qemu-monitor.hx | 10 +++--- 2 files changed, 65 insertions(+), 20 deletions(-)hw/qdev.c +38
-11@@ -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 (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 (path, true);if (!bus) {return NULL ; }@@ -475,7 +475,7 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,return NULL ; } -static DeviceState *qdev_find_recursive (BusState *bus,const char *id) +static DeviceState *qdev_find_id_recursive (BusState *bus,const char *id) {DeviceState *dev, *ret;BusState *child;@@ -484,7 +484,7 @@ static DeviceState *qdev_find_recursive(BusState *bus, const char *id)if (dev->id &&strcmp (dev->id, id) ==0 )return dev; QTAILQ_FOREACH(child, &dev->child_bus, sibling) { - ret =qdev_find_recursive (child, id); + ret =qdev_find_id_recursive (child, id);if (ret) {return ret; }@@ -590,7 +590,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)return NULL ; } -static BusState *qbus_find (const char *path) +static BusState *qbus_find (const char *path,bool report_errors) {DeviceState *dev;BusState *bus = main_system_bus;@@ -600,7 +600,7 @@ static BusState *qbus_find(const char *path)/* search for bus name recursively if path is not absolute */ if (path[0 ] !='/' ) { bus =qbus_find_recursive (bus, path,NULL ); -if (!bus) { +if (!bus && report_errors) {qerror_report (QERR_BUS_NOT_FOUND , path); }return bus;@@ -623,12 +623,16 @@ 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 ; }if (dev->num_child_bus ==0 ) { -qerror_report (QERR_DEVICE_NO_BUS , elem); +if (report_errors) { +qerror_report (QERR_DEVICE_NO_BUS , elem); + }return NULL ; }@@ -644,13 +648,55 @@ static BusState *qbus_find(const char *path) pos += len; bus =qbus_find_bus (dev, elem);if (!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) +{ +const char *dev_name; +DeviceState *dev; +char *bus_path; +BusState *bus; + +/* search for unique ID recursively if path is not absolute */ +if (path[0 ] !='/' ) { + dev =qdev_find_id_recursive (main_system_bus, path); +if (!dev) { +qerror_report (QERR_DEVICE_NOT_FOUND , path); + } +return dev; + } + + dev_name =strrchr (path,'/' ) +1 ; + + bus_path =qemu_strdup (path); + bus_path[dev_name - path] =0 ; +
hw/qdev.c static BusState *qbus_find(const char *path) · line 690
Quoted 10 lines
hw/qdev.c +10
-0+ bus =qbus_find (bus_path, false); +qemu_free (bus_path); +if (!bus) { +/* retry with full path to generate correct error message */ + bus =qbus_find (path, true); +if (!bus) { +return NULL ; + } + dev_name ="" ; + }
Awkward.
What happens here is you hack off the last path component (because it must be a device), then resolve the rest as bus, suppressing errors. If it fails, you resolve the complete path as bus without error suppression. Since the first try failed, the second try must surely run into the same failure before it reaches the last path component. So it doesn't matter that the whole path is supposed to be a device, not a bus.
This leads to suboptimal error messages for arguments that are actually buses, not devices. For instance, /i440FX-pcihost/pci.0/piix3-ide/ide.0 will try to resolve /i440FX-pcihost/pci.0/piix3-ide as bus, fail, and report it as "Bus '' not found." (I think).
I don't like separate qdev path resolution for buses and devices. Just resolve it, and if what you got is not of the kind you want, report that.
Quoted 12 lines
hw/qdev.c +9
-0+ + dev =qbus_find_dev (bus, dev_name); +if (!dev) { +qerror_report (QERR_DEVICE_NOT_FOUND , dev_name); +qbus_list_dev (bus); + } +return dev; +} +void qbus_create_inplace (BusState *bus,BusInfo *info,DeviceState *parent,const char *name) {
[...]
PATCH v4 8/23 qdev: Introduce qdev_iterate_recursive
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Add qdev_iterate_recursive to walk the complete qtree invoking a callback for each device. Use this service to implement qdev_find_id_recursive.
--- hw/qdev.c | 29 +++++++++++++++++++++++++---- hw/qdev.h | 3 +++ 2 files changed, 28 insertions(+), 4 deletions(-)
hw/qdev.c +25 -4
@@ -475,16 +475,22 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,return NULL ; } -static DeviceState *qdev_find_id_recursive (BusState *bus,const char *id) +void *qdev_iterate_recursive (BusState *bus,qdev_iteratefn callback, +void *opaque) {DeviceState *dev, *ret;BusState *child; +if (!bus) { + bus = main_system_bus; + } QTAILQ_FOREACH(dev, &bus->children, sibling) { -if (dev->id &&strcmp (dev->id, id) ==0 ) -return dev; + ret =callback (dev, opaque); +if (ret) { +return ret; + } QTAILQ_FOREACH(child, &dev->child_bus, sibling) { - ret =qdev_find_id_recursive (child, id); + ret =qdev_iterate_recursive (child, callback, opaque);if (ret) {return ret; }
@@ -493,6 +499,21 @@ static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)return NULL ; } +static void *find_id_callback (DeviceState *dev,void *opaque) +{ +const char *id = opaque; + +if (dev->id &&strcmp (dev->id, id) ==0 ) { +return dev; + } +return NULL ; +} + +static DeviceState *qdev_find_id_recursive (BusState *bus,const char *id) +{ +return qdev_iterate_recursive (bus, find_id_callback, (void *)id); +} +static int qdev_instance_no (DeviceState *dev) {struct DeviceState *sibling;
hw/qdev.h +3 -0
@@ -134,6 +134,7 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char *name);typedef int (*qdev_initfn )(DeviceState *dev,DeviceInfo *info);typedef int (*qdev_event )(DeviceState *dev);typedef void (*qdev_resetfn )(DeviceState *dev); +typedef void *(*qdev_iteratefn )(DeviceState *dev,void *opaque);struct DeviceInfo {const char *name;
@@ -168,6 +169,8 @@ 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); +void *qdev_iterate_recursive (BusState *bus,qdev_iteratefn callback, +void *opaque);/*** BUS API. ***/
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 08/23] qdev: Introduce qdev_iterate_recursive
Quoted 33 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Add
qdev_iterate_recursiveto walk the complete qtree invoking a callback for each device. Use this service to implementqdev_find_id_recursive.Signed-off-by Jan Kiszka <[email protected]>--- hw/qdev.c | 29 +++++++++++++++++++++++++---- hw/qdev.h | 3 +++ 2 files changed, 28 insertions(+), 4 deletions(-)hw/qdev.c +5
-3@@ -475,16 +475,22 @@ static BusState *qbus_find_recursive(BusState *bus, const char *name,return NULL ; } -static DeviceState *qdev_find_id_recursive (BusState *bus,const char *id) +void *qdev_iterate_recursive (BusState *bus,qdev_iteratefn callback, +void *opaque) {DeviceState *dev, *ret;BusState *child; +if (!bus) { + bus = main_system_bus; + } QTAILQ_FOREACH(dev, &bus->children, sibling) { -if (dev->id &&strcmp (dev->id, id) ==0 ) -return dev;
hw/qdev.c static BusState *qbus_find_recursive(BusState *bus, const char *name, · line 496
Quoted 10 lines
hw/qdev.c +5
-1+ ret =callback (dev, opaque); +if (ret) { +return ret; + } QTAILQ_FOREACH(child, &dev->child_bus, sibling) { - ret =qdev_find_id_recursive (child, id); + ret =qdev_iterate_recursive (child, callback, opaque);if (ret) {return ret; }
I'd call "iterate recursive" simply "walk". It's common usage for trees.
Except this isn't a walk or iteration, it's a search: the walk stops as soon as the callback returns non-null. Not obvious from the name, therefore needs a comment.
[...]
PATCH v4 9/23 monitor: Fix leakage during completion processing
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Given too many arguments or an invalid command, we were leaking the duplicated argument strings.
--- monitor.c | 23 +++++++++++++++-------- 1 files changed, 15 insertions(+), 8 deletions(-)
monitor.c +15 -8
@@ -3877,8 +3877,9 @@ static void monitor_find_completion(const char *cmdline)next arg */ len =strlen (cmdline);if (len >0 &&qemu_isspace (cmdline[len -1 ])) { -if (nb_args >=MAX_ARGS ) -return ; +if (nb_args >=MAX_ARGS ) { + goto cleanup; + } args[nb_args++] =qemu_strdup ("" ); }if (nb_args <=1 ) {
@@ -3893,12 +3894,15 @@ static void monitor_find_completion(const char *cmdline) } }else {/* find the command */ -for (cmd = mon_cmds; cmd->name !=NULL ; cmd++) { -if (compare_cmd (args[0 ], cmd->name)) - goto found; +for (cmd = mon_cmds; cmd->name !=NULL ; cmd++) { +if (compare_cmd (args[0 ], cmd->name)) { +break ; + } } -return ; - found: +if (!cmd->name) { + goto cleanup; + } + ptype =next_arg_type (cmd->args_type);for (i =0 ; i < nb_args -2 ; i++) {if (*ptype !='\0' ) {
@@ -3948,8 +3952,11 @@ static void monitor_find_completion(const char *cmdline)break ; } } -for (i =0 ; i < nb_args; i++) + +cleanup: +for (i =0 ; i < nb_args; i++) {qemu_free (args[i]); + } }static int monitor_can_read (void *opaque)
Signature · 1 line
1.6.0.2
PATCH v4 10/23 monitor: Fix command completion vs. boolean switches
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
We now have to move forward to the next argument type via next_arg_type. This patch fixes completion for 'eject' and maybe also other commands.
--- monitor.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
monitor.c +1 -1
@@ -3913,7 +3913,7 @@ static void monitor_find_completion(const char *cmdline) } str = args[nb_args -1 ];if (*ptype =='-' && ptype[1 ] !='\0' ) { - ptype +=2 ; + ptype =next_arg_type (ptype); }switch (*ptype) {case 'F' :
Signature · 1 line
1.6.0.2
PATCH v4 11/23 monitor: Add completion support for option lists
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.
--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)
monitor.c +57 -11
@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer
@@ -3353,6 +3356,11 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,QemuOptsList *opts_list;QemuOpts *opts; +if (*typestr =='(' ) { +while (*typestr++ !=')' ) { +assert (*typestr !='\0' ); + } + } opts_list =qemu_find_opts (key);if (!opts_list || opts_list->desc->name) { goto bad_type;
@@ -3857,12 +3865,30 @@ static const char *next_arg_type(const char *typestr)return (p !=NULL ? ++p : typestr); } +static bool process_completion_type (char type,const char *str) +{ +switch (type) { +case 'F' : +/* file completion */ +readline_set_completion_index (cur_mon->rs,strlen (str)); +file_completion (str); +return true; +case 'B' : +/* block device name completion */ +readline_set_completion_index (cur_mon->rs,strlen (str)); +bdrv_iterate (block_completion_it, (void *)str); +return true; +default : +return false; + } +} +static void monitor_find_completion (const char *cmdline) {const char *cmdname;char *args[MAX_ARGS ];int nb_args, i, len; -const char *ptype, *str; +const char *ptype, *str, *opt, *sep;const mon_cmd_t *cmd;const KeyDef *key;
@@ -3915,16 +3941,31 @@ static void monitor_find_completion(const char *cmdline)if (*ptype =='-' && ptype[1 ] !='\0' ) { ptype =next_arg_type (ptype); } +if (process_completion_type (*ptype, str)) { + goto cleanup; + }switch (*ptype) { -case 'F' : -/* file completion */ -readline_set_completion_index (cur_mon->rs,strlen (str)); -file_completion (str); -break ; -case 'B' : -/* block device name completion */ -readline_set_completion_index (cur_mon->rs,strlen (str)); -bdrv_iterate (block_completion_it, (void *)str); +case 'O' : + sep =strrchr (str,',' ); + opt = sep ? sep +1 : str; + sep =strchr (opt,'=' ); +if (!sep) { +break ; + } + len = sep - opt; + str = sep +1 ; + ptype +=2 ; +while (*ptype !=')' ) { +if (strlen (ptype) > len+1 && ptype[len] ==':' && +strncmp (ptype, opt, len) ==0 ) { +process_completion_type (ptype[len+1 ], str); + } +while (*ptype++ !=',' ) { +if (*ptype ==')' ) { +break ; + } + } + }break ;case 's' :/* XXX: more generic ? */
@@ -3934,7 +3975,7 @@ static void monitor_find_completion(const char *cmdline)cmd_completion (str, cmd->name); } }else if (!strcmp (cmd->name,"sendkey" )) { -char *sep =strrchr (str,'-' ); + sep =strrchr (str,'-' );if (sep) str = sep +1 ;readline_set_completion_index (cur_mon->rs,strlen (str));
@@ -4114,6 +4155,11 @@ static int monitor_check_qmp_args(const mon_cmd_t *cmd, QDict *args) cmd_args.flag = *p++; cmd_args.optional =1 ; }else if (cmd_args.type =='O' ) { +if (*p =='(' ) { +while (*p++ !=')' ) { +assert (*p !='\0' ); + } + } opts_list =qemu_find_opts (qstring_get_str (cmd_args.name));assert (opts_list); }else if (*p =='?' ) {
qemu-monitor.hx +1 -1
@@ -660,7 +660,7 @@ ETEXI { .name = "device_add", - .args_type = "device:O", + .args_type = "device:O(drive:B)", .params = "driver[,prop=value][,...]", .help = "add device, like -device on the command line", .user_print = monitor_user_noop,
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0
monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer
Ugh.
Replacement of args_type by a proper data structure is long overdue. We keep piling features into that poor, hapless string.
Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
[...]
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
Quoted 32 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Quoted 5 lines
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what device_show makes direct use of. Same for separate args for HMP and QMP.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
Quoted 36 lines · Jan Kiszka
Quoted 35 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.
Quoted 7 lines
Quoted 2 lines
Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what
device_showmakes direct use of. Same for separate args for HMP and QMP.
Sensible.
Speaking of args_type. We've accumulated too many ways to represent dynamic data types, and too many ways to describe data types.
The most powerful dynamic data type is QObject. It comes with a useful plain-text representation (JSON). We lack a data type for describing QObjects. We use args_type to describe special QObjects, namely monitor command arguments. We've clearly stretched that to the limit and then some. We could use a general solution. JSON Schema as its plain-text representation could be nice.
Then we have QemuOpts. It's more limited (flat list instead of trees), but it comes with a real data type to describe it (QemuOptsList).
There's also QEMUOptionParameter. Serves a similar purpose. I don't know why we have both.
Over in qdev-land, we find Property, which can be viewed a data type to describe (parts of) a struct.
Some unification would be nice.
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
In reply to Jan KiszkaQuoted 36 lines · On Wed, 23 Jun 2010 12:28:27 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 35 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.
Quoted 7 lines
Quoted 2 lines
Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what
device_showmakes direct use of. Same for separate args for HMP and QMP.
IIRC, the separate args idea use case was to allow commands like device_del to have an ID argument and a path argument, right? If so, I think it doesn't matter anymore as we have agreed on having a device argument which would accept both, even under QMP, right Markus?
By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 and 22/23 in a different series, I could try pushing them in my next pull request.
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
Quoted 45 lines · Luiz Capitulino
Quoted 35 lines · On Wed, 23 Jun 2010 12:28:27 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 34 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.Quoted 6 lines
Quoted 2 lines
Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what
device_showmakes direct use of. Same for separate args for HMP and QMP.
Quoted 4 lines
IIRC, the separate args idea use case was to allow commands like
device_delto have an ID argument and a path argument, right? If so, I think it doesn't matter anymore as we have agreed on having a device argument which would accept both, even under QMP, right Markus?
To my understanding: As a leading element in qdev path, at least to address a device, maybe also to abbreviate only the beginning of a full path (that's currently to major remaining open issue).
Quoted 4 lines
By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 and 22/23 in a different series, I could try pushing them in my next pull request.
Do they need rebasing? If not, feel free to pick them up as you like. My series requires a v5 round anyway once discussion on path construction finally came to an end.
Jan
Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PATCH v4 11/23] monitor: Add completion support for option lists
Quoted 46 lines · On Mon, 28 Jun 2010 16:40:58 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 45 lines · Luiz Capitulino
Quoted 41 lines · On Wed, 23 Jun 2010 12:28:27 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 31 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Quoted 5 lines
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what
device_showmakes direct use of. Same for separate args for HMP and QMP.
Quoted 8 lines
Quoted 4 lines
IIRC, the separate args idea use case was to allow commands like
device_delto have an ID argument and a path argument, right? If so, I think it doesn't matter anymore as we have agreed on having a device argument which would accept both, even under QMP, right Markus?To my understanding: As a leading element in qdev path, at least to address a device, maybe also to abbreviate only the beginning of a full path (that's currently to major remaining open issue).
I'm ok with it if it's unambiguous.
Quoted 7 lines
Quoted 3 lines
By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 and 22/23 in a different series, I could try pushing them in my next pull request.
Do they need rebasing? If not, feel free to pick them up as you like. My series requires a v5 round anyway once discussion on path construction finally came to an end.
Done for them all, except 16/23 which mentions device_show in the changelog.
I should send a pull request until this Wednesday.
PATCH 1/1 monitor: Allow to exclude commands from QMP
Quoted 58 lines · Luiz Capitulino
Quoted 45 lines · On Mon, 28 Jun 2010 16:40:58 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 44 lines · Luiz Capitulino
Quoted 41 lines · On Wed, 23 Jun 2010 12:28:27 +0200 Jan Kiszka <[email protected]> wrote:
Quoted 31 lines · Markus Armbruster
Quoted 18 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This enables command line completion inside option strings. A list of expected key names and their completion type can be appended to the 'O' inside parentheses ('
O(key:type,...)'). The first use case is block device completion for the 'drive' option of 'device_add'.Signed-off-by Jan Kiszka <[email protected]>--- monitor.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++--------- qemu-monitor.hx | 2 +- 2 files changed, 58 insertions(+), 12 deletions(-)monitor.c +0
-0monitor.c line 76
Quoted 9 lines
monitor.c +3
-0@@ -68,6 +68,9 @@* 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. +* Command completion for specific keys can be requested via +* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction * 'i' 32 bit integer Ugh.
Quoted 5 lines
Replacement of
args_typeby a proper data structure is long overdue. We keep piling features into that poor, hapless string.Information on how to complete QemuOptsList options arguably belongs into the option description, i.e. QemuOptDesc.
For sure, that would be better. I just wonder how much of it should be stuffed into this series. I guess I will drop this part for now, just focusing on what
device_showmakes direct use of. Same for separate args for HMP and QMP.Quoted 7 lines
Quoted 4 lines
IIRC, the separate args idea use case was to allow commands like
device_delto have an ID argument and a path argument, right? If so, I think it doesn't matter anymore as we have agreed on having a device argument which would accept both, even under QMP, right Markus?To my understanding: As a leading element in qdev path, at least to address a device, maybe also to abbreviate only the beginning of a full path (that's currently to major remaining open issue).
I'm ok with it if it's unambiguous.
Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
By the way, if you send patches 09/23, 10/23, 15/23, (maybe) 16/23, 21/23 and 22/23 in a different series, I could try pushing them in my next pull request.
Do they need rebasing? If not, feel free to pick them up as you like. My series requires a v5 round anyway once discussion on path construction finally came to an end.
Done for them all, except 16/23 which mentions
device_showin the changelog.
If that's the only issue of 16/23, feel free to pick up the cleaned version below.
Quoted 2 lines
I should send a pull request until this Wednesday.
Great, thanks.
Jan
-------------->
Ported commands that are marked 'user_only' will not be considered for QMP monitor sessions. This allows to implement new commands that do not (yet) provide a sufficiently stable interface for QMP use.
---
For Luiz' queue, depends on "monitor: Establish cmd flags and convert the async tag" as posted earlier.
monitor.c | 18 +++++++++++++++--- monitor.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-)
monitor.c +15 -3
@@ -330,6 +330,11 @@ static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)return cmd->flags &MONITOR_CMD_ASYNC ; } +static inline bool monitor_cmd_user_only (const mon_cmd_t *cmd) +{ +return (cmd->flags &MONITOR_CMD_USER_ONLY ); +} +static inline int monitor_has_error (const Monitor *mon) {return mon->error !=NULL ;
@@ -612,6 +617,11 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data) goto help; } +if (monitor_ctrl_mode (mon) &&monitor_cmd_user_only (cmd)) { +qerror_report (QERR_COMMAND_NOT_FOUND , item); +return -1 ; + } +if (monitor_handler_is_async (cmd)) {if (monitor_ctrl_mode (mon)) {qmp_async_info_handler (mon, cmd);
@@ -709,13 +719,14 @@ static void do_info_commands(Monitor *mon, QObject **ret_data) cmd_list =qlist_new ();for (cmd = mon_cmds; cmd->name !=NULL ; cmd++) { -if (monitor_handler_ported (cmd) && !compare_cmd (cmd->name,"info" )) { +if (monitor_handler_ported (cmd) && !monitor_cmd_user_only (cmd) && + !compare_cmd (cmd->name,"info" )) {qlist_append_obj (cmd_list,get_cmd_dict (cmd->name)); } }for (cmd = info_cmds; cmd->name !=NULL ; cmd++) { -if (monitor_handler_ported (cmd)) { +if (monitor_handler_ported (cmd) && !monitor_cmd_user_only (cmd)) {char buf[128 ];snprintf (buf,sizeof (buf),"query-%s" , cmd->name);qlist_append_obj (cmd_list,get_cmd_dict (buf));
@@ -4207,7 +4218,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) qobject_from_jsonf("{ 'item': %s }", info_item)); }else { cmd =monitor_find_command (cmd_name); -if (!cmd || !monitor_handler_ported (cmd)) { +if (!cmd || !monitor_handler_ported (cmd) + ||monitor_cmd_user_only (cmd)) {qerror_report (QERR_COMMAND_NOT_FOUND , cmd_name); goto err_input; }
monitor.h +1 -0
@@ -17,6 +17,7 @@ extern Monitor *default_mon;/* flags for monitor commands */ #define MONITOR_CMD_ASYNC 0x0001 +#define MONITOR_CMD_USER_ONLY 0x0002/* QMP events */ typedef enum MonitorEvent {
Signature · 1 line
1.7.1
PATCH v4 12/23 monitor: Add completion for qdev paths
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Implement monitor command line completion for device tree paths. The first user are device_add ('bus' option) and device_del.
--- hw/qdev.c | 19 +++++---- hw/qdev.h | 3 + monitor.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- qemu-monitor.hx | 4 +- 4 files changed, 129 insertions(+), 12 deletions(-)
hw/qdev.c +10 -9
@@ -39,7 +39,6 @@ DeviceInfo *device_info_list;static BusState *qbus_find_recursive (BusState *bus,const char *name,const BusInfo *info); -static BusState *qbus_find (const char *path,bool report_errors);/* Register a new device type. */ void qdev_register (DeviceInfo *info)
@@ -514,7 +513,7 @@ static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)return qdev_iterate_recursive (bus, find_id_callback, (void *)id); } -static int qdev_instance_no (DeviceState *dev) +int qdev_instance_no (DeviceState *dev) {struct DeviceState *sibling;int instance =0 ;
@@ -611,7 +610,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)return NULL ; } -static BusState *qbus_find (const char *path,bool report_errors) +BusState *qbus_find (const char *path,bool report_errors) {DeviceState *dev;BusState *bus = main_system_bus;
@@ -678,7 +677,7 @@ static BusState *qbus_find(const char *path, bool report_errors) } } -static DeviceState *qdev_find (const char *path) +DeviceState *qdev_find (const char *path,bool report_errors) {const char *dev_name;DeviceState *dev;
@@ -688,7 +687,7 @@ static DeviceState *qdev_find(const char *path)/* search for unique ID recursively if path is not absolute */ if (path[0 ] !='/' ) { dev =qdev_find_id_recursive (main_system_bus, path); -if (!dev) { +if (!dev && report_errors) {qerror_report (QERR_DEVICE_NOT_FOUND , path); }return dev;
@@ -702,8 +701,10 @@ static DeviceState *qdev_find(const char *path) bus =qbus_find (bus_path, false);qemu_free (bus_path);if (!bus) { -/* retry with full path to generate correct error message */ - bus =qbus_find (path, true); +if (report_errors) { +/* retry with full path to generate correct error message */ + bus =qbus_find (path, report_errors); + }if (!bus) {return NULL ; }
@@ -711,7 +712,7 @@ 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); }
@@ -880,7 +881,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)const char *path =qdict_get_str (qdict,"device" );DeviceState *dev; - dev =qdev_find (path); + dev =qdev_find (path, true);if (!dev) {return -1 ; }
hw/qdev.h +3 -0
@@ -171,6 +171,8 @@ CharDriverState *qdev_init_chardev(DeviceState *dev);BusState *qdev_get_parent_bus (DeviceState *dev);void *qdev_iterate_recursive (BusState *bus,qdev_iteratefn callback,void *opaque); +DeviceState *qdev_find (const char *path,bool report_errors); +int qdev_instance_no (DeviceState *dev);/*** BUS API. ***/
@@ -178,6 +180,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,bool report_errors);#define FROM_QBUS (type, dev) DO_UPCAST(type, qbus, dev)
monitor.c +114 -1
156 lines not on this page · raw message
qemu-monitor.hx +2 -2
@@ -660,7 +660,7 @@ ETEXI { .name = "device_add", - .args_type = "device:O(drive:B)", + .args_type = "device:O(bus:q,drive:B)", .params = "driver[,prop=value][,...]", .help = "add device, like -device on the command line", .user_print = monitor_user_noop,
@@ -703,7 +703,7 @@ EQMP { .name = "device_del", - .args_type = "device:s", + .args_type = "device:Q", .params = "device", .help = "remove device", .user_print = monitor_user_noop,
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 12/23] monitor: Add completion for qdev paths
Quoted 123 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
Implement monitor command line completion for device tree paths. The first user are
device_add('bus' option) anddevice_del.Signed-off-by Jan Kiszka <[email protected]>--- hw/qdev.c | 19 +++++---- hw/qdev.h | 3 + monitor.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- qemu-monitor.hx | 4 +- 4 files changed, 129 insertions(+), 12 deletions(-)hw/qdev.c +10
-9@@ -39,7 +39,6 @@ DeviceInfo *device_info_list;static BusState *qbus_find_recursive (BusState *bus,const char *name,const BusInfo *info); -static BusState *qbus_find (const char *path,bool report_errors);/* Register a new device type. */ void qdev_register (DeviceInfo *info)@@ -514,7 +513,7 @@ static DeviceState *qdev_find_id_recursive(BusState *bus, const char *id)return qdev_iterate_recursive (bus, find_id_callback, (void *)id); } -static int qdev_instance_no (DeviceState *dev) +int qdev_instance_no (DeviceState *dev) {struct DeviceState *sibling;int instance =0 ;@@ -611,7 +610,7 @@ static DeviceState *qbus_find_dev(BusState *bus, const char *elem)return NULL ; } -static BusState *qbus_find (const char *path,bool report_errors) +BusState *qbus_find (const char *path,bool report_errors) {DeviceState *dev;BusState *bus = main_system_bus;@@ -678,7 +677,7 @@ static BusState *qbus_find(const char *path, bool report_errors) } } -static DeviceState *qdev_find (const char *path) +DeviceState *qdev_find (const char *path,bool report_errors) {const char *dev_name;DeviceState *dev;@@ -688,7 +687,7 @@ static DeviceState *qdev_find(const char *path)/* search for unique ID recursively if path is not absolute */ if (path[0 ] !='/' ) { dev =qdev_find_id_recursive (main_system_bus, path); -if (!dev) { +if (!dev && report_errors) {qerror_report (QERR_DEVICE_NOT_FOUND , path); }return dev;@@ -702,8 +701,10 @@ static DeviceState *qdev_find(const char *path) bus =qbus_find (bus_path, false);qemu_free (bus_path);if (!bus) { -/* retry with full path to generate correct error message */ - bus =qbus_find (path, true); +if (report_errors) { +/* retry with full path to generate correct error message */ + bus =qbus_find (path, report_errors); + }if (!bus) {return NULL ; }@@ -711,7 +712,7 @@ 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); }@@ -880,7 +881,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data)const char *path =qdict_get_str (qdict,"device" );DeviceState *dev; - dev =qdev_find (path); + dev =qdev_find (path, true);if (!dev) {return -1 ; }hw/qdev.h +3
-0@@ -171,6 +171,8 @@ CharDriverState *qdev_init_chardev(DeviceState *dev);BusState *qdev_get_parent_bus (DeviceState *dev);void *qdev_iterate_recursive (BusState *bus,qdev_iteratefn callback,void *opaque); +DeviceState *qdev_find (const char *path,bool report_errors); +int qdev_instance_no (DeviceState *dev);/*** BUS API. ***/ @@ -178,6 +180,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,bool report_errors);#define FROM_QBUS (type, dev) DO_UPCAST(type, qbus, dev)monitor.c +2
-0@@ -64,12 +64,14 @@* * 'F' filename * 'B' block device name +* 'q' qdev bus path +* 'Q' qdev device path
monitor.c line 77
Quoted 10 lines
monitor.c +1
-1* 's' string (accept optional quote) * 'O' option string of the form NAME=VALUE,... * parsed according to QemuOptsList given by its name * Example: 'device:O' uses qemu_device_opts. * Command completion for specific keys can be requested via -* appending '(NAME:TYPE,...)' with 'F', 'B' as type. +* appending '(NAME:TYPE,...)' with 'F', 'B', 'q', 'Q' as type. * Example: 'device:O(bus:Q)' to expand 'bus=...' as qtree path. * Restriction: only lists with empty desc are supported * TODO lift the restriction
Nitpick: you add "Example: 'device:O(bus:Q)'" in patch 11, before 'Q' gets defined in patch 12.
PATCH v4 13/23 monitor: Allow to specify HMP-specifc command arguments
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
As we may want to shrink or enhance the argument set used for monitor command in HMP mode, add a separate, optional argument string for that case. When an HMP request is parsed, this argument string, if available, takes precedence over the standard string.
--- monitor.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-)
monitor.c +3 -2
@@ -108,6 +108,7 @@ typedef struct mon_cmd_t {const char *params;const char *help;void (*user_print)(Monitor *mon,const QObject *data); +const char *user_args_type;union {void (*info)(Monitor *mon);void (*info_new)(Monitor *mon,QObject **ret_data);
@@ -3310,7 +3311,7 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, }/* parse the parameters */ - typestr = cmd->args_type; + typestr = cmd->user_args_type ? : cmd->args_type;for (;;) { typestr =key_get_info (typestr, &key);if (!typestr)
@@ -4040,7 +4041,7 @@ static void monitor_find_completion(const char *cmdline) goto cleanup; } - ptype =next_arg_type (cmd->args_type); + ptype =next_arg_type (cmd->user_args_type ? : cmd->args_type);for (i =0 ; i < nb_args -2 ; i++) {if (*ptype !='\0' ) { ptype =next_arg_type (ptype);
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 13/23] monitor: Allow to specify HMP-specifc command arguments
Quoted 8 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
As we may want to shrink or enhance the argument set used for monitor command in HMP mode, add a separate, optional argument string for that case. When an HMP request is parsed, this argument string, if available, takes precedence over the standard string.
Signed-off-by Jan Kiszka <[email protected]>
args_type strings are such a crude tool... I fear having two of them just breeds inconsistencies.
There's a big difference between QMP and HMP: QMP has only named parameters, HMP only positional parameters (type 'O' provides named options within a positional parameter). I figure the proper way to deal with this is to define QMP parameters in a data structure (we need that for self-documentation anyway), then define a mapping from HMP positional parameters to QMP named parameters.
PATCH v4 14/23 monitor: return length of printed string via monitor_[v]printf
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
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.
--- monitor.c | 23 +++++++++++++++-------- monitor.h | 4 ++-- qemu-tool.c | 6 ++++-- 3 files changed, 21 insertions(+), 12 deletions(-)
monitor.c +15 -8
@@ -263,29 +263,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)
monitor.h +2 -2
@@ -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);
qemu-tool.c +4 -2
@@ -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)
Signature · 1 line
1.6.0.2
Re: [Qemu-devel] [PATCH v4 14/23] monitor: return length of printed string via monitor_[v]printf
Quoted 7 lines · Jan Kiszka
From: Jan Kiszka <[email protected]>
This simply forwards the result of the internal vsnprintf to the callers of
monitor_printfandmonitor_vprintf. When invoked over a QMP session or in absence of an active monitor, -1 is returned.Signed-off-by Jan Kiszka <[email protected]>
Permits fixing the TODOs in qemu-error.c. Feel free to leave that to me.
PATCH v4 15/23 monitor: Establish cmd flags and convert the async tag
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
As we want to add more flags to monitor commands, convert the only so far existing one accordingly.
--- monitor.c | 6 +++--- monitor.h | 3 +++ qemu-monitor.hx | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-)
monitor.c +3 -3
@@ -118,7 +118,7 @@ typedef struct mon_cmd_t {int (*cmd_async)(Monitor *mon,const QDict *params,MonitorCompletion *cb,void *opaque); } mhandler; -int async; +int flags; } mon_cmd_t;/* file descriptors passed via SCM_RIGHTS */
@@ -340,7 +340,7 @@ static inline int monitor_handler_ported(const mon_cmd_t *cmd)static inline bool monitor_handler_is_async (const mon_cmd_t *cmd) { -return cmd->async !=0 ; +return cmd->flags &MONITOR_CMD_ASYNC ; }static inline int monitor_has_error (const Monitor *mon)
@@ -2544,7 +2544,7 @@ static const mon_cmd_t info_cmds[] = { .help = "show balloon information", .user_print = monitor_print_balloon, .mhandler.info_async = do_info_balloon, - .async = 1, + .flags = MONITOR_CMD_ASYNC, }, { .name = "qtree",
monitor.h +3 -0
@@ -15,6 +15,9 @@ extern Monitor *default_mon;#define MONITOR_USE_READLINE 0x02#define MONITOR_USE_CONTROL 0x04 +/* flags for monitor commands */ +#define MONITOR_CMD_ASYNC 0x0001 +/* QMP events */ typedef enum MonitorEvent { QEVENT_SHUTDOWN,
qemu-monitor.hx +1 -1
@@ -1287,7 +1287,7 @@ ETEXI .help = "request VM to change its memory allocation (in MB)", .user_print = monitor_user_noop, .mhandler.cmd_async = do_balloon, - .async = 1, + .flags = MONITOR_CMD_ASYNC, }, STEXI
Signature · 1 line
1.6.0.2
PATCH v4 16/23 monitor: Allow to exclude commands from QMP
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Ported commands that are marked 'user_only' will not be considered for QMP monitor sessions. This allows to implement new commands that do not (yet) provide a sufficiently stable interface for QMP use (e.g. device_show).
--- monitor.c | 18 +++++++++++++++--- monitor.h | 1 + 2 files changed, 16 insertions(+), 3 deletions(-)
monitor.c +15 -3
@@ -343,6 +343,11 @@ static inline bool monitor_handler_is_async(const mon_cmd_t *cmd)return cmd->flags &MONITOR_CMD_ASYNC ; } +static inline bool monitor_cmd_user_only (const mon_cmd_t *cmd) +{ +return (cmd->flags &MONITOR_CMD_USER_ONLY ); +} +static inline int monitor_has_error (const Monitor *mon) {return mon->error !=NULL ;
@@ -625,6 +630,11 @@ static int do_info(Monitor *mon, const QDict *qdict, QObject **ret_data) goto help; } +if (monitor_ctrl_mode (mon) &&monitor_cmd_user_only (cmd)) { +qerror_report (QERR_COMMAND_NOT_FOUND , item); +return -1 ; + } +if (monitor_handler_is_async (cmd)) {if (monitor_ctrl_mode (mon)) {qmp_async_info_handler (mon, cmd);
@@ -722,13 +732,14 @@ static void do_info_commands(Monitor *mon, QObject **ret_data) cmd_list =qlist_new ();for (cmd = mon_cmds; cmd->name !=NULL ; cmd++) { -if (monitor_handler_ported (cmd) && !compare_cmd (cmd->name,"info" )) { +if (monitor_handler_ported (cmd) && !monitor_cmd_user_only (cmd) && + !compare_cmd (cmd->name,"info" )) {qlist_append_obj (cmd_list,get_cmd_dict (cmd->name)); } }for (cmd = info_cmds; cmd->name !=NULL ; cmd++) { -if (monitor_handler_ported (cmd)) { +if (monitor_handler_ported (cmd) && !monitor_cmd_user_only (cmd)) {char buf[128 ];snprintf (buf,sizeof (buf),"query-%s" , cmd->name);qlist_append_obj (cmd_list,get_cmd_dict (buf));
@@ -4376,7 +4387,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) qobject_from_jsonf("{ 'item': %s }", info_item)); }else { cmd =monitor_find_command (cmd_name); -if (!cmd || !monitor_handler_ported (cmd)) { +if (!cmd || !monitor_handler_ported (cmd) + ||monitor_cmd_user_only (cmd)) {qerror_report (QERR_COMMAND_NOT_FOUND , cmd_name); goto err_input; }
monitor.h +1 -0
@@ -17,6 +17,7 @@ extern Monitor *default_mon;/* flags for monitor commands */ #define MONITOR_CMD_ASYNC 0x0001 +#define MONITOR_CMD_USER_ONLY 0x0002/* QMP events */ typedef enum MonitorEvent {
Signature · 1 line
1.6.0.2
PATCH v4 17/23 Add base64 encoder/decoder
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Will be used by QBuffer.
--- Makefile.objs | 2 +- base64.c | 202 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ base64.h | 19 ++++++ 3 files changed, 222 insertions(+), 1 deletions(-) create mode 100644 base64.c create mode 100644 base64.h
Makefile.objs +1 -1
@@ -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
base64.c new file +202 -0
202 lines not on this page · raw message
base64.h new file +19 -0
@@ -0,0 +1,19 @@ +/* +* 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 <[email protected]> +* +* 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 <stddef.h> + +void base64_encode (const uint8_t *src,size_t srclen,char *dest); +int base64_decode (const char *src,size_t srclen,uint8_t *dest);
Signature · 1 line
1.6.0.2
PATCH v4 18/23 QMP: Reserve namespace for complex object classes
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This reserves JSON objects that contain the key '__class__' for QMP-specific complex objects. First user will be the buffer class.
--- QMP/qmp-spec.txt | 16 +++++++++++++--- 1 files changed, 13 insertions(+), 3 deletions(-)
QMP/qmp-spec.txt +13 -3
@@ -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
Signature · 1 line
1.6.0.2
PATCH v4 19/23 QMP: Add QBuffer
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
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.
--- 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
Makefile +3 -2
@@ -146,14 +146,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
Makefile.objs +1 -1
@@ -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
QMP/qmp-spec.txt +9 -1
@@ -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 ===============
check-qbuffer.c new file +172 -0
172 lines not on this page · raw message
configure +1 -1
@@ -2008,7 +2008,7 @@ if test "$softmmu" = yes ; 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
qbuffer.c new file +116 -0
116 lines not on this page · raw message
qbuffer.h new file +33 -0
@@ -0,0 +1,33 @@ +/* +* QBuffer Module +* +* Copyright (C) 2010 Siemens AG +* +* Authors: +* Jan Kiszka <[email protected]> +* +* 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 */
qjson.c +15 -0
@@ -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 {
@@ -238,6 +240,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 :
qobject.h +1 -0
@@ -44,6 +44,7 @@ typedef enum {QTYPE_QFLOAT ,QTYPE_QBOOL ,QTYPE_QERROR , +QTYPE_QBUFFER , } qtype_code;struct QObject ;
Signature · 1 line
1.6.0.2
PATCH v4 20/23 monitor: Add basic device state visualization
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This introduces device_show, a monitor command that saves the vmstate of a qdev device and visualizes it. 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. QMP is not supported as we cannot provide a stable interface, at least at this point.
--- hw/hw.h | 2 + hw/qdev.c | 248 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/qdev.h | 2 + qemu-monitor.hx | 20 +++++ qerror.c | 4 + qerror.h | 3 + 6 files changed, 279 insertions(+), 0 deletions(-)
hw/hw.h +2 -0
@@ -298,6 +298,7 @@ enum VMStateFlags { typedef struct { const char *name; + const char *start_index; size_t offset; size_t size; size_t start;
@@ -412,6 +413,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) {\
hw/qdev.c +248 -0
257 lines not on this page · raw message
hw/qdev.h +2 -0
@@ -190,6 +190,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 ***/
qemu-monitor.hx +20 -0
@@ -734,6 +734,26 @@ Example: EQMP { + .name = "device_show", + .args_type = "path:Q", + .user_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, + .flags = MONITOR_CMD_USER_ONLY, + }, + +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 + + { .name = "cpu", .args_type = "index:i", .params = "index",
qerror.c +4 -0
@@ -97,6 +97,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)", },
qerror.h +3 -0
@@ -88,6 +88,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 } }"
Signature · 1 line
1.6.0.2
PATCH v4 21/23 QMP: Teach basic capability negotiation to python example
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
As sending "qmp_capabilities" on session start became mandatory, both python examples were broken.
--- QMP/qmp-shell | 1 + QMP/vm-info | 1 + 2 files changed, 2 insertions(+), 0 deletions(-)
QMP/qmp-shell +1 -0
@@ -42,6 +42,7 @@ def main(): qemu = qmp.QEMUMonitorProtocol(argv[1]) qemu.connect() + qemu.send("qmp_capabilities") print 'Connected!'
QMP/vm-info +1 -0
@@ -24,6 +24,7 @@ def main(): qemu = qmp.QEMUMonitorProtocol(argv[1]) qemu.connect() + qemu.send("qmp_capabilities") for cmd in [ 'version', 'kvm', 'status', 'uuid', 'balloon' ]: print cmd + ': ' + str(qemu.send('query-' + cmd))
Signature · 1 line
1.6.0.2
PATCH v4 22/23 QMP: Fix python helper /wrt long return strings
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
Remove the arbitrary limitation of 1024 characters per return string and read complete lines instead. Required for device_show.
--- QMP/qmp.py | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
QMP/qmp.py +5 -1
@@ -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()
Signature · 1 line
1.6.0.2
PATCH v4 23/23 QMP: Add support for buffer class to qmp python helper
In reply to Jan KiszkaFrom: Jan Kiszka <[email protected]>
This demonstrates the conversion of QMP buffer objects and does some minimalistic pretty-printing.
--- QMP/qmp.py | 25 +++++++++++++++++++++++-- 1 files changed, 23 insertions(+), 2 deletions(-)
QMP/qmp.py +23 -2
@@ -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)): + 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:
Signature · 1 line
1.6.0.2
Quoted 2 lines · Jan Kiszka
This is v4 of this series. Besides small fixes, it's main focus is on the groundwork for the visualization command: qdev path usability.
Don't let my griping on individual patches mislead you: there's lots of good stuff in here.