Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

[Alex Williamson]

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)

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.

Another example; if we have two drivers that create a vmstate with name
"foo", plug one driver into slot 03.0 on the migration source, the other
into slot 03.0 on the migration destination, what happens?  It seems
likely that the destination will try to load the vmstate from a
different driver and fail in wonderful and bizarre ways.  If we use (A),
each path automatically has it's own namespace.

ISA is also a good example even though it doesn't do hotplug.  Given
this set:

/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300

versus this set:

/pci.0.01.0/isa.0
/pci.0.01.0/isa.0/0x3f8
/pci.0.01.0/isa.0/0x378
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0/0x300

Which one has devices that are easier to uniquely identify?
 
> > > 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 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.

> > > 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.
> 
> If we have bus-address then I see no good reason to also add instance-no.
> For busses that no natural address, we can define the address to be an 
> instance number.

I agree, it should be a bug for any device with a bus address to have an
instance number other than zero.  Anything without a bus number can make
use of instance numbers, and hopefully will never be hotplugged.

> > > That way, we gain a useful feature, and avoid having an savevm-specific
> > > "device path" that isn't recognized anywhere else.
> > 
> > Agreed, we should find one solution for all use cases.
> 
> I wasn't aware that there was any suggestion of a separate savevm-specific 
> path.  The whole point of a device path is to uniquely identify a device 
> within a machine. There may be many different paths that identify the same 
> device.  When given a device and asked to generate  path, the result should 
> be 
> the canonical address.  IMO this should be the least volatile, and avoid 
> redundant information.

Yep, I'm certainly aiming to not make this specific to savevm, but maybe
the difference is that savevm does care about migration.  If we take
migration out of the picture, then I would tend to agree that we can
remove driver names.  A bus address uniquely identifies a given device
on a given VM.  But if we add migration back in, I think we can increase
the robustness of the device path without reducing the stability by
including the driver names.  Most of my examples above are pathological
cases that aren't going to happen.  I think the driver name does provide
useful information across VMs, but I'll go with the consensus whether to
include it.  BTW, find below output for a VM using the full driver name
path versus no driver names.

Alex

(A)

/i440FX-pcihost
/i440FX-pcihost/pci.0/i440FX.00.0
/i440FX-pcihost/pci.0/PIIX3.01.0
/i440FX-pcihost/pci.0/cirrus-vga.02.0
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc
/i440FX-pcihost/pci.0/i82551.03.0
/i440FX-pcihost/pci.0/virtio-net-pci.04.0
/i440FX-pcihost/pci.0/e1000.05.0
/i440FX-pcihost/pci.0/rtl8139.06.0
/i440FX-pcihost/pci.0/pcnet.07.0
/i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300
/i440FX-pcihost/pci.0/piix3-ide.01.1
/i440FX-pcihost/pci.0/piix3-ide.01.1/ide.0/ide-drive.0
/i440FX-pcihost/pci.0/piix3-ide.01.1/ide.1/ide-drive.0
/i440FX-pcihost/pci.0/piix3-usb-uhci.01.2
/i440FX-pcihost/pci.0/PIIX4_PM.01.3
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.80
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.81
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.82
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.83
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.84
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.85
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.86
/i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.87
/i440FX-pcihost/pci.0/lsi53c895a.08.0/scsi.0/scsi-disk.0
/i440FX-pcihost/pci.0/lsi53c895a.08.0/scsi.0/scsi-disk.3
/i440FX-pcihost/pci.0/lsi53c895a.08.0
/i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0/scsi.0/scsi-disk.0
/i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0
/i440FX-pcihost/pci.0/virtio-blk-pci.09.0

(B)

/pci.0.00.0
/pci.0.01.0
/pci.0.02.0
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0/0x3f8
/pci.0.01.0/isa.0/0x378
/pci.0.01.0/isa.0
/pci.0.01.0/isa.0
/pci.0.03.0
/pci.0.04.0
/pci.0.05.0
/pci.0.06.0
/pci.0.07.0
/pci.0.01.0/isa.0/0x300
/pci.0.01.1
/pci.0.01.1/ide.0.0
/pci.0.01.1/ide.1.0
/pci.0.01.2
/pci.0.01.3
/pci.0.01.3/i2c/80
/pci.0.01.3/i2c/81
/pci.0.01.3/i2c/82
/pci.0.01.3/i2c/83
/pci.0.01.3/i2c/84
/pci.0.01.3/i2c/85
/pci.0.01.3/i2c/86
/pci.0.01.3/i2c/87
/pci.0.08.0/scsi.0/0
/pci.0.08.0/scsi.0/3
/pci.0.08.0
/pci.0.01.2/usb.0/usb0/scsi.0/0
/pci.0.01.2/usb.0/usb0
/pci.0.09.0