From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=46637 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OOCr9-0004N4-AQ for qemu-devel@nongnu.org; Mon, 14 Jun 2010 12:49:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OOCr8-0006T6-05 for qemu-devel@nongnu.org; Mon, 14 Jun 2010 12:49:51 -0400 Received: from thoth.sbs.de ([192.35.17.2]:21471) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OOCr7-0006Sp-Ml for qemu-devel@nongnu.org; Mon, 14 Jun 2010 12:49:49 -0400 Message-ID: <4C165DA5.70105@siemens.com> Date: Mon, 14 Jun 2010 18:49:41 +0200 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path() References: <20100614054923.879.33717.stgit@localhost.localdomain> <1276519930.12015.104.camel@x201> <201006141409.08653.paul@codesourcery.com> <1276529350.12015.136.camel@x201> <4C16522F.3020006@siemens.com> <1276533496.12015.191.camel@x201> In-Reply-To: <1276533496.12015.191.camel@x201> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: "chrisw@redhat.com" , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , Markus Armbruster , "kraxel@redhat.com" , "avi@redhat.com" , Paul Brook 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. .. > > 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 :, 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