| Message ID | 913f4f6a67305a72ed3d994bd12d5b34de9b9bb9.1273843151.git.jan.kiszka@siemens.com |
|---|---|
| State | New |
| Headers | show |
Jan Kiszka <jan.kiszka@siemens.com> writes: > Extend qbus_find_dev to allow addressing of devices without an unique id > via an optional instance number. The new formats are 'driver.instance' > and 'alias.instance'. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> How's the instance number defined? Should be documented. Is an instance number a good user interface? Gerd, what do you think? [...]
On 05/18/10 14:15, Markus Armbruster wrote: > Jan Kiszka<jan.kiszka@siemens.com> writes: > >> Extend qbus_find_dev to allow addressing of devices without an unique id >> via an optional instance number. The new formats are 'driver.instance' >> and 'alias.instance'. >> >> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > > How's the instance number defined? Should be documented. savevm instance id, used to identify multiple instances of some device on loadvm. By default is just incrementing (0,1,2,...) for each new device instance I think. Drivers can also specify one. Most don't do that. IIRC some ISA drivers use the base ioport as instance id, which sort-of makes sense as it makes sure the id identifies the correct device no matter what the initialization order is. It probably makes sense to replace the instance id with the device path once all devices are converted over to qdev+vmstate, so we avoid the nasty ordering issues altogether. cheers, Gerd
Gerd Hoffmann <kraxel@redhat.com> wrote: > On 05/18/10 14:15, Markus Armbruster wrote: >> Jan Kiszka<jan.kiszka@siemens.com> writes: >> >>> Extend qbus_find_dev to allow addressing of devices without an unique id >>> via an optional instance number. The new formats are 'driver.instance' >>> and 'alias.instance'. >>> >>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >> >> How's the instance number defined? Should be documented. > > savevm instance id, used to identify multiple instances of some device > on loadvm. By default is just incrementing (0,1,2,...) for each new > device instance I think. Drivers can also specify one. Most don't do > that. IIRC some ISA drivers use the base ioport as instance id, which > sort-of makes sense as it makes sure the id identifies the correct > device no matter what the initialization order is. > > It probably makes sense to replace the instance id with the device > path once all devices are converted over to qdev+vmstate, so we avoid > the nasty ordering issues altogether. Agreed. The problem here is that we sent the instance_id on the wire, so for "legacy" devices that used an instance_id != -1, we are stuck with it :( Guess why a friend calls "backwards compatibility": A Curse of Bible proportions :( Later, Juan.
Hi, > Agreed. The problem here is that we sent the instance_id on the wire, > so for "legacy" devices that used an instance_id != -1, we are stuck > with it :( Another way would be to fill the instance id with something useful, say encode the pci address there for pci devices. That should be good enough to reliably identify devices even after hot-plugging them in and out. Of course this has transition isses too. But maybe it is easier to handle than replacing instance_id with something entirely different. And it also makes the instance id less useful to address devices in a human-friendly way. cheers, Gerd
Gerd Hoffmann wrote: > On 05/18/10 14:15, Markus Armbruster wrote: >> Jan Kiszka<jan.kiszka@siemens.com> writes: >> >>> Extend qbus_find_dev to allow addressing of devices without an unique id >>> via an optional instance number. The new formats are 'driver.instance' >>> and 'alias.instance'. >>> >>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >> >> How's the instance number defined? Should be documented. OK, will do. For now it is the per-bus instance ID. I think that makes most sense and is easily handleable. Still, we should probably print it also via "info qtree" (and a future "query-qtree"). > > savevm instance id, used to identify multiple instances of some device > on loadvm. By default is just incrementing (0,1,2,...) for each new > device instance I think. Drivers can also specify one. Most don't do > that. IIRC some ISA drivers use the base ioport as instance id, which > sort-of makes sense as it makes sure the id identifies the correct > device no matter what the initialization order is. That io-address-based instance numbers have just been deprecated, see 4d2ffa08b601bdd40d9ccf225480c0a7e90ca078. ISA devices are already converted, there are just a few non-PC devices remaining that don't use the (auto-generated) vmstate instance number. But that is actually a different, user-invisible numbering scheme. > > It probably makes sense to replace the instance id with the device path > once all devices are converted over to qdev+vmstate, so we avoid the > nasty ordering issues altogether. You are always free to address devices via a unique user-defined ID. Jan
On 05/18/2010 03:31 PM, Gerd Hoffmann wrote: > On 05/18/10 14:15, Markus Armbruster wrote: >> Jan Kiszka<jan.kiszka@siemens.com> writes: >> >>> Extend qbus_find_dev to allow addressing of devices without an >>> unique id >>> via an optional instance number. The new formats are 'driver.instance' >>> and 'alias.instance'. >>> >>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> >> >> How's the instance number defined? Should be documented. > > savevm instance id, used to identify multiple instances of some device > on loadvm. By default is just incrementing (0,1,2,...) for each new > device instance I think. That's an implementation detail that's being exposed in an external interface. Granted, users shouldn't expect this to remain stable across invocations, yet it makes me uncomfortable. Why not always use topology to locate devices? > Drivers can also specify one. Most don't do that. IIRC some ISA > drivers use the base ioport as instance id, which sort-of makes sense > as it makes sure the id identifies the correct device no matter what > the initialization order is. > > It probably makes sense to replace the instance id with the device > path once all devices are converted over to qdev+vmstate, so we avoid > the nasty ordering issues altogether. Oh, that's what you're suggesting. So we agree.
diff --git a/hw/qdev.c b/hw/qdev.c index d3bf0fa..fe49e71 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -515,28 +515,41 @@ static BusState *qbus_find_bus(DeviceState *dev, char *elem) return NULL; } -static DeviceState *qbus_find_dev(BusState *bus, char *elem) +static DeviceState *qbus_find_dev(BusState *bus, const char *elem) { DeviceState *dev; + int instance, n; + char buf[128]; /* * try to match in order: * (1) instance id, if present - * (2) driver name - * (3) driver alias, if present + * (2) driver name [.instance] + * (3) driver alias [.instance], if present */ QLIST_FOREACH(dev, &bus->children, sibling) { if (dev->id && strcmp(dev->id, elem) == 0) { return dev; } } + + if (sscanf(elem, "%127[^.].%u", buf, &instance) == 2) { + elem = buf; + } else { + instance = 0; + } + + n = 0; QLIST_FOREACH(dev, &bus->children, sibling) { - if (strcmp(dev->info->name, elem) == 0) { + if (strcmp(dev->info->name, elem) == 0 && n++ == instance) { return dev; } } + + n = 0; QLIST_FOREACH(dev, &bus->children, sibling) { - if (dev->info->alias && strcmp(dev->info->alias, elem) == 0) { + if (dev->info->alias && strcmp(dev->info->alias, elem) == 0 && + n++ == instance) { return dev; } }
Extend qbus_find_dev to allow addressing of devices without an unique id via an optional instance number. The new formats are 'driver.instance' and 'alias.instance'. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- hw/qdev.c | 23 ++++++++++++++++++----- 1 files changed, 18 insertions(+), 5 deletions(-)