From: Jan Kiszka <jan.kiszka@siemens.com> To: Alex Williamson <alex.williamson@redhat.com> Cc: Paul Brook <paul@codesourcery.com>, Markus Armbruster <armbru@redhat.com>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, "chrisw@redhat.com" <chrisw@redhat.com>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "kraxel@redhat.com" <kraxel@redhat.com>, "avi@redhat.com" <avi@redhat.com> Subject: Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path() Date: Mon, 14 Jun 2010 18:49:41 +0200 [thread overview] Message-ID: <4C165DA5.70105@siemens.com> (raw) In-Reply-To: <1276533496.12015.191.camel@x201> 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
WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@siemens.com> To: Alex Williamson <alex.williamson@redhat.com> Cc: "chrisw@redhat.com" <chrisw@redhat.com>, "kvm@vger.kernel.org" <kvm@vger.kernel.org>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>, Markus Armbruster <armbru@redhat.com>, "kraxel@redhat.com" <kraxel@redhat.com>, "avi@redhat.com" <avi@redhat.com>, Paul Brook <paul@codesourcery.com> Subject: Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path() Date: Mon, 14 Jun 2010 18:49:41 +0200 [thread overview] Message-ID: <4C165DA5.70105@siemens.com> (raw) In-Reply-To: <1276533496.12015.191.camel@x201> 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
next prev parent reply other threads:[~2010-06-14 16:50 UTC|newest] Thread overview: 160+ messages / expand[flat|nested] mbox.gz Atom feed top 2010-06-14 5:51 [RFC PATCH 0/5] Introduce canonical device hierarchy string Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 5:51 ` [RFC PATCH 1/5] qdev: Create qdev_get_dev_path() Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 6:39 ` Markus Armbruster 2010-06-14 6:39 ` Markus Armbruster 2010-06-14 12:52 ` Alex Williamson 2010-06-14 12:52 ` Alex Williamson 2010-06-14 13:00 ` Jan Kiszka 2010-06-14 13:00 ` Jan Kiszka 2010-06-14 13:09 ` Paul Brook 2010-06-14 13:09 ` Paul Brook 2010-06-14 15:29 ` Alex Williamson 2010-06-14 15:29 ` Alex Williamson 2010-06-14 15:42 ` Paul Brook 2010-06-14 15:42 ` Paul Brook 2010-06-14 16:00 ` Jan Kiszka 2010-06-14 16:00 ` Jan Kiszka 2010-06-14 16:38 ` Alex Williamson 2010-06-14 16:38 ` Alex Williamson 2010-06-14 16:49 ` Jan Kiszka [this message] 2010-06-14 16:49 ` Jan Kiszka 2010-06-14 18:35 ` Alex Williamson 2010-06-14 18:35 ` Alex Williamson 2010-06-14 21:43 ` Paul Brook 2010-06-14 21:43 ` Paul Brook 2010-06-14 22:11 ` Alex Williamson 2010-06-14 22:11 ` Alex Williamson 2010-06-14 22:46 ` Paul Brook 2010-06-14 22:46 ` Paul Brook 2010-06-15 1:14 ` Alex Williamson 2010-06-15 1:14 ` Alex Williamson 2010-06-15 11:24 ` Paul Brook 2010-06-15 11:24 ` Paul Brook 2010-06-15 8:47 ` Markus Armbruster 2010-06-15 8:47 ` Markus Armbruster 2010-06-15 9:34 ` Jan Kiszka 2010-06-15 9:34 ` Jan Kiszka 2010-06-15 11:28 ` Paul Brook 2010-06-15 11:28 ` Paul Brook 2010-06-15 11:45 ` Jan Kiszka 2010-06-15 11:45 ` Jan Kiszka 2010-06-15 12:04 ` Paul Brook 2010-06-15 12:04 ` Paul Brook 2010-06-15 12:16 ` Jan Kiszka 2010-06-15 12:16 ` Jan Kiszka 2010-06-15 12:39 ` Paul Brook 2010-06-15 12:39 ` Paul Brook 2010-06-15 13:00 ` Jan Kiszka 2010-06-15 13:00 ` Jan Kiszka 2010-06-15 13:14 ` Paul Brook 2010-06-15 13:14 ` Paul Brook 2010-06-15 13:16 ` Markus Armbruster 2010-06-15 13:16 ` Markus Armbruster 2010-06-15 13:32 ` Jan Kiszka 2010-06-15 13:32 ` Jan Kiszka 2010-06-15 20:53 ` Alex Williamson 2010-06-15 20:53 ` Alex Williamson 2010-06-15 21:55 ` Paul Brook 2010-06-15 21:55 ` Paul Brook 2010-06-15 22:33 ` Alex Williamson 2010-06-15 22:33 ` Alex Williamson 2010-06-15 23:01 ` Paul Brook 2010-06-15 23:01 ` Paul Brook 2010-06-15 23:10 ` Alex Williamson 2010-06-15 23:10 ` Alex Williamson 2010-06-16 0:25 ` Chris Wright 2010-06-16 0:25 ` Chris Wright 2010-06-16 0:30 ` Paul Brook 2010-06-16 0:30 ` Paul Brook 2010-06-16 0:35 ` Chris Wright 2010-06-16 0:35 ` Chris Wright 2010-06-16 1:30 ` Paul Brook 2010-06-16 1:30 ` Paul Brook 2010-06-16 2:55 ` Alex Williamson 2010-06-16 2:55 ` Alex Williamson 2010-06-16 8:23 ` Markus Armbruster 2010-06-16 8:23 ` Markus Armbruster 2010-06-17 22:25 ` Alex Williamson 2010-06-17 22:25 ` Alex Williamson 2010-06-18 9:16 ` Jan Kiszka 2010-06-18 9:16 ` Jan Kiszka 2010-06-18 15:01 ` Alex Williamson 2010-06-18 15:01 ` Alex Williamson 2010-06-18 15:22 ` Jan Kiszka 2010-06-18 15:22 ` Jan Kiszka 2010-06-18 14:03 ` Markus Armbruster 2010-06-18 14:03 ` Markus Armbruster 2010-06-18 14:14 ` Jan Kiszka 2010-06-18 14:14 ` Jan Kiszka 2010-06-18 15:21 ` Alex Williamson 2010-06-18 15:21 ` Alex Williamson 2010-06-15 11:42 ` Markus Armbruster 2010-06-15 11:42 ` Markus Armbruster 2010-06-15 11:59 ` Jan Kiszka 2010-06-15 11:59 ` Jan Kiszka 2010-06-15 13:07 ` Markus Armbruster 2010-06-15 13:07 ` Markus Armbruster 2010-06-15 13:19 ` Paul Brook 2010-06-15 13:19 ` Paul Brook 2010-06-15 13:32 ` Paul Brook 2010-06-15 13:32 ` Paul Brook 2010-06-15 15:08 ` Jan Kiszka 2010-06-15 15:08 ` Jan Kiszka 2010-06-16 13:02 ` Markus Armbruster 2010-06-16 13:02 ` Markus Armbruster 2010-06-14 5:51 ` [RFC PATCH 2/5] savevm: Add DeviceState param Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 5:51 ` [RFC PATCH 3/5] savevm: Make use of the new " Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 5:51 ` [RFC PATCH 4/5] eepro100: Add a dev field to eeprom new/free functions Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 5:51 ` [RFC PATCH 5/5] virtio-net: Incorporate a DeviceState pointer and let savevm track instances Alex Williamson 2010-06-14 5:51 ` [Qemu-devel] " Alex Williamson 2010-06-14 7:02 ` [RFC PATCH 0/5] Introduce canonical device hierarchy string Gerd Hoffmann 2010-06-14 7:02 ` [Qemu-devel] " Gerd Hoffmann 2010-06-14 19:56 ` Alex Williamson 2010-06-14 19:56 ` [Qemu-devel] " Alex Williamson 2010-06-15 8:53 ` Markus Armbruster 2010-06-15 8:53 ` Markus Armbruster 2010-06-15 18:01 ` Alex Williamson 2010-06-15 18:01 ` Alex Williamson 2010-06-16 8:34 ` Markus Armbruster 2010-06-16 8:36 ` Markus Armbruster 2010-06-15 9:12 ` Gerd Hoffmann 2010-06-15 9:12 ` [Qemu-devel] " Gerd Hoffmann 2010-06-15 18:03 ` Alex Williamson 2010-06-15 18:03 ` [Qemu-devel] " Alex Williamson 2010-06-16 9:46 ` RFC qdev path semantics (was: [Qemu-devel] [RFC PATCH 0/5] Introduce canonical device hierarchy string) Markus Armbruster 2010-06-16 9:46 ` Markus Armbruster 2010-06-16 10:40 ` Paul Brook 2010-06-16 10:40 ` Paul Brook 2010-06-16 11:37 ` RFC qdev path semantics Jan Kiszka 2010-06-16 11:37 ` [Qemu-devel] " Jan Kiszka 2010-06-16 11:45 ` Paul Brook 2010-06-16 11:45 ` [Qemu-devel] " Paul Brook 2010-06-16 12:01 ` Jan Kiszka 2010-06-16 12:01 ` [Qemu-devel] " Jan Kiszka 2010-06-16 12:21 ` Paul Brook 2010-06-16 12:21 ` Paul Brook 2010-06-16 13:50 ` Jan Kiszka 2010-06-16 13:50 ` Jan Kiszka 2010-06-16 13:05 ` Markus Armbruster 2010-06-16 13:05 ` [Qemu-devel] " Markus Armbruster 2010-06-16 13:23 ` Paul Brook 2010-06-16 13:23 ` [Qemu-devel] " Paul Brook 2010-06-16 14:31 ` Markus Armbruster 2010-06-16 14:31 ` Markus Armbruster 2010-06-17 21:43 ` Alex Williamson 2010-06-17 21:43 ` [Qemu-devel] " Alex Williamson 2010-06-17 22:01 ` Paul Brook 2010-06-17 22:01 ` [Qemu-devel] " Paul Brook 2010-06-17 22:34 ` Alex Williamson 2010-06-17 22:34 ` [Qemu-devel] " Alex Williamson 2010-06-18 7:52 ` Gerd Hoffmann 2010-06-18 7:52 ` [Qemu-devel] " Gerd Hoffmann 2010-06-18 14:58 ` Markus Armbruster 2010-06-18 14:58 ` [Qemu-devel] " Markus Armbruster 2010-06-22 14:27 ` Anthony Liguori 2010-06-22 14:27 ` [Qemu-devel] " Anthony Liguori
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=4C165DA5.70105@siemens.com \ --to=jan.kiszka@siemens.com \ --cc=alex.williamson@redhat.com \ --cc=armbru@redhat.com \ --cc=avi@redhat.com \ --cc=chrisw@redhat.com \ --cc=kraxel@redhat.com \ --cc=kvm@vger.kernel.org \ --cc=paul@codesourcery.com \ --cc=qemu-devel@nongnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.