All of lore.kernel.org
 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>,
	"kraxel@redhat.com" <kraxel@redhat.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alex Williamson <alex.williamson@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: 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

WARNING: multiple messages have this Message-ID (diff)
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: 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
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 [this message]
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=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 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.