Alex Williamson wrote: > On Mon, 2010-06-14 at 18:00 +0200, Jan Kiszka wrote: >> Alex Williamson wrote: >>> On Mon, 2010-06-14 at 14:09 +0100, Paul Brook wrote: >>>>>>> "/main-system-bus/pci.0,addr=09.0/virtio-blk-pci" >>>> There's a device missing between the main system bus and the pci bus. Should >>>> be something like: >>>> >>>> /main-system-bus/piix4-pcihost/pci.0/_09.0 >>> Ok, I can easily come up with: >>> >>> /System/main-system-bus/i440FX-pcihost/PCI/pci.0/_09.0/virtio-blk-pci/virtio-blk >> First two elements are redundant, '/' already stands for the main system >> bus. > > Ok, we can start printing after the system bus. > >> Then I don't get what last element expresses (the target device is >> virtio-blk-pci). Is this due to some vmstate layout? But that should not >> be part into qdev paths (maybe I'm missing your use case). > > Sorry, I wasn't clear about that. My printf is in the savevm code, > after it's already appended the idstr passed in from the device. The > device path in the example above ends at virtio-blk-pci. That's the > dev->info->name of the device passed into this function. > >> And instead of introducing another hierarchy level with the bus address, >> I would also prefer to add this as prefix or suffix to the device name, >> e.g. <driver>.<busaddr>. > > That's what I had started with. The first post in this thread has > "pci.0,addr=09.0" as a single hierarchy level. The "addr=" may be > unnecessary there, but I also prefer something along those lines. For > PCI it'd make sense to have <name>:<addr>, which comes out to > "pci.0:09.0". (Maybe rather than flagging properties as being relevant > to the path and printing them generically, we should extract specific > properties based on the bus type.) Not bus.addr, driver.addr. We only have one PCI bus here, not as many as there are slots on that bus. > >>>> For busses that don't have a consistent addressing scheme then some sort of >>>> instance ID is unavoidable. I guess it may be possible to invent something >>>> based on other device properties (e.g. address of the first IO port/memory >>>> region). >>> Instance IDs aren't always bad, we just run into trouble with hotplug >>> and maybe creating unique ramblock names. But, such busses typically >>> don't support hotplug and don't have multiple instances of the same >>> device on the bus, so I don't expect us to hit many issues there as long >>> as we can get a stable address path. Thanks, >>> >> If stable instance numbers are required, we could simply keep them in >> DeviceState and search for the smallest free one on additions. Actually, >> I'm more in favor of this direction than including the bus address. That >> way we could keep a canonical format across all buses and do not have to >> provide possibly complex ID generation rules for each of them. > > I started down that path, but it still breaks for hotplug. If we start > a VM with two e1000 NICs, then remove the first, we can no longer > migrate because the target can't represent having a single e1000 with a > non-zero instance ID. That's indeed a good point. Still, I'm worried about having to define numbering schemes for all the buses qemu supports. Maybe we can run a mixture: address-based for hotplug-capably buses (they tend to be cooperative in this regard) and simple instance numbers for the rest, likely the majority. > >> BTW, the problem isn't truly solved by printing paths. We also need to >> parse them. It would be counterproductive if such paths ever see the >> light of day (ie. "leak" outside vmstate) and a user tries to hack it >> into the monitor or pass it on the command line. With my series, I'm >> currently able to process paths like this one: >> >> /i440FX-pcihost.0/pci.0/PIIX4_PM.0/i2c/smbus-eeprom.6 > > Yes, these are only intended for internal use, but we should get them to > sync up as much as possible. Thanks, Unless there is a good reason, the match should be 100%. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
