Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson wrote: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? What is the device name? /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. I object to being able to use anything but an address under a bus that supports hotplug, but that aside, I think the paths for my example system look like below. Note that anything behind and including the $ is not the canonical path, that begins the free form, usage specific string, here filled by missing device names (open to suggestions other than $ here). What about : instead of $? Visually more appealing IMO. Note that were isa devices do not have a bus identifier, I'm using the device name, Don't understand. Why don't they have their I/O base as prefix? This is inconsistent, and if we consider anything behind $ (or :) informative, the bus address is mandatory for any device (on a bus with an addressing scheme). which I think follows the same rule as i440FX-pcihost. Can we agree this is the goal? Thanks, It looks almost acceptable from my POV. We just need to define how to get hold of devices on buses without an addressing scheme (e.g. the system bus). Using the driver name is insufficient once you have 1 devices of the same type. Introducing device instance numbers to those buses would be straightforward IMO. Once this is done, the next step should be a specification for later use in docs/qdev-device-use.txt. Jan Alex /i440FX-pcihost/pci.0/@00.0$i440FX /i440FX-pcihost/pci.0/@01.0$PIIX3 /i440FX-pcihost/pci.0/@02.0$cirrus-vga /i440FX-pcihost/pci.0/@01.0/isa.0/mc146818rtc /i440FX-pcihost/pci.0/@01.0/isa.0/@0x3f8$isa-serial /i440FX-pcihost/pci.0/@01.0/isa.0/@0x378$isa-parallel /i440FX-pcihost/pci.0/@01.0/isa.0/i8042 /i440FX-pcihost/pci.0/@01.0/isa.0/isa-fdc /i440FX-pcihost/pci.0/@03.0$i82551 /i440FX-pcihost/pci.0/@04.0$virtio-net-pci /i440FX-pcihost/pci.0/@05.0$e1000 /i440FX-pcihost/pci.0/@06.0$rtl8139 /i440FX-pcihost/pci.0/@07.0$pcnet /i440FX-pcihost/pci.0/@01.0/isa.0/@0x300$ne2k_isa /i440FX-pcihost/pci.0/@01.1$piix3-ide /i440FX-pcihost/pci.0/@01.1/ide.0/@0$ide-drive /i440FX-pcihost/pci.0/@01.1/ide.1/@0$ide-drive /i440FX-pcihost/pci.0/@01.2$piix3-usb-uhci /i440FX-pcihost/pci.0/@01.3$PIIX4_PM /i440FX-pcihost/pci.0/@01.3/i2c/@80$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@81$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@82$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@83$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@84$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@85$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@86$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@87$smbus-eeprom /i440FX-pcihost/pci.0/@08.0/scsi.0/@0$scsi-disk /i440FX-pcihost/pci.0/@08.0/scsi.0/@3$scsi-disk /i440FX-pcihost/pci.0/@08.0$lsi53c895a /i440FX-pcihost/pci.0/@01.2/usb.0/@usb0/scsi.0/@0$scsi-disk /i440FX-pcihost/pci.0/@01.2/usb.0/@usb0$usb-storage /i440FX-pcihost/pci.0/@09.0$virtio-blk-pci -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson alex.william...@redhat.com writes: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? I think the general rule for constructing a *canonical* qdev path should be: * If it's the main system bus, the path is /. * If it's another bus, the path is P/B, where P is the canonical path of the device providing the bus, and B is the bus name. Unambiguous, since no device ever defines two buses with the same name. * If it's a device, the path is P/D, where P is the canonical path of the bus. If the bus defines bus addresses, then D is @A, where A is the device's bus address. We haven't made up our minds whether the else case exists, or what to do if it does. The simple else D is the device model driver's name works only if the bus can't take multiple device models with the same driver. The canonical path is not the only path. For instance, a qdev ID is a valid path, but it's not canonical. /i440FX-pcihost/pci.0/e1000 is another valid, non-canonical path. [...]
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? I think the general rule for constructing a *canonical* qdev path should be: * If it's the main system bus, the path is /. * If it's another bus, the path is P/B, where P is the canonical path of the device providing the bus, and B is the bus name. Unambiguous, since no device ever defines two buses with the same name. * If it's a device, the path is P/D, where P is the canonical path of the bus. If the bus defines bus addresses, then D is @A, where A is the device's bus address. We haven't made up our minds whether the else case exists, or what to do if it does. The simple else D is the device model driver's name works only if the bus can't take multiple device models with the same driver. ...which is already on x86 the case with multiple APICs or HPETs on the system bus. The canonical path is not the only path. For instance, a qdev ID is a valid path, but it's not canonical. /i440FX-pcihost/pci.0/e1000 is another valid, non-canonical path. Not only canonical paths need to be specified, also alias like the above. We should limit those alias to a required minimum (required means to me: improves human-friendliness compared to canonical form and preserves backward compatibility where relevant). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Fri, 2010-06-18 at 11:16 +0200, Jan Kiszka wrote: Alex Williamson wrote: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? What is the device name? dev-info-name /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. I object to being able to use anything but an address under a bus that supports hotplug, but that aside, I think the paths for my example system look like below. Note that anything behind and including the $ is not the canonical path, that begins the free form, usage specific string, here filled by missing device names (open to suggestions other than $ here). What about : instead of $? Visually more appealing IMO. Sure, that works. Note that were isa devices do not have a bus identifier, I'm using the device name, Don't understand. Why don't they have their I/O base as prefix? This is inconsistent, and if we consider anything behind $ (or :) informative, the bus address is mandatory for any device (on a bus with an addressing scheme). Not all isa devices expose an iobase property. isa doesn't have an addressing scheme, it's basically a free for all of grabbing addresses and interrupts and hoping they don't conflict with no means to uniquely address a specific device. That's why isa cards are loaded with jumpers so you can try to move them around. which I think follows the same rule as i440FX-pcihost. Can we agree this is the goal? Thanks, It looks almost acceptable from my POV. We just need to define how to get hold of devices on buses without an addressing scheme (e.g. the system bus). Using the driver name is insufficient once you have 1 devices of the same type. Introducing device instance numbers to those buses would be straightforward IMO. Yep, as I mentioned, I can live with instance numbers for devices that do not live on a hotpluggable bus. Where will the instance number go? A property on the parent bus, the device, field in the DeviceState? Thanks, Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Fri, 2010-06-18 at 16:03 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: (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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? I think the general rule for constructing a *canonical* qdev path should be: * If it's the main system bus, the path is /. * If it's another bus, the path is P/B, where P is the canonical path of the device providing the bus, and B is the bus name. Unambiguous, since no device ever defines two buses with the same name. * If it's a device, the path is P/D, where P is the canonical path of the bus. If the bus defines bus addresses, then D is @A, where A is the device's bus address. We haven't made up our minds whether the else case exists, or what to do if it does. The simple else D is the device model driver's name works only if the bus can't take multiple device models with the same driver. It certainly exists today, so do we make up bus addresses, do we use driver name + instance number, or ...? Both of these cases need to make sure they're not artificially imposing a creation order than we can't guarantee across migration. Hopefully Paul has a better suggestion than creation order. The canonical path is not the only path. For instance, a qdev ID is a valid path, but it's not canonical. /i440FX-pcihost/pci.0/e1000 is another valid, non-canonical path. Is it not canonical because of e1000 or because or i440FX-pcihost. It seems like both to me with the nuance that i440FX-pcihost doesn't support multiple instances, so we know which one it is. Is the correct canonical path for this then: /TBD/pci.0/@d.f How do we make progress on defining that TBD and all the ones under isa buses? Thanks, Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson wrote: On Fri, 2010-06-18 at 11:16 +0200, Jan Kiszka wrote: Alex Williamson wrote: On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? What is the device name? dev-info-name That's for me the driver name. However, this one needs extension by an instance number. /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. I object to being able to use anything but an address under a bus that supports hotplug, but that aside, I think the paths for my example system look like below. Note that anything behind and including the $ is not the canonical path, that begins the free form, usage specific string, here filled by missing device names (open to suggestions other than $ here). What about : instead of $? Visually more appealing IMO. Sure, that works. Note that were isa devices do not have a bus identifier, I'm using the device name, Don't understand. Why don't they have their I/O base as prefix? This is inconsistent, and if we consider anything behind $ (or :) informative, the bus address is mandatory for any device (on a bus with an addressing scheme). Not all isa devices expose an iobase property. isa doesn't have an addressing scheme, it's basically a free for all of grabbing addresses and interrupts and hoping they don't conflict with no means to uniquely address a specific device. That's why isa cards are loaded with jumpers so you can try to move them around. The question is if there are ISA devices that do not use io, thus have no such property. Paul pointed out that this might be unrealistic, and in fact we have no such devices in QEMU so far. which I think follows the same rule as i440FX-pcihost. Can we agree this is the goal? Thanks, It looks almost acceptable from my POV. We just need to define how to get hold of devices on buses without an addressing scheme (e.g. the system bus). Using the driver name is insufficient once you have 1 devices of the same type. Introducing device instance numbers to those buses would be straightforward IMO. Yep, as I mentioned, I can live with instance numbers for devices that do not live on a hotpluggable bus. Where will the instance number go? A property on the parent bus, the device, field in the DeviceState? So far I'm generating them on the fly. We could store them in the DeviceState if aliases based on driver-name.instance-no shall be forbidden on hotplug buses, i.e. the instance numbers will not change during VM lifetime. but the question is if such a restriction of alias (even if the are unstable) wouldn't make things too much asymmetric. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. So is the general rule If a device's parent bus does not provide an address, print device name? /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. I object to being able to use anything but an address under a bus that supports hotplug, but that aside, I think the paths for my example system look like below. Note that anything behind and including the $ is not the canonical path, that begins the free form, usage specific string, here filled by missing device names (open to suggestions other than $ here). Note that were isa devices do not have a bus identifier, I'm using the device name, which I think follows the same rule as i440FX-pcihost. Can we agree this is the goal? Thanks, Alex /i440FX-pcihost/pci.0/@00.0$i440FX /i440FX-pcihost/pci.0/@01.0$PIIX3 /i440FX-pcihost/pci.0/@02.0$cirrus-vga /i440FX-pcihost/pci.0/@01.0/isa.0/mc146818rtc /i440FX-pcihost/pci.0/@01.0/isa.0/@0x3f8$isa-serial /i440FX-pcihost/pci.0/@01.0/isa.0/@0x378$isa-parallel /i440FX-pcihost/pci.0/@01.0/isa.0/i8042 /i440FX-pcihost/pci.0/@01.0/isa.0/isa-fdc /i440FX-pcihost/pci.0/@03.0$i82551 /i440FX-pcihost/pci.0/@04.0$virtio-net-pci /i440FX-pcihost/pci.0/@05.0$e1000 /i440FX-pcihost/pci.0/@06.0$rtl8139 /i440FX-pcihost/pci.0/@07.0$pcnet /i440FX-pcihost/pci.0/@01.0/isa.0/@0x300$ne2k_isa /i440FX-pcihost/pci.0/@01.1$piix3-ide /i440FX-pcihost/pci.0/@01.1/ide.0/@0$ide-drive /i440FX-pcihost/pci.0/@01.1/ide.1/@0$ide-drive /i440FX-pcihost/pci.0/@01.2$piix3-usb-uhci /i440FX-pcihost/pci.0/@01.3$PIIX4_PM /i440FX-pcihost/pci.0/@01.3/i2c/@80$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@81$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@82$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@83$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@84$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@85$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@86$smbus-eeprom /i440FX-pcihost/pci.0/@01.3/i2c/@87$smbus-eeprom /i440FX-pcihost/pci.0/@08.0/scsi.0/@0$scsi-disk /i440FX-pcihost/pci.0/@08.0/scsi.0/@3$scsi-disk /i440FX-pcihost/pci.0/@08.0$lsi53c895a /i440FX-pcihost/pci.0/@01.2/usb.0/@usb0/scsi.0/@0$scsi-disk /i440FX-pcihost/pci.0/@01.2/usb.0/@usb0$usb-storage /i440FX-pcihost/pci.0/@09.0$virtio-blk-pci
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson alex.william...@redhat.com writes: 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) / is the main system bus. System bus defines no bus address at the moment. Therefore, you have to use the driver name i440FX-pcihost. /i440FX-pcihost/pci.0 is the PCI bus. PCI bus defines a bus address: dev.fn. Therefore, you can either use the bus address @05.0, or the driver name e1000. We have /i440FX-pcihost/pci.0/e1000 vs. /i440FX-pcihost/pci.0/@05.0. 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. Identifying a device on a given VM is all a qdev path does. If you want to check two VMs have the same device in the same slot, then you need to compare *devices*, not their names. You propose to encode *one* property of the device in the name, namely its driver. This is far from sufficient. If you tell me you need it anyway for migration, I'll have to take that at face value (I'm not an expert there). But please do not call it qdev path, because it ain't. 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. Migration needs to recreate the same qdev tree on the destination. Driver name is just *one* property of a device. Migration needs to transfer *all* properties. Why encode driver name in the path, but not the rest? Why can't you put the driver name wherever you put the rest? An easy way to get that is to reserve part of the name space for bus addresses. If
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: [...] 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). Doesn't this break existing usage? Please name one. It could only be weird corner cases like device_add driver,bus=bus1/ID/bus2 or bus=ID/bus. But given that we always allowed to address bus2 directly (and this is something I cannot and will not change), does this really matter? Maybe allowing paths to start with an ID is something worth considering, will think about this again. I checked with Dan: libvirt doesn't care. So I don't either. [...]
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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. 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! 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. 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. That way, we gain a useful feature, and avoid having an savevm-specific device path that isn't recognized anywhere else.
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Markus Armbruster wrote: Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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. 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. 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. 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. 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. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
In fact what you really want to do is transfer the device tree (including properties), and create the machine from scratch, not load state into a pre-supplied device tree. Well, I agree, but that's a lot more of an overhaul, and once again we're changing the problem. I think it's you that's changing the problem. The requirement is to uniquely identify a device within a machine. Verifying that this device is that compatible with the device at the same address in a different machine is a separate problem. We should not be trying to encode this information in the canonical device path. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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 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. I see no reason to require the driver name, or to include it in the canonical device address. 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. 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. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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. 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. 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. 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. When counting driver devices either way, we can count per bus or globally. I prefer per bus. None of the above instance numbers are nearly as neat as bus addresses. 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. Jan
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? 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. I see no reason to require the driver name, or to include it in the canonical device address. Readability and simplicity (less aliases - for the same reason, I'm removing ID-based addresses from qtree paths, restricting them to the global, flat namespace). 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. Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. 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. Given that it is also user-visible, it should also have an intuitive and informative format to avoid confusions. That may imply slightly more information than strictly required for machine-based processing. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? Autocompletion can report all of them. Errors should report either what the user specified (if the problem is with the address), or the canonical address (if the problem is unrelated to the address). Remember that we also have global aliases (paths that do not begin with /). 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. Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. I don't think either of these are intuitive. There's a good chance that isa-serial.0 will not correspond to the first serial port in the guest. Much better would be allowing use of IO port or MMIO addresses to identify ISA devices. Some modification to the ISABus code may be required to implement this. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Paul Brook wrote: 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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? Autocompletion can report all of them. Autocompletion can only provide what is later on parseable. It doesn't help to see e1000 and 03.0 as device addresses because you do not know their relation that way. Only combining the information into a single name does. Errors should report either what the user specified (if the problem is with the address), or the canonical address (if the problem is unrelated to the address). Remember that we also have global aliases (paths that do not begin with /). They only help if the user set them and therefore should know their semantics. 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. Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. I don't think either of these are intuitive. There's a good chance that isa-serial.0 will not correspond to the first serial port in the guest. Only if you start tweaking the base addresses. Then it will still correspond to the addition order AND the user should be aware of this special setup. Much better would be allowing use of IO port or MMIO addresses to identify ISA devices. Some modification to the ISABus code may be required to implement this. Works for serial, but fails for ISA devices not occupying an address. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Paul Brook wrote: 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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? Autocompletion can report all of them. Autocompletion can only provide what is later on parseable. Of course. It doesn't help to see e1000 and 03.0 as device addresses because you do not know their relation that way. Only combining the information into a single name does. I don't get your argument here. Why shouldn't e1000 and @03.0 refer to the same device? Querying the device itself will tell you both, so it's not hard to figure out that they refer to the same thing. Either piece of information is sufficient, so why do we require both? Note that autocompletion and enumeration for mechanical traversal are different problems. The former should include useful aliases for humans (i.e. both e1000 and @03.0). The latter should be limited to the unique values corresponding to canonical addresses (i.e. @03.0). Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. I don't think either of these are intuitive. There's a good chance that isa-serial.0 will not correspond to the first serial port in the guest. Only if you start tweaking the base addresses. Then it will still correspond to the addition order AND the user should be aware of this special setup. I'm pretty sure there are some machines that have both internal UARTs (considered to be the primary ports), and secondary ports on an ISA bus. If you really want instance numbers, then they may make most sense appended to the driver name. However I think this should be independent of bus addressing, and bus addresses make most sense as the canonical address. Much better would be allowing use of IO port or MMIO addresses to identify ISA devices. Some modification to the ISABus code may be required to implement this. Works for serial, but fails for ISA devices not occupying an address. An ISA device without an IO/MMIO capabilities seems extremely unlikely. What exactly would such a device do? Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Paul Brook wrote: Paul Brook wrote: 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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? Autocompletion can report all of them. Autocompletion can only provide what is later on parseable. Of course. It doesn't help to see e1000 and 03.0 as device addresses because you do not know their relation that way. Only combining the information into a single name does. I don't get your argument here. Why shouldn't e1000 and @03.0 refer to the same device? Querying the device itself will tell you both, so it's not hard to figure out that they refer to the same thing. Either piece of information is sufficient, so why do we require both? To avoid having to issue an info qtree in the middle of an auto-completion for some other command. Note that autocompletion and enumeration for mechanical traversal are different problems. The former should include useful aliases for humans (i.e. both e1000 and @03.0). The latter should be limited to the unique values corresponding to canonical addresses (i.e. @03.0). Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. I don't think either of these are intuitive. There's a good chance that isa-serial.0 will not correspond to the first serial port in the guest. Only if you start tweaking the base addresses. Then it will still correspond to the addition order AND the user should be aware of this special setup. I'm pretty sure there are some machines that have both internal UARTs (considered to be the primary ports), and secondary ports on an ISA bus. If you really want instance numbers, then they may make most sense appended to the driver name. However I think this should be independent of bus addressing, and bus addresses make most sense as the canonical address. That's how my current implementation looks like. Much better would be allowing use of IO port or MMIO addresses to identify ISA devices. Some modification to the ISABus code may be required to implement this. Works for serial, but fails for ISA devices not occupying an address. An ISA device without an IO/MMIO capabilities seems extremely unlikely. What exactly would such a device do? Inject interrupts via that bus (while exposing registers in some other way). The m48t59 seems to fall in this category (unless I'm missing something ATM). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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). Doesn't this break existing usage? We have a rule to resolve any ambiguity added by ID: it always takes precedence over driver name. What path/ID does add is shadowing: it can make a device inaccessible by driver name. Not much of a difference to adding a second device with the same driver name. 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
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Works for serial, but fails for ISA devices not occupying an address. An ISA device without an IO/MMIO capabilities seems extremely unlikely. What exactly would such a device do? Inject interrupts via that bus (while exposing registers in some other way). The m48t59 seems to fall in this category (unless I'm missing something ATM). m48t59_isa responds to IO ports. You're probably confused because it has not been properly converted to the qdev. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Jan Kiszka jan.kis...@siemens.com writes: 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. I prefer having one name per device, both unique AND human-friendly. Adding yet another alias will solve only the first requirement. E.g., which one should I present to the monitor user when listing a bus for auto-completion or path error reporting? 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. I see no reason to require the driver name, or to include it in the canonical device address. Readability and simplicity (less aliases - for the same reason, I'm removing ID-based addresses from qtree paths, restricting them to the global, flat namespace). 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. Again readability: isa-serial.0 isa-serial.1 is more intuitive than isa-serial.6 isa-serial.7 just because there happen to be 6 other ISA devices registered before them. Readability is in the eye of the beholder. If the beholder wants to number his serial devices a certain way, he better makes his wishes known with id=, because the system has a hard time guessing them. 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. Given that it is also user-visible, it should also have an intuitive and informative format to avoid confusions. That may imply slightly more information than strictly required for machine-based processing. I'm with Paul here.
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. What about USB? USB has useful device addresses (physical ports). These aren't used by most of the higher-level protocols (logically USB is a flat bus structure), but the physically tree topology is still there if you look hard enough. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. What about USB? USB has useful device addresses (physical ports). These aren't used by most of the higher-level protocols (logically USB is a flat bus structure), but the physically tree topology is still there if you look hard enough. ... and thinking about it a bit more, this adds an extra addressing possibility. USB devices have both physical and logical. The physical addresses is which port the device is plugged into (including port addresses of any intermediate hubs). These never change, so should be the canonical address. The logical address is set by the guest OS so may change (or be absent). Most software uses the logical address, so this is by far the most useful option for mapping from a guest device to a qemu device. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Markus Armbruster wrote: 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. Given that it is also user-visible, it should also have an intuitive and informative format to avoid confusions. That may imply slightly more information than strictly required for machine-based processing. I'm with Paul here. Well, what I'm proposing is derived from my experiences collected while playing with device_show and device_add/del over the past weeks. For these monitor scenarios, it was very handy to have expressive path elements which avoid having to issue 'info qtree' all the time (which is annoying if you are in the middle of a command input). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Jan Kiszka jan.kis...@siemens.com writes: Markus Armbruster wrote: Paul Brook p...@codesourcery.com writes: Alex Williamson alex.william...@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). Doesn't this break existing usage? Please name one. It could only be weird corner cases like device_add driver,bus=bus1/ID/bus2 or bus=ID/bus. But given that we always allowed to address bus2 directly (and this is something I cannot and will not change), does this really matter? Maybe allowing paths to start with an ID is something worth considering, will think about this again. We have a rule to resolve any ambiguity added by ID: it always takes precedence over driver name. What path/ID does add is shadowing: it can make a device inaccessible by driver name. Not much of a difference to adding a second device with the same driver name. ID was introduces as a _global_ address, there is simply no point using it _inside_ qtree paths, it will just cause troubles and confusions. Rather, we need to resolve the ambiguity of bus-local names - what this thread is about. An easy way to get that is to reserve part of the name space for bus addresses. If the path
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Wed, 2010-06-16 at 00:01 +0100, Paul Brook wrote: 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. See my comment above, I'm not seeing a sufficient argument about why driver name matching is a false sense of security. If on an incoming migration I'm able to match the source provided e1000.03.0/vmstate against the target registered e1000.03.0/vmstate and hand off to the e1000 driver to check version ids, you bet I'm feeling a lot more secure than if I'm handing off to whatever happened to register 03.0/vmstate on the target. I still say it should be the migration code that checks that both vmstate structures are for the same type of device. i.e. if necessary the device name should be embedded in the device state, not the device path. The migration code would check that (%s/%s, path, name) match. So embedding the driver name into path gives us a per path namespace. Sure the migration code could check (%s/%s/%s, path, dev-info-name, name), but should it be the migration code's responsibility to dig that out? And if you think that i440FX-pcihost is a useful driver name, then we'd have to figure out which driver names are useful. I think it's more consistent to simply put them all there. Thanks, Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. See my comment above, I'm not seeing a sufficient argument about why driver name matching is a false sense of security. If on an incoming migration I'm able to match the source provided e1000.03.0/vmstate against the target registered e1000.03.0/vmstate and hand off to the e1000 driver to check version ids, you bet I'm feeling a lot more secure than if I'm handing off to whatever happened to register 03.0/vmstate on the target. I still say it should be the migration code that checks that both vmstate structures are for the same type of device. i.e. if necessary the device name should be embedded in the device state, not the device path. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
* Paul Brook (p...@codesourcery.com) wrote: 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. See my comment above, I'm not seeing a sufficient argument about why driver name matching is a false sense of security. If on an incoming migration I'm able to match the source provided e1000.03.0/vmstate against the target registered e1000.03.0/vmstate and hand off to the e1000 driver to check version ids, you bet I'm feeling a lot more secure than if I'm handing off to whatever happened to register 03.0/vmstate on the target. I still say it should be the migration code that checks that both vmstate structures are for the same type of device. i.e. if necessary the device name should be embedded in the device state, not the device path. I not sure how that fixes the ramblock issue that started this whole discussion. It's not part of device's vmstate, it goes w/ ram. I think I'm missing a piece of the puzzle here? thanks, -chris
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
* Paul Brook (p...@codesourcery.com) wrote: 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. See my comment above, I'm not seeing a sufficient argument about why driver name matching is a false sense of security. If on an incoming migration I'm able to match the source provided e1000.03.0/vmstate against the target registered e1000.03.0/vmstate and hand off to the e1000 driver to check version ids, you bet I'm feeling a lot more secure than if I'm handing off to whatever happened to register 03.0/vmstate on the target. I still say it should be the migration code that checks that both vmstate structures are for the same type of device. i.e. if necessary the device name should be embedded in the device state, not the device path. I not sure how that fixes the ramblock issue that started this whole discussion. It's not part of device's vmstate, it goes w/ ram. I think I'm missing a piece of the puzzle here? My main point there was that ram blocks should be associated with a device state. Once you do this it should just work as we already know how to match device states. It you're trying to transfer ram blocks before matching up the rest of the state then you're likely to loose in all kinds of different ways. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
* Paul Brook (p...@codesourcery.com) wrote: * Paul Brook (p...@codesourcery.com) wrote: 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. See my comment above, I'm not seeing a sufficient argument about why driver name matching is a false sense of security. If on an incoming migration I'm able to match the source provided e1000.03.0/vmstate against the target registered e1000.03.0/vmstate and hand off to the e1000 driver to check version ids, you bet I'm feeling a lot more secure than if I'm handing off to whatever happened to register 03.0/vmstate on the target. I still say it should be the migration code that checks that both vmstate structures are for the same type of device. i.e. if necessary the device name should be embedded in the device state, not the device path. I not sure how that fixes the ramblock issue that started this whole discussion. It's not part of device's vmstate, it goes w/ ram. I think I'm missing a piece of the puzzle here? My main point there was that ram blocks should be associated with a device state. Once you do this it should just work as we already know how to match device states. It you're trying to transfer ram blocks before matching up the rest of the state then you're likely to loose in all kinds of different ways. Yes, I see, thanks for clarifying. I agree, ideally we'd create the entire target image dynamically. It'd still need to know how to connect all the guest devices to the host, but it makes more sense to me than half building the system from cmdline on target, then rest filled in. I think that level of work is a ways away though. thanks, -chris
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
See my comment above, I'm not seeing a sufficient argument about
why driver name matching is a false sense of security.
I still say it should be the migration code that checks that both
vmstate structures are for the same type of device. i.e. if
necessary the device name should be embedded in the device state,
not the device path.
I not sure how that fixes the ramblock issue that started this whole
discussion. It's not part of device's vmstate, it goes w/ ram. I
think I'm missing a piece of the puzzle here?
My main point there was that ram blocks should be associated with a
device state. Once you do this it should just work as we already know
how to match device states.
It you're trying to transfer ram blocks before matching up the rest of
the state then you're likely to loose in all kinds of different ways.
... or not. See below.
Yes, I see, thanks for clarifying. I agree, ideally we'd create the
entire target image dynamically. It'd still need to know how to connect
all the guest devices to the host, but it makes more sense to me than
half building the system from cmdline on target, then rest filled in.
Transferring the machine description on migration is a separate problem.
Lets say we associate each RAM block with a device. Each ram block also has a
name. These names distinguish between blocks attached to a given device, but
need not be globally unique. i.e. devices A and B can both have block named
foo. RAM block migration happens before device state migration (including
device properties).
There are three relevant migration failure modes:
(1) The same device is present, but has a different size property.
If the incoming block is larger than the allocated block then you loose.
(2) A different device is present, but does not have a ram block of the same
name.
This safely fails migration because of the block name mismatch.
(3) A different device is present, that happens to have a ram block of the
same name.
If the blocks are the same size then transferring the contents is harmless.
If they are different sizes then this will be caught by (1). Either way, the
migration will be failed once we get to the vmstate check.
Note how adding the device type to the canonical address does not effect the
outcome.
Going back to the original problem, (1) is the most interesting.
I suggest that the initial migration phase transfers a list of ram blocks.
Each entry in that list should be {canonical device path, name, size}. You
should lookup all these ram blocks, and fail migration if they are not present
with the correct size[1]. This list also gives you a convenient numeric index
to identify the block during RAM migration.
[1] In the future we may be able to resize blocks. However this is not safe
with the current API.
Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Wed, 2010-06-16 at 02:30 +0100, Paul Brook wrote:
Transferring the machine description on migration is a separate problem.
Lets say we associate each RAM block with a device. Each ram block also has a
name. These names distinguish between blocks attached to a given device, but
need not be globally unique. i.e. devices A and B can both have block named
foo. RAM block migration happens before device state migration (including
device properties).
There are three relevant migration failure modes:
(1) The same device is present, but has a different size property.
If the incoming block is larger than the allocated block then you loose.
(2) A different device is present, but does not have a ram block of the same
name.
This safely fails migration because of the block name mismatch.
(3) A different device is present, that happens to have a ram block of the
same name.
If the blocks are the same size then transferring the contents is harmless.
If they are different sizes then this will be caught by (1). Either way, the
migration will be failed once we get to the vmstate check.
Note how adding the device type to the canonical address does not effect the
outcome.
Going back to the original problem, (1) is the most interesting.
I suggest that the initial migration phase transfers a list of ram blocks.
Each entry in that list should be {canonical device path, name, size}. You
should lookup all these ram blocks, and fail migration if they are not
present
with the correct size[1]. This list also gives you a convenient numeric
index
to identify the block during RAM migration.
[1] In the future we may be able to resize blocks. However this is not safe
with the current API.
I think for the most part, you've just described the RAMBlock series of
patches I sent out last week. I'll note that that series creates ram
blocks on the target if they aren't present because of the technicality
that we currently do not have a qemu_ram_free() to cleanup the list when
things go away. Once we have that and cleanup drivers to use it, I
agree we should fail the migration if it occurs, or at least print out a
big warning so we can go fix the driver. If I'm missing where else it's
significantly different please let me know.
Yes, case (3) would fail in the vmstate code without driver name in the
canonical path... or at least we hope it would. But with the driver
name in the canonical path, we can avoid doing a useless operation, fail
earlier, and provide the vmstate with a key piece of information it can
use to help ensure that the incoming state information belongs to the
driver it thinks it does. Seems like a win to me. Thanks,
Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson alex.william...@redhat.com writes: qdev_get_dev_path() is intended to be the canonical utility for creating a string representing the qdev hierarchy of a device. The path consists of bus and device names as well as identified properties of the immediate parent bus and device. This results in strings like: /main-system-bus/pci.0,addr=00.0/i440FX /main-system-bus/pci.0,addr=01.0/PIIX3 /main-system-bus/pci.0,addr=02.0/cirrus-vga /main-system-bus/pci.0/isa.0/mc146818rtc /main-system-bus/pci.0/isa.0/isa-serial /main-system-bus/pci.0/isa.0/i8042 /main-system-bus/pci.0/isa.0/isa-fdc /main-system-bus/pci.0,addr=03.0/i82551,mac=52:54:00:12:34:56 /main-system-bus/pci.0,addr=04.0/virtio-net-pci,mac=52:54:00:12:34:57 /main-system-bus/pci.0,addr=05.0/e1000,mac=52:54:00:12:34:58 /main-system-bus/pci.0,addr=06.0/rtl8139,mac=52:54:00:12:34:59 /main-system-bus/pci.0,addr=07.0/pcnet,mac=52:54:00:12:34:5a /main-system-bus/pci.0,addr=01.1/piix3-ide /main-system-bus/pci.0,addr=01.3/PIIX4_PM /main-system-bus/pci.0,addr=08.0/lsi53c895a /main-system-bus/pci.0,addr=09.0/virtio-blk-pci There are two primary targets for these strings. The first is vmsave, where we currently use a device provided string plus instance number to track SaveStateEntries. This fails when we introduce device hotplug, particularly in a case were we have gaps in the instance numbers that cannot easily be reproduced on a migration target. The second is for naming RAMBlocks. For these, we would like a string that matches the vmstate string. 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...
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: qdev_get_dev_path() is intended to be the canonical utility for creating a string representing the qdev hierarchy of a device. The path consists of bus and device names as well as identified properties of the immediate parent bus and device. This results in strings like: /main-system-bus/pci.0,addr=00.0/i440FX /main-system-bus/pci.0,addr=01.0/PIIX3 /main-system-bus/pci.0,addr=02.0/cirrus-vga /main-system-bus/pci.0/isa.0/mc146818rtc /main-system-bus/pci.0/isa.0/isa-serial /main-system-bus/pci.0/isa.0/i8042 /main-system-bus/pci.0/isa.0/isa-fdc /main-system-bus/pci.0,addr=03.0/i82551,mac=52:54:00:12:34:56 /main-system-bus/pci.0,addr=04.0/virtio-net-pci,mac=52:54:00:12:34:57 /main-system-bus/pci.0,addr=05.0/e1000,mac=52:54:00:12:34:58 /main-system-bus/pci.0,addr=06.0/rtl8139,mac=52:54:00:12:34:59 /main-system-bus/pci.0,addr=07.0/pcnet,mac=52:54:00:12:34:5a /main-system-bus/pci.0,addr=01.1/piix3-ide /main-system-bus/pci.0,addr=01.3/PIIX4_PM /main-system-bus/pci.0,addr=08.0/lsi53c895a /main-system-bus/pci.0,addr=09.0/virtio-blk-pci There are two primary targets for these strings. The first is vmsave, where we currently use a device provided string plus instance number to track SaveStateEntries. This fails when we introduce device hotplug, particularly in a case were we have gaps in the instance numbers that cannot easily be reproduced on a migration target. The second is for naming RAMBlocks. For these, we would like a string that matches the vmstate string. 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. Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
/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 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). Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Alex Williamson wrote: On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: Alex Williamson alex.william...@redhat.com writes: qdev_get_dev_path() is intended to be the canonical utility for creating a string representing the qdev hierarchy of a device. The path consists of bus and device names as well as identified properties of the immediate parent bus and device. This results in strings like: /main-system-bus/pci.0,addr=00.0/i440FX /main-system-bus/pci.0,addr=01.0/PIIX3 /main-system-bus/pci.0,addr=02.0/cirrus-vga /main-system-bus/pci.0/isa.0/mc146818rtc /main-system-bus/pci.0/isa.0/isa-serial /main-system-bus/pci.0/isa.0/i8042 /main-system-bus/pci.0/isa.0/isa-fdc /main-system-bus/pci.0,addr=03.0/i82551,mac=52:54:00:12:34:56 /main-system-bus/pci.0,addr=04.0/virtio-net-pci,mac=52:54:00:12:34:57 /main-system-bus/pci.0,addr=05.0/e1000,mac=52:54:00:12:34:58 /main-system-bus/pci.0,addr=06.0/rtl8139,mac=52:54:00:12:34:59 /main-system-bus/pci.0,addr=07.0/pcnet,mac=52:54:00:12:34:5a /main-system-bus/pci.0,addr=01.1/piix3-ide /main-system-bus/pci.0,addr=01.3/PIIX4_PM /main-system-bus/pci.0,addr=08.0/lsi53c895a /main-system-bus/pci.0,addr=09.0/virtio-blk-pci There are two primary targets for these strings. The first is vmsave, where we currently use a device provided string plus instance number to track SaveStateEntries. This fails when we introduce device hotplug, particularly in a case were we have gaps in the instance numbers that cannot easily be reproduced on a migration target. The second is for naming RAMBlocks. For these, we would like a string that matches the vmstate string. 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. Will soon (re-)post a patch that adds per-bus instance numbers to deal with identically named devices. That's required as not all buses have canonical node IDs (e.g. ISA or the main system bus). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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
to expand the previous output. Code below.
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.
Sure, if there was a guaranteed unique char* on a DeviceState that was
consistent between guest invocations, we could just use that instead. I
suppose all devices could have a global system id property and if that
was present we'd use that instead of creating the device path.
Device names have a restricted namespace, so we
can use an initial prefix to disambiguate a name/label from a bus address.
I'm not sure it's necessary. It seems like it would only be necessary
to differentiate the two if we wanted to translate between namespaces.
But I think it's reasonable to require that if a global name is
provided, it must always be provided for the life of the VM.
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,
Alex
Signed-off-by: Alex Williamson alex.william...@redhat.com
---
static int qdev_strprint_path_props(DeviceState *dev, Property *props,
char *buf, size_t len)
{
int offset = 0;
char pbuf[64];
if (!props)
return 0;
while (props-name) {
if (props-info-flags PROP_FLAG_PATH) {
if (props-info-print) {
props-info-print(dev, props, pbuf, sizeof(pbuf));
offset += snprintf(buf + offset, len - offset, /_%s, pbuf);
}
}
props++;
}
return offset;
}
static int qdev_strprint_dev(DeviceState *dev, char *buf, size_t len);
static int qdev_strprint_bus(DeviceState *dev, char *buf, size_t len)
{
int offset = 0;
if (dev-parent_bus-parent)
offset += qdev_strprint_dev(dev-parent_bus-parent, buf, len);
offset += snprintf(buf + offset, len - offset, /%s/%s,
dev-parent_bus-info-name, dev-parent_bus-name);
offset += qdev_strprint_path_props(dev, dev-parent_bus-info-props,
buf + offset, len - offset);
return offset;
}
static int qdev_strprint_dev(DeviceState *dev, char *buf, size_t len)
{
int offset = 0;
if (dev-parent_bus)
offset += qdev_strprint_bus(dev, buf, len);
offset += snprintf(buf + offset, len - offset, /%s, dev-info-name);
offset += qdev_strprint_path_props(dev, dev-info-props,
buf + offset, len - offset);
if (dev-id)
offset += snprintf(buf + offset, len - offset, /%s, dev-id);
return offset;
}
char *qdev_get_dev_path(DeviceState *dev)
{
char buf[256] = ;
if (!dev)
return NULL;
qdev_strprint_dev(dev, buf, sizeof(buf));
return qemu_strdup(buf);
}
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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/virti o-blk No. Now you've got way to many elements in the path. My point is that you replace the name of the device with the bus address of the device. However you determine the element names the path should be /busname/devicename pairs. I'm undecided whether the root element should be the main system bus, or a system device node that contains the main system bus. 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. Sure, if there was a guaranteed unique char* on a DeviceState that was consistent between guest invocations, we could just use that instead. I suppose all devices could have a global system id property and if that was present we'd use that instead of creating the device path. All we require is some way of uniquely addressing each device on the bus. For PCI that's trivial (devfn). For other busses we get to pick something appropriate. Device names have a restricted namespace, so we can use an initial prefix to disambiguate a name/label from a bus address. I'm not sure it's necessary. It seems like it would only be necessary to differentiate the two if we wanted to translate between namespaces. But I think it's reasonable to require that if a global name is provided, it must always be provided for the life of the VM. I don't think requiring that all devices are given a globally unique name is going to fly. locally (per-bus) unique user-specified are still a PITA, but may be acceptable. Having a canonical addressing system that's independent of user assigned IDs seems like a good thing. 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, Simple consecutive instance IDs are inherently unstable. They depend on teh order of device creation. The ID really wants to be something that can be reliably determined from the device bus itself (and/or its interaction with its parent bus). Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. 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). 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. driver.busaddr. to expand the previous output. Code below. 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. Sure, if there was a guaranteed unique char* on a DeviceState that was consistent between guest invocations, we could just use that instead. I suppose all devices could have a global system id property and if that was present we'd use that instead of creating the device path. Device names have a restricted namespace, so we can use an initial prefix to disambiguate a name/label from a bus address. I'm not sure it's necessary. It seems like it would only be necessary to differentiate the two if we wanted to translate between namespaces. But I think it's reasonable to require that if a global name is provided, it must always be provided for the life of the VM. 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. 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 Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. driver.busaddr. 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 name:addr, 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.) 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. 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, Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
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. driver.busaddr. 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 name:addr, 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
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Mon, 2010-06-14 at 18:49 +0200, Jan Kiszka wrote: Alex Williamson wrote: On Mon, 2010-06-14 at 18:00 +0200, Jan Kiszka wrote: 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. driver.busaddr. 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 name:addr, 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. Ok, I can get it down to something like: /i440FX-pcihost/pci.0/virtio-blk-pci,09.0 The addr on the device is initially a little non-intuitive to me since it's a property of the bus, but I guess it make sense if we think of that level as slot, which includes an address and driver. 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. Yep, I share that concern, which is I say instance numbers aren't always bad. If we have devices that we don't care about doing hotplug on, we can live with instance numbers. Thanks, Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Mon, 2010-06-14 at 18:49 +0200, Jan Kiszka wrote: Alex Williamson wrote: On Mon, 2010-06-14 at 18:00 +0200, Jan Kiszka wrote: 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. driver.busaddr. 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 name:addr, 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. Ok, I can get it down to something like: /i440FX-pcihost/pci.0/virtio-blk-pci,09.0 The addr on the device is initially a little non-intuitive to me since it's a property of the bus, but I guess it make sense if we think of that level as slot, which includes an address and driver. That indicates you're thinking about things the wrong way. The point of this path is to uniquely identify an entity. /i440FX-pcihost/pci0 identifies a PCI bus attached to the i440FX-pcihost device. To identify a device attached to that bus all you need to know is the devfn of the device. For an end-user it may be helpful to allow devices to be identified by the device type (assuming only one device of a particular type on that bus), or specify the device type as a crude error checking mechanism. However we're talking about canonical addresses. These need not include the device type. Verifying that the device is actually what you expect is a separate problem, and the device type is not sufficient for that. i.e. /i440FX-pcihost/pci.0/,09.0 Is an appropriate canonical address. 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. That's a feature. If you start with two NICs and remove the first, the chances are that the second will be in a different place to the nice created in a single-nic config. Allowing migration between these two will cause a PCI device to suddenly change location. This is not physically or logically possible, and everyone looses. Hot-removing a nic and then hotplugging a new nic in the same location may result in something that is reasonable to migrate. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Mon, 2010-06-14 at 22:43 +0100, Paul Brook wrote: On Mon, 2010-06-14 at 18:49 +0200, Jan Kiszka wrote: Alex Williamson wrote: Ok, I can get it down to something like: /i440FX-pcihost/pci.0/virtio-blk-pci,09.0 The addr on the device is initially a little non-intuitive to me since it's a property of the bus, but I guess it make sense if we think of that level as slot, which includes an address and driver. That indicates you're thinking about things the wrong way. The point of this path is to uniquely identify an entity. /i440FX-pcihost/pci0 identifies a PCI bus attached to the i440FX-pcihost device. To identify a device attached to that bus all you need to know is the devfn of the device. Hmm, I think that identifies where the device is, not what the device is. It's more helpful to say the e1000 in slot 7 than it is to say the device in slot 7. For an end-user it may be helpful to allow devices to be identified by the device type (assuming only one device of a particular type on that bus), or specify the device type as a crude error checking mechanism. However we're talking about canonical addresses. These need not include the device type. Verifying that the device is actually what you expect is a separate problem, and the device type is not sufficient for that. i.e. /i440FX-pcihost/pci.0/,09.0 Is an appropriate canonical address. We seem to keep introducing new problems, and I'm not sure this one exists. If I drop the device name/type and use only the devfn, then what's to prevent the /e1000,09.0/rom (/,09.0/rom) from being stuffed into the /rtl8139,09.0/rom (/,09.0/rom) on a migration? (or we match it to /09.0/savevm when trying to migrate state) We can argue that e1000 isn't a sufficient identifier, but I can't think of a case where it'd fail. 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. That's a feature. If you start with two NICs and remove the first, the chances are that the second will be in a different place to the nice created in a single-nic config. Allowing migration between these two will cause a PCI device to suddenly change location. This is not physically or logically possible, and everyone looses. If the BAR addresses don't follow the VM when it's migrated, that's another bug that needs to be fixed, but we can't get there until we at least have some infrastructure in place to make that bug possible. Hot-removing a nic and then hotplugging a new nic in the same location may result in something that is reasonable to migrate. It might... it might not. I'd rather make it work than try to document the restrictions. Alex
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
Ok, I can get it down to something like: /i440FX-pcihost/pci.0/virtio-blk-pci,09.0 The addr on the device is initially a little non-intuitive to me since it's a property of the bus, but I guess it make sense if we think of that level as slot, which includes an address and driver. That indicates you're thinking about things the wrong way. The point of this path is to uniquely identify an entity. /i440FX-pcihost/pci0 identifies a PCI bus attached to the i440FX-pcihost device. To identify a device attached to that bus all you need to know is the devfn of the device. Hmm, I think that identifies where the device is, not what the device is. It's more helpful to say the e1000 in slot 7 than it is to say the device in slot 7. Why is this more useful? Canonical addresses should not be helpful. They should identify entities within a machine that is already known to be consistent. Making them helpful just makes them more volatile. For an end-user it may be helpful to allow devices to be identified by the device type (assuming only one device of a particular type on that bus), or specify the device type as a crude error checking mechanism. However we're talking about canonical addresses. These need not include the device type. Verifying that the device is actually what you expect is a separate problem, and the device type is not sufficient for that. i.e. /i440FX-pcihost/pci.0/,09.0 Is an appropriate canonical address. We seem to keep introducing new problems, and I'm not sure this one exists. If I drop the device name/type and use only the devfn, then what's to prevent the /e1000,09.0/rom (/,09.0/rom) from being stuffed into the /rtl8139,09.0/rom (/,09.0/rom) on a migration? (or we match it to /09.0/savevm when trying to migrate state) We can argue that e1000 isn't a sufficient identifier, but I can't think of a case where it'd fail. The migration code needs to check that the devices are actually compatible. I'd expect this to require much more than just the device name. What you actually need is more like An e1000 with 64k eeprom, fast ethernet PHY, and frobnitz B. In fact what you really want to do is transfer the device tree (including properties), and create the machine from scratch, not load state into a pre-supplied device tree. 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. That's a feature. If you start with two NICs and remove the first, the chances are that the second will be in a different place to the nice created in a single-nic config. Allowing migration between these two will cause a PCI device to suddenly change location. This is not physically or logically possible, and everyone looses. If the BAR addresses don't follow the VM when it's migrated, that's another bug that needs to be fixed, but we can't get there until we at least have some infrastructure in place to make that bug possible. Not BAR addresses, the actual PCI device addresses. Devices on the PCI bus are addressed by device and function. This is guest visible. The device part of this address corresponds to the physical slot, which typically effects IRQ routing (amongst other things). If you arbitrarily move a device from slot A to slot B then this will have catastrophic effects on a running machine. Paul
Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()
On Mon, 2010-06-14 at 23:46 +0100, Paul Brook wrote: Ok, I can get it down to something like: /i440FX-pcihost/pci.0/virtio-blk-pci,09.0 The addr on the device is initially a little non-intuitive to me since it's a property of the bus, but I guess it make sense if we think of that level as slot, which includes an address and driver. That indicates you're thinking about things the wrong way. The point of this path is to uniquely identify an entity. /i440FX-pcihost/pci0 identifies a PCI bus attached to the i440FX-pcihost device. To identify a device attached to that bus all you need to know is the devfn of the device. Hmm, I think that identifies where the device is, not what the device is. It's more helpful to say the e1000 in slot 7 than it is to say the device in slot 7. Why is this more useful? Canonical addresses should not be helpful. They should identify entities within a machine that is already known to be consistent. Making them helpful just makes them more volatile. Being able to check that device 09.0 is attached to the e1000 driver on source and the rtl8139 driver on the target seems pretty useful to me. For an end-user it may be helpful to allow devices to be identified by the device type (assuming only one device of a particular type on that bus), or specify the device type as a crude error checking mechanism. However we're talking about canonical addresses. These need not include the device type. Verifying that the device is actually what you expect is a separate problem, and the device type is not sufficient for that. i.e. /i440FX-pcihost/pci.0/,09.0 Is an appropriate canonical address. We seem to keep introducing new problems, and I'm not sure this one exists. If I drop the device name/type and use only the devfn, then what's to prevent the /e1000,09.0/rom (/,09.0/rom) from being stuffed into the /rtl8139,09.0/rom (/,09.0/rom) on a migration? (or we match it to /09.0/savevm when trying to migrate state) We can argue that e1000 isn't a sufficient identifier, but I can't think of a case where it'd fail. The migration code needs to check that the devices are actually compatible. I'd expect this to require much more than just the device name. What you actually need is more like An e1000 with 64k eeprom, fast ethernet PHY, and frobnitz B. No, that's savevm's problem, and it's perfectly capable of doing it via the version ids. We're not using the driver name to describe just any random e1000, it's the one that the e1000 driver created, version foo. If it added a frobnitz in version foo+1, either that savevm knows how to import a version foo or rejects it. In fact what you really want to do is transfer the device tree (including properties), and create the machine from scratch, not load state into a pre-supplied device tree. Well, I agree, but that's a lot more of an overhaul, and once again we're changing the problem. 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. That's a feature. If you start with two NICs and remove the first, the chances are that the second will be in a different place to the nice created in a single-nic config. Allowing migration between these two will cause a PCI device to suddenly change location. This is not physically or logically possible, and everyone looses. If the BAR addresses don't follow the VM when it's migrated, that's another bug that needs to be fixed, but we can't get there until we at least have some infrastructure in place to make that bug possible. Not BAR addresses, the actual PCI device addresses. Devices on the PCI bus are addressed by device and function. This is guest visible. The device part of this address corresponds to the physical slot, which typically effects IRQ routing (amongst other things). If you arbitrarily move a device from slot A to slot B then this will have catastrophic effects on a running machine. Sorry, I jumped to BARs because the PCI device address mismatch is kinda the point of where I'm going. With these changes, we're at least allowing that a smart enough management tool is actually able to create a VM state to match a source instance that has done hotplug operations. As it is today, I don't think it's possible to migrate a VM that has gaps in it's savevm instance ids. Alex
