qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: "chrisw@redhat.com" <chrisw@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Paul Brook <paul@codesourcery.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	"avi@redhat.com" <avi@redhat.com>,
	"kraxel@redhat.com" <kraxel@redhat.com>
Subject: Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Date: Tue, 15 Jun 2010 13:59:30 +0200	[thread overview]
Message-ID: <4C176B22.30201@siemens.com> (raw)
In-Reply-To: <m38w6gy121.fsf@blackfin.pond.sub.org>

Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> Markus Armbruster wrote:
>>> Paul Brook <paul@codesourcery.com> writes:
>>>
>>>> Alex Williamson <alex.williamson@redhat.com> writes:
>>>>
>>>>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote:
>>>>>> Could you explain why you add "identified properties of the immediate
>>>>>> parent bus and device"?  They make the result ver much *not* a "dev
>>>>>> path" in the qdev sense...
>>>>> In order to try to get a unique string.  Without looking into device
>>>>> properties, two e1000s would both be:
>>>>>
>>>>> /main-system-bus/pci.0/e1000
>>>>> /main-system-bus/pci.0/e1000
>>>>>
>>>>> Which is no better than simply "e1000" and would require us to fall back
>>>>> to instance ids again.  The goal here is that anything that makes use of
>>>>> passing a dev when registering a vmstate gets an instance id of zero.
>>>> You already got the information you need, you just put it in the wrong place. 
>>>> The canonical ID for the device could be its bus address. In practice we'd 
>>>> probably want to allow the user to specify it by name, provided these are 
>>>> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would 
>>>> as an aias for [...]:_09.0. Device names have a restricted namespace, so we 
>>>> can use an initial prefix to disambiguate a name/label from a bus address.
>>>>
>>>> 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).
>>> When that's inconvenient or impossible, we can still punt to user: make
>>> device ID mandatory.
>> No option due to auto-created devices. And auto-generating IDs would
>> just create usability issues.
> 
> Auto-generated IDs would become part of the ABI.  Really so bad that
> it's "no option"?  Mind, device ID becomes mandatory *only* for devices
> that don't have a useful bus address.  We could even waive the ID
> requirement for the first device of a kind, i.e. require ID if and only
> if it's needed to disambiguate.

IDs are there to find devices the user (or a higher level tool) passed
to QEMU, qtree paths allow to locate _every_ device in a VM, and that in
a well-organized hierarchy. That allows to explore and address a qtree
element at the same time.

> 
>>> We obviously need a way to unambigously name a device.  It's okay to
>>> have multiple names for the same device.
>>>
>>> If the device has a device ID, that's an unambigous name.
>>>
>>> qdev paths may be ambigous when path components are resolved to driver
>>> names instead of IDs.
>>>
>>> Alex proposed to disambiguate by adding "identified properties of the
>>> immediate parent bus and device" to the path component.  For PCI, these
>>> are dev.fn.  Likewise for any other bus where devices have unambigous
>>> bus address.  The driver name carries no information!
>> >From user POV, driver names are very handly to address a device
>> intuitively - except for the case you have tones of devices on the same
>> bus that are handled by the same driver. For that case we need to
>> augment the device name with a useful per-bus ID, derived from the bus
>> address where available, otherwise based on instance numbers.
> 
> I'm not arguing against the use of driver names at all.
> 
>>> For other buses, we need to make something up.
>>>
>>> Note that addressing by bus address rather than name is generally
>>> useful, not just in the context of savevm.  For instance, I'd appreciate
>>> being able to say something like "device_del pci.0/04.0".
>> And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump
>> the bus first before you can identify which device you want to remove.
> 
> It's not either/or.  Addressing by ID continues to work.  Addressing by
> bus/driver-name continues to work.  We merely add addressing by
> bus/@bus-address.

The format I will propose is "global-ID|/absolute/path", no more
/path/global-ID as this comes with the risk of ambiguity (ID may shadow
bus-local name of a device).

> 
>>> An easy way to get that is to reserve part of the name space for bus
>>> addresses.  If the path component starts with a letter, it's an ID or
>>> driver name.  If it starts with say '@', it's a bus address in
>>> bus-specific syntax.  The bus provides a method to look it up.
>> I would prefer <driver>[@<bus-address>|.<instance-no>]. The former is
>> set for buses that implement some to-be-defined device addressing
>> service, the latter is the default on buses where that service is not
>> available.
> 
> I object to <driver>@<bus-address>, because the <driver> part carries no
> information.

I does for a human being as bus addresses tend to be unreadable and can
easily be confused, hence the additional, sometimes redundant driver name.

> 
> Not the case for <driver>.<instance-no>.  We still need a suitable
> definition of <instance-no>.  Possible definitions:
> 
> * n-th creation of a <driver> device.  Drawbacks: depends on creation
>   order.  Relatively hard to maintain across migration.
> 
> * n-th instance of a <driver> device.  Drawback: changes on unplug.
>   Good enough for interactive use, but it doesn't provide a stable
>   device name.

Every hotplug-capable bus must have a proper addressing scheme, I think
this is a reasonable and achievable requirement. Then we don't need
instance numbers for those buses.

