| Message ID | 42cafcab8804bcabac400dc4104fbb3da2d1aed4.1276641524.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
Jan Kiszka <jan.kiszka@web.de> writes: > From: Jan Kiszka <jan.kiszka@siemens.com> > > 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 <jan.kiszka@siemens.com> 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.
Markus Armbruster wrote: > Jan Kiszka <jan.kiszka@web.de> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> 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 <jan.kiszka@siemens.com> > > 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. 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
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> Jan Kiszka <jan.kiszka@web.de> writes: >> >>> From: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> 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 <jan.kiszka@siemens.com> >> >> 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. > > 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.
diff --git a/hw/qdev.c b/hw/qdev.c index 7c4f039..c272c51 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -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 (;;) {