| Message ID | fce0e55f5bf02172957365f5674e89a4d64257e4.1276641524.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> > > 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 <jan.kiszka@siemens.com> 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.
Markus Armbruster wrote: > [cc: kraxel] > > Jan Kiszka <jan.kiszka@web.de> writes: > >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> 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 <jan.kiszka@siemens.com> > > 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. [ you already found how this is resolved ] > > 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
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> [cc: kraxel] >> >> Jan Kiszka <jan.kiszka@web.de> writes: >> >>> From: Jan Kiszka <jan.kiszka@siemens.com> >>> >>> 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 <jan.kiszka@siemens.com> >> >> 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. > > [ 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. >> 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.
Markus Armbruster wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> Markus Armbruster wrote: >>> [cc: kraxel] >>> >>> Jan Kiszka <jan.kiszka@web.de> writes: >>> >>>> From: Jan Kiszka <jan.kiszka@siemens.com> >>>> >>>> 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 <jan.kiszka@siemens.com> >>> 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. >> [ 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. > >>> 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 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. > > 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
diff --git a/hw/qdev.c b/hw/qdev.c index c272c51..aa25155 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -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;