> 
> When counting <driver> devices either way, we can count per bus or
> globally.  I prefer per bus.

Yes, counting should be both per-driver and per-bus ("the <n>th device
managed by <driver> on this bus").

> 
> None of the above instance numbers are nearly as neat as bus addresses.

Right, wherever they are available.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

  reply	other threads:[~2010-06-15 11:59 UTC|newest]

Thread overview: 81+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-14  5:51 [Qemu-devel] [RFC PATCH 0/5] Introduce canonical device hierarchy string Alex Williamson
2010-06-14  5:51 ` [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path() Alex Williamson
2010-06-14  6:39   ` Markus Armbruster
2010-06-14 12:52     ` Alex Williamson
2010-06-14 13:00       ` Jan Kiszka
2010-06-14 13:09       ` Paul Brook
2010-06-14 15:29         ` Alex Williamson
2010-06-14 15:42           ` Paul Brook
2010-06-14 16:00           ` Jan Kiszka
2010-06-14 16:38             ` Alex Williamson
2010-06-14 16:49               ` Jan Kiszka
2010-06-14 18:35                 ` Alex Williamson
2010-06-14 21:43                   ` Paul Brook
2010-06-14 22:11                     ` Alex Williamson
2010-06-14 22:46                       ` Paul Brook
2010-06-15  1:14                         ` Alex Williamson
2010-06-15 11:24                           ` Paul Brook
2010-06-15  8:47         ` Markus Armbruster
2010-06-15  9:34           ` Jan Kiszka
2010-06-15 11:28             ` Paul Brook
2010-06-15 11:45               ` Jan Kiszka
2010-06-15 12:04                 ` Paul Brook
2010-06-15 12:16                   ` Jan Kiszka
2010-06-15 12:39                     ` Paul Brook
2010-06-15 13:00                       ` Jan Kiszka
2010-06-15 13:14                         ` Paul Brook
2010-06-15 13:16                 ` Markus Armbruster
2010-06-15 13:32                   ` Jan Kiszka
2010-06-15 20:53               ` Alex Williamson
2010-06-15 21:55                 ` Paul Brook
2010-06-15 22:33                   ` Alex Williamson
2010-06-15 23:01                     ` Paul Brook
2010-06-15 23:10                       ` Alex Williamson
2010-06-16  0:25                       ` Chris Wright
2010-06-16  0:30                         ` Paul Brook
2010-06-16  0:35                           ` Chris Wright
2010-06-16  1:30                             ` Paul Brook
2010-06-16  2:55                               ` Alex Williamson
2010-06-16  8:23                 ` Markus Armbruster
2010-06-17 22:25                   ` Alex Williamson
2010-06-18  9:16                     ` Jan Kiszka
2010-06-18 15:01                       ` Alex Williamson
2010-06-18 15:22                         ` Jan Kiszka
2010-06-18 14:03                     ` Markus Armbruster
2010-06-18 14:14                       ` Jan Kiszka
2010-06-18 15:21                       ` Alex Williamson
2010-06-15 11:42             ` Markus Armbruster
2010-06-15 11:59               ` Jan Kiszka [this message]
2010-06-15 13:07                 ` Markus Armbruster
2010-06-15 13:19                   ` Paul Brook
2010-06-15 13:32                     ` Paul Brook
2010-06-15 15:08                   ` Jan Kiszka
2010-06-16 13:02                     ` Markus Armbruster
2010-06-14  5:51 ` [Qemu-devel] [RFC PATCH 2/5] savevm: Add DeviceState param Alex Williamson
2010-06-14  5:51 ` [Qemu-devel] [RFC PATCH 3/5] savevm: Make use of the new " Alex Williamson
2010-06-14  5:51 ` [Qemu-devel] [RFC PATCH 4/5] eepro100: Add a dev field to eeprom new/free functions Alex Williamson
2010-06-14  5:51 ` [Qemu-devel] [RFC PATCH 5/5] virtio-net: Incorporate a DeviceState pointer and let savevm track instances Alex Williamson
2010-06-14  7:02 ` [Qemu-devel] Re: [RFC PATCH 0/5] Introduce canonical device hierarchy string Gerd Hoffmann
2010-06-14 19:56   ` Alex Williamson
2010-06-15  8:53     ` Markus Armbruster
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 18:03       ` 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 10:40   ` Paul Brook
2010-06-16 11:37   ` [Qemu-devel] Re: RFC qdev path semantics Jan Kiszka
2010-06-16 11:45     ` Paul Brook
2010-06-16 12:01       ` Jan Kiszka
2010-06-16 12:21         ` Paul Brook
2010-06-16 13:50           ` Jan Kiszka
2010-06-16 13:05   ` Markus Armbruster
2010-06-16 13:23     ` Paul Brook
2010-06-16 14:31       ` Markus Armbruster
2010-06-17 21:43   ` Alex Williamson
2010-06-17 22:01     ` Paul Brook
2010-06-17 22:34       ` Alex Williamson
2010-06-18  7:52     ` Gerd Hoffmann
2010-06-18 14:58   ` Markus Armbruster
2010-06-22 14:27   ` 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=4C176B22.30201@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).