qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paul Brook <paul@codesourcery.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: chrisw@redhat.com, kvm@vger.kernel.org,
	Jan Kiszka <jan.kiszka@siemens.com>,
	Markus Armbruster <armbru@redhat.com>,
	qemu-devel@nongnu.org, avi@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Date: Tue, 15 Jun 2010 22:55:47 +0100	[thread overview]
Message-ID: <201006152255.48221.paul@codesourcery.com> (raw)
In-Reply-To: <1276635192.12015.901.camel@x201>

> On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote:
> > > > 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.
> > 
> > This is where I think you're missing a trick. We don't need to augment
> > the name, we just need to allow the bus id to be used instead.
> 
> For the case of a hot remove, I agree.  If the user specifies "pci_del
> pci.0/03.0", that's completely sufficient because we don't care what's
> in that slot, just remove it.  However, I still see some use cases for
> device names in the path.  Take for example:
> 
> (A): /i440FX-pcihost/pci.0/e1000.05.0
> 
> vs
> 
> (B): /pci.0/05.0

> (removing both the root bridge driver name and the device driver name)

This is wrong. You still need an entry for the host pci bridge.

 > If we attach a pci option rom to the device, create some ram to store
> the option rom and name the ram block $(PATH)/rom, with (A) we know more
> about the hierarchy to get to the actual devfn device, and we know the
> type of device that's in the slot.  With (B), there's no robustness.  If
> we migrated using (B), we could be stuffing a pc e1000 option rom into a
> ppc lsi895, just because it happened to live that the same place and
> have a ram block named rom.  Including driver names increases the
> uniqueness of the path.

No it doesn't. It adds redundant information and a false sense of security. 
Driver name is not sufficient to determine whether you've actually got a 
compatible device. The type of the device doesn't reliably tell you anything 
about how many ram blocks that device has, or how big they are.

> > > > 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.
> > 
> > We can allow both.
> > A bus address is sufficient to uniquely identify a device.
> 
> A bus address (assuming it exists) is sufficient to uniquely identify a
> device, on a given VM.  A bus address only identifies a location when
> comparing two separate VMs.

I can't make any sense of this statement.
 
> >   I see no reason to
> > require the driver name,  or to include it in the canonical device
> > address.
> 
> Migration.  Including the driver name extends our ability to uniquely
> identify a device across separate VMs.  It's then up to the vmstate code
> to figure out whether the device are compatible for migrate state.

I find this argument contradictory. The migration code already needs to check 
whether a device is compatible before it allows migration.  The driver name is 
not sufficient to ensure compatibility, so I see no benefit in including it in 
the device address. If we include the device name, why aren't we also 
including all other properties that would make the devices incompatible?

Paul

  reply	other threads:[~2010-06-15 21:56 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 [this message]
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
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=201006152255.48221.paul@codesourcery.com \
    --to=paul@codesourcery.com \
    --cc=alex.williamson@redhat.com \
    --cc=armbru@redhat.com \
    --cc=avi@redhat.com \
    --cc=chrisw@redhat.com \
    --cc=jan.kiszka@siemens.com \
    --cc=kraxel@redhat.com \
    --cc=kvm@vger.kernel.org \
    --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).