| Message ID | c375fba941d2c62025d5e60a85d4db2a301e30b1.1274516288.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
[cc: kraxel] Jan Kiszka <jan.kiszka@web.de> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to > make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as > there is only one child bus per device. We auto-root a path not starting with '/' via convention "first component is ID then" (I like that). We auto-complete a path ending with a device when we want a bus (a bit too clever). Auto-inserting bus names in the middle is too clever by half! Such cleverness can be convenient in the human monitor, but all it adds to QMP is complexity. I'm worried about possibly ambigous interpretation of paths. Would be bad if a path could name different node after we add something to the tree. I *think* your fine print makes that impossible, but it's not exactly obvious. Heck, I could be wrong. Wouldn't it be better to put the convenience into the interactive completion function? That way we keep it out of QMP. Subject is misleading, it's a feature, not a bug fix.
Markus Armbruster wrote: > [cc: kraxel] > > Jan Kiszka <jan.kiszka@web.de> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to >> make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as >> there is only one child bus per device. > > We auto-root a path not starting with '/' via convention "first > component is ID then" (I like that). We auto-complete a path ending > with a device when we want a bus (a bit too clever). Auto-inserting bus > names in the middle is too clever by half! > > Such cleverness can be convenient in the human monitor, but all it adds > to QMP is complexity. > > I'm worried about possibly ambigous interpretation of paths. Would be > bad if a path could name different node after we add something to the > tree. I *think* your fine print makes that impossible, but it's not > exactly obvious. Heck, I could be wrong. For the above example, that would mean a bus called 'dev2' would have to be added to dev1. If that makes sense is one question. The other is why this should be more broken than the case that this happens to a bus specified at the end of some path? > > Wouldn't it be better to put the convenience into the interactive > completion function? That way we keep it out of QMP. That would leave user interfaces over QMP broken behind, or at least complicate their implementations as they would have to expand the qtree path on their own to achieve consistency. > > Subject is misleading, it's a feature, not a bug fix. Depends on the POV. For me it is a highly confusing inconsistency in the way you can specify qtree paths. IMHO, we either have to drop this path abbreviation or apply it to the whole path. As it's intention is (according to my understanding) to ease the usage, an unintuitive construction role is fairly counterproductive. Jan
On 05/29/10 09:38, Markus Armbruster wrote: > [cc: kraxel] > > Jan Kiszka<jan.kiszka@web.de> writes: > >> From: Jan Kiszka<jan.kiszka@siemens.com> >> >> As long as we allow /dev.1 as shortcut for /dev1/bus1, we also have to >> make sure that /dev1/dev2 works for /dev1/bus1/dev2/bus2 - as long as >> there is only one child bus per device. > > We auto-root a path not starting with '/' via convention "first > component is ID then" (I like that). We auto-complete a path ending > with a device when we want a bus (a bit too clever). ... only if there is a single child bus only (i.e. it isn't ambiguous). Which is the common case though. > Auto-inserting bus > names in the middle is too clever by half! Yea. I have to agree here. It can only work reliably if you make sure all your devices have unique ids. In which case there is no need to reference dev1 at all, you can just use the "first component is ID then" mechanism for dev2. cheers, Gerd
diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt index f252c8e..9ac1fa1 100644 --- a/docs/qdev-device-use.txt +++ b/docs/qdev-device-use.txt @@ -20,6 +20,10 @@ bus named pci.0. To put a FOO device into its slot 4, use -device FOO,bus=/i440FX-pcihost/pci.0,addr=4. The abbreviated form bus=pci.0 also works as long as the bus name is unique. +Furthermore, if a device only hosts a single bus, the bus name can be +omitted in the path. Example: /i440FX-pcihost/PIIX3 abbreviates +/i440FX-pcihost/pci.0/PIIX3/isa.0 as none of the buses has siblings. + Note: the USB device address can't be controlled at this time. === Block Devices === diff --git a/hw/qdev.c b/hw/qdev.c index aa2ce01..2e50531 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -557,7 +557,7 @@ static DeviceState *qbus_find_dev(BusState *bus, char *elem) static BusState *qbus_find(const char *path) { - DeviceState *dev; + DeviceState *dev, *next_dev; BusState *bus; char elem[128]; int pos, len; @@ -603,6 +603,7 @@ static BusState *qbus_find(const char *path) return NULL; } +search_dev_bus: assert(path[pos] == '/' || !path[pos]); while (path[pos] == '/') { pos++; @@ -633,6 +634,15 @@ static BusState *qbus_find(const char *path) pos += len; bus = qbus_find_bus(dev, elem); if (!bus) { + if (dev->num_child_bus == 1) { + /* Last element might have been a short-cut to a device on + * the single child bus of the parent device. */ + next_dev = qbus_find_dev(QTAILQ_FIRST(&dev->child_bus), elem); + if (next_dev) { + dev = next_dev; + goto search_dev_bus; + } + } qerror_report(QERR_BUS_NOT_FOUND, elem); if (!monitor_cur_is_qmp()) { qbus_list_bus(dev);