| Message ID | 20100614055119.879.92321.stgit@localhost.localdomain |
|---|---|
| State | New |
| Headers | show |
Alex Williamson <alex.williamson@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...
On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: > Alex Williamson <alex.williamson@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
Alex Williamson wrote: > On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >> Alex Williamson <alex.williamson@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
> > > "/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
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.williamson@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); }
> 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
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
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
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
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
> 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
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
> > > 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
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
Paul Brook <paul@codesourcery.com> writes: > Alex Williamson <alex.williamson@redhat.com> writes: > >> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >> > Could you explain why you add "identified properties of the immediate >> > parent bus and device"? They make the result ver much *not* a "dev >> > path" in the qdev sense... >> >> In order to try to get a unique string. Without looking into device >> properties, two e1000s would both be: >> >> /main-system-bus/pci.0/e1000 >> /main-system-bus/pci.0/e1000 >> >> Which is no better than simply "e1000" and would require us to fall back >> to instance ids again. The goal here is that anything that makes use of >> passing a dev when registering a vmstate gets an instance id of zero. > > You already got the information you need, you just put it in the wrong place. > The canonical ID for the device could be its bus address. In practice we'd > probably want to allow the user to specify it by name, provided these are > unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would > as an aias for [...]:_09.0. Device names have a restricted namespace, so we > can use an initial prefix to disambiguate a name/label from a bus address. > > For busses that don't have a consistent addressing scheme then some sort of > instance ID is unavoidable. I guess it may be possible to invent something > based on other device properties (e.g. address of the first IO port/memory > region). When that's inconvenient or impossible, we can still punt to user: make device ID mandatory. 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.
Markus Armbruster wrote: > Paul Brook <paul@codesourcery.com> writes: > >> Alex Williamson <alex.williamson@redhat.com> writes: >> >>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >>>> Could you explain why you add "identified properties of the immediate >>>> parent bus and device"? They make the result ver much *not* a "dev >>>> path" in the qdev sense... >>> In order to try to get a unique string. Without looking into device >>> properties, two e1000s would both be: >>> >>> /main-system-bus/pci.0/e1000 >>> /main-system-bus/pci.0/e1000 >>> >>> Which is no better than simply "e1000" and would require us to fall back >>> to instance ids again. The goal here is that anything that makes use of >>> passing a dev when registering a vmstate gets an instance id of zero. >> You already got the information you need, you just put it in the wrong place. >> The canonical ID for the device could be its bus address. In practice we'd >> probably want to allow the user to specify it by name, provided these are >> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would >> as an aias for [...]:_09.0. Device names have a restricted namespace, so we >> can use an initial prefix to disambiguate a name/label from a bus address. >> >> For busses that don't have a consistent addressing scheme then some sort of >> instance ID is unavoidable. I guess it may be possible to invent something >> based on other device properties (e.g. address of the first IO port/memory >> region). > > When that's inconvenient or impossible, we can still punt to user: make > device ID mandatory. No option due to auto-created devices. And auto-generating IDs would just create usability issues. > > > 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
> > 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
> > 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
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> Paul Brook <paul@codesourcery.com> writes: >> >>> Alex Williamson <alex.williamson@redhat.com> writes: >>> >>>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >>>>> Could you explain why you add "identified properties of the immediate >>>>> parent bus and device"? They make the result ver much *not* a "dev >>>>> path" in the qdev sense... >>>> In order to try to get a unique string. Without looking into device >>>> properties, two e1000s would both be: >>>> >>>> /main-system-bus/pci.0/e1000 >>>> /main-system-bus/pci.0/e1000 >>>> >>>> Which is no better than simply "e1000" and would require us to fall back >>>> to instance ids again. The goal here is that anything that makes use of >>>> passing a dev when registering a vmstate gets an instance id of zero. >>> You already got the information you need, you just put it in the wrong place. >>> The canonical ID for the device could be its bus address. In practice we'd >>> probably want to allow the user to specify it by name, provided these are >>> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would >>> as an aias for [...]:_09.0. Device names have a restricted namespace, so we >>> can use an initial prefix to disambiguate a name/label from a bus address. >>> >>> For busses that don't have a consistent addressing scheme then some sort of >>> instance ID is unavoidable. I guess it may be possible to invent something >>> based on other device properties (e.g. address of the first IO port/memory >>> region). >> >> When that's inconvenient or impossible, we can still punt to user: make >> device ID mandatory. > > No option due to auto-created devices. And auto-generating IDs would > just create usability issues. Auto-generated IDs would become part of the ABI. Really so bad that it's "no option"? Mind, device ID becomes mandatory *only* for devices that don't have a useful bus address. We could even waive the ID requirement for the first device of a kind, i.e. require ID if and only if it's needed to disambiguate. >> 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
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
Markus Armbruster wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> Markus Armbruster wrote: >>> Paul Brook <paul@codesourcery.com> writes: >>> >>>> Alex Williamson <alex.williamson@redhat.com> writes: >>>> >>>>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >>>>>> Could you explain why you add "identified properties of the immediate >>>>>> parent bus and device"? They make the result ver much *not* a "dev >>>>>> path" in the qdev sense... >>>>> In order to try to get a unique string. Without looking into device >>>>> properties, two e1000s would both be: >>>>> >>>>> /main-system-bus/pci.0/e1000 >>>>> /main-system-bus/pci.0/e1000 >>>>> >>>>> Which is no better than simply "e1000" and would require us to fall back >>>>> to instance ids again. The goal here is that anything that makes use of >>>>> passing a dev when registering a vmstate gets an instance id of zero. >>>> You already got the information you need, you just put it in the wrong place. >>>> The canonical ID for the device could be its bus address. In practice we'd >>>> probably want to allow the user to specify it by name, provided these are >>>> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would >>>> as an aias for [...]:_09.0. Device names have a restricted namespace, so we >>>> can use an initial prefix to disambiguate a name/label from a bus address. >>>> >>>> For busses that don't have a consistent addressing scheme then some sort of >>>> instance ID is unavoidable. I guess it may be possible to invent something >>>> based on other device properties (e.g. address of the first IO port/memory >>>> region). >>> When that's inconvenient or impossible, we can still punt to user: make >>> device ID mandatory. >> No option due to auto-created devices. And auto-generating IDs would >> just create usability issues. > > Auto-generated IDs would become part of the ABI. Really so bad that > it's "no option"? Mind, device ID becomes mandatory *only* for devices > that don't have a useful bus address. We could even waive the ID > requirement for the first device of a kind, i.e. require ID if and only > if it's needed to disambiguate. IDs are there to find devices the user (or a higher level tool) passed to QEMU, qtree paths allow to locate _every_ device in a VM, and that in a well-organized hierarchy. That allows to explore and address a qtree element at the same time. > >>> We obviously need a way to unambigously name a device. It's okay to >>> have multiple names for the same device. >>> >>> If the device has a device ID, that's an unambigous name. >>> >>> qdev paths may be ambigous when path components are resolved to driver >>> names instead of IDs. >>> >>> Alex proposed to disambiguate by adding "identified properties of the >>> immediate parent bus and device" to the path component. For PCI, these >>> are dev.fn. Likewise for any other bus where devices have unambigous >>> bus address. The driver name carries no information! >> >From user POV, driver names are very handly to address a device >> intuitively - except for the case you have tones of devices on the same >> bus that are handled by the same driver. For that case we need to >> augment the device name with a useful per-bus ID, derived from the bus >> address where available, otherwise based on instance numbers. > > I'm not arguing against the use of driver names at all. > >>> For other buses, we need to make something up. >>> >>> Note that addressing by bus address rather than name is generally >>> useful, not just in the context of savevm. For instance, I'd appreciate >>> being able to say something like "device_del pci.0/04.0". >> And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump >> the bus first before you can identify which device you want to remove. > > It's not either/or. Addressing by ID continues to work. Addressing by > bus/driver-name continues to work. We merely add addressing by > bus/@bus-address. The format I will propose is "global-ID|/absolute/path", no more /path/global-ID as this comes with the risk of ambiguity (ID may shadow bus-local name of a device). > >>> An easy way to get that is to reserve part of the name space for bus >>> addresses. If the path component starts with a letter, it's an ID or >>> driver name. If it starts with say '@', it's a bus address in >>> bus-specific syntax. The bus provides a method to look it up. >> I would prefer <driver>[@<bus-address>|.<instance-no>]. The former is >> set for buses that implement some to-be-defined device addressing >> service, the latter is the default on buses where that service is not >> available. > > I object to <driver>@<bus-address>, because the <driver> part carries no > information. I does for a human being as bus addresses tend to be unreadable and can easily be confused, hence the additional, sometimes redundant driver name. > > Not the case for <driver>.<instance-no>. We still need a suitable > definition of <instance-no>. Possible definitions: > > * n-th creation of a <driver> device. Drawbacks: depends on creation > order. Relatively hard to maintain across migration. > > * n-th instance of a <driver> device. Drawback: changes on unplug. > Good enough for interactive use, but it doesn't provide a stable > device name. Every hotplug-capable bus must have a proper addressing scheme, I think this is a reasonable and achievable requirement. Then we don't need instance numbers for those buses. > > When counting <driver> devices either way, we can count per bus or > globally. I prefer per bus. Yes, counting should be both per-driver and per-bus ("the <n>th device managed by <driver> on this bus"). > > None of the above instance numbers are nearly as neat as bus addresses. Right, wherever they are available. Jan
> >> 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
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
> 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
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
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> Jan Kiszka <jan.kiszka@siemens.com> writes: >> >>> Markus Armbruster wrote: >>>> Paul Brook <paul@codesourcery.com> writes: >>>> >>>>> Alex Williamson <alex.williamson@redhat.com> writes: >>>>> >>>>>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >>>>>>> Could you explain why you add "identified properties of the immediate >>>>>>> parent bus and device"? They make the result ver much *not* a "dev >>>>>>> path" in the qdev sense... >>>>>> In order to try to get a unique string. Without looking into device >>>>>> properties, two e1000s would both be: >>>>>> >>>>>> /main-system-bus/pci.0/e1000 >>>>>> /main-system-bus/pci.0/e1000 >>>>>> >>>>>> Which is no better than simply "e1000" and would require us to fall back >>>>>> to instance ids again. The goal here is that anything that makes use of >>>>>> passing a dev when registering a vmstate gets an instance id of zero. >>>>> You already got the information you need, you just put it in the wrong place. >>>>> The canonical ID for the device could be its bus address. In practice we'd >>>>> probably want to allow the user to specify it by name, provided these are >>>>> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would >>>>> as an aias for [...]:_09.0. Device names have a restricted namespace, so we >>>>> can use an initial prefix to disambiguate a name/label from a bus address. >>>>> >>>>> For busses that don't have a consistent addressing scheme then some sort of >>>>> instance ID is unavoidable. I guess it may be possible to invent something >>>>> based on other device properties (e.g. address of the first IO port/memory >>>>> region). >>>> When that's inconvenient or impossible, we can still punt to user: make >>>> device ID mandatory. >>> No option due to auto-created devices. And auto-generating IDs would >>> just create usability issues. >> >> Auto-generated IDs would become part of the ABI. Really so bad that >> it's "no option"? Mind, device ID becomes mandatory *only* for devices >> that don't have a useful bus address. We could even waive the ID >> requirement for the first device of a kind, i.e. require ID if and only >> if it's needed to disambiguate. > > IDs are there to find devices the user (or a higher level tool) passed > to QEMU, qtree paths allow to locate _every_ device in a VM, and that in > a well-organized hierarchy. That allows to explore and address a qtree > element at the same time. > >> >>>> We obviously need a way to unambigously name a device. It's okay to >>>> have multiple names for the same device. >>>> >>>> If the device has a device ID, that's an unambigous name. >>>> >>>> qdev paths may be ambigous when path components are resolved to driver >>>> names instead of IDs. >>>> >>>> Alex proposed to disambiguate by adding "identified properties of the >>>> immediate parent bus and device" to the path component. For PCI, these >>>> are dev.fn. Likewise for any other bus where devices have unambigous >>>> bus address. The driver name carries no information! >>> >From user POV, driver names are very handly to address a device >>> intuitively - except for the case you have tones of devices on the same >>> bus that are handled by the same driver. For that case we need to >>> augment the device name with a useful per-bus ID, derived from the bus >>> address where available, otherwise based on instance numbers. >> >> I'm not arguing against the use of driver names at all. >> >>>> For other buses, we need to make something up. >>>> >>>> Note that addressing by bus address rather than name is generally >>>> useful, not just in the context of savevm. For instance, I'd appreciate >>>> being able to say something like "device_del pci.0/04.0". >>> And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump >>> the bus first before you can identify which device you want to remove. >> >> It's not either/or. Addressing by ID continues to work. Addressing by >> bus/driver-name continues to work. We merely add addressing by >> bus/@bus-address. > > The format I will propose is "global-ID|/absolute/path", no more > /path/global-ID as this comes with the risk of ambiguity (ID may shadow > bus-local name of a device). 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 driver name. I *strenuously* object to making the driver name mandatory with bus-address addressing. If you think the human user needs to be protected from mistakes by making him supply redundant information, then "device_del pci.0/04.0 e1000" is much better way than complicating device paths for human and machine users alike, not to mention uses internal to QEMU. >> Not the case for <driver>.<instance-no>. We still need a suitable >> definition of <instance-no>. Possible definitions: >> >> * n-th creation of a <driver> device. Drawbacks: depends on creation >> order. Relatively hard to maintain across migration. >> >> * n-th instance of a <driver> device. Drawback: changes on unplug. >> Good enough for interactive use, but it doesn't provide a stable >> device name. > > Every hotplug-capable bus must have a proper addressing scheme, I think > this is a reasonable and achievable requirement. Then we don't need > instance numbers for those buses. What about USB? >> When counting <driver> devices either way, we can count per bus or >> globally. I prefer per bus. > > Yes, counting should be both per-driver and per-bus ("the <n>th device > managed by <driver> on this bus"). Works for me. >> None of the above instance numbers are nearly as neat as bus addresses. > > Right, wherever they are available. > > Jan
> >> 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
Jan Kiszka <jan.kiszka@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.
> > 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
> > > 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
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
Markus Armbruster wrote: > Jan Kiszka <jan.kiszka@siemens.com> writes: > >> Markus Armbruster wrote: >>> Jan Kiszka <jan.kiszka@siemens.com> writes: >>> >>>> Markus Armbruster wrote: >>>>> Paul Brook <paul@codesourcery.com> writes: >>>>> >>>>>> Alex Williamson <alex.williamson@redhat.com> writes: >>>>>> >>>>>>> On Mon, 2010-06-14 at 08:39 +0200, Markus Armbruster wrote: >>>>>>>> Could you explain why you add "identified properties of the immediate >>>>>>>> parent bus and device"? They make the result ver much *not* a "dev >>>>>>>> path" in the qdev sense... >>>>>>> In order to try to get a unique string. Without looking into device >>>>>>> properties, two e1000s would both be: >>>>>>> >>>>>>> /main-system-bus/pci.0/e1000 >>>>>>> /main-system-bus/pci.0/e1000 >>>>>>> >>>>>>> Which is no better than simply "e1000" and would require us to fall back >>>>>>> to instance ids again. The goal here is that anything that makes use of >>>>>>> passing a dev when registering a vmstate gets an instance id of zero. >>>>>> You already got the information you need, you just put it in the wrong place. >>>>>> The canonical ID for the device could be its bus address. In practice we'd >>>>>> probably want to allow the user to specify it by name, provided these are >>>>>> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would >>>>>> as an aias for [...]:_09.0. Device names have a restricted namespace, so we >>>>>> can use an initial prefix to disambiguate a name/label from a bus address. >>>>>> >>>>>> For busses that don't have a consistent addressing scheme then some sort of >>>>>> instance ID is unavoidable. I guess it may be possible to invent something >>>>>> based on other device properties (e.g. address of the first IO port/memory >>>>>> region). >>>>> When that's inconvenient or impossible, we can still punt to user: make >>>>> device ID mandatory. >>>> No option due to auto-created devices. And auto-generating IDs would >>>> just create usability issues. >>> Auto-generated IDs would become part of the ABI. Really so bad that >>> it's "no option"? Mind, device ID becomes mandatory *only* for devices >>> that don't have a useful bus address. We could even waive the ID >>> requirement for the first device of a kind, i.e. require ID if and only >>> if it's needed to disambiguate. >> IDs are there to find devices the user (or a higher level tool) passed >> to QEMU, qtree paths allow to locate _every_ device in a VM, and that in >> a well-organized hierarchy. That allows to explore and address a qtree >> element at the same time. >> >>>>> We obviously need a way to unambigously name a device. It's okay to >>>>> have multiple names for the same device. >>>>> >>>>> If the device has a device ID, that's an unambigous name. >>>>> >>>>> qdev paths may be ambigous when path components are resolved to driver >>>>> names instead of IDs. >>>>> >>>>> Alex proposed to disambiguate by adding "identified properties of the >>>>> immediate parent bus and device" to the path component. For PCI, these >>>>> are dev.fn. Likewise for any other bus where devices have unambigous >>>>> bus address. The driver name carries no information! >>>> >From user POV, driver names are very handly to address a device >>>> intuitively - except for the case you have tones of devices on the same >>>> bus that are handled by the same driver. For that case we need to >>>> augment the device name with a useful per-bus ID, derived from the bus >>>> address where available, otherwise based on instance numbers. >>> I'm not arguing against the use of driver names at all. >>> >>>>> For other buses, we need to make something up. >>>>> >>>>> Note that addressing by bus address rather than name is generally >>>>> useful, not just in the context of savevm. For instance, I'd appreciate >>>>> being able to say something like "device_del pci.0/04.0". >>>> And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump >>>> the bus first before you can identify which device you want to remove. >>> It's not either/or. Addressing by ID continues to work. Addressing by >>> bus/driver-name continues to work. We merely add addressing by >>> bus/@bus-address. >> The format I will propose is "global-ID|/absolute/path", no more >> /path/global-ID as this comes with the risk of ambiguity (ID may shadow >> bus-local name of a device). > > 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 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. > > I *strenuously* object to making the driver name mandatory with > bus-address addressing. > > If you think the human user needs to be protected from mistakes by > making him supply redundant information, then "device_del pci.0/04.0 > e1000" is much better way than complicating device paths for human and > machine users alike, not to mention uses internal to QEMU. The only think that may speak against expressive device names might be the overall path length. Not sure if we can already reach critical limits for any involved subsystem, though. Anything else can and should easily be encapsulated for qdev users in a single service providing the unique device name for a given device - independent of the final format. Jan
On Tue, 2010-06-15 at 12:28 +0100, Paul Brook wrote: > > > Alex proposed to disambiguate by adding "identified properties of the > > > immediate parent bus and device" to the path component. For PCI, these > > > are dev.fn. Likewise for any other bus where devices have unambigous > > > bus address. The driver name carries no information! > > > > From user POV, driver names are very handly to address a device > > intuitively - except for the case you have tones of devices on the same > > bus that are handled by the same driver. For that case we need to > > augment the device name with a useful per-bus ID, derived from the bus > > address where available, otherwise based on instance numbers. > > This is where I think you're missing a trick. We don't need to augment the > name, we just need to allow the bus id to be used instead. For the case of a hot remove, I agree. If the user specifies "pci_del pci.0/03.0", that's completely sufficient because we don't care what's in that slot, just remove it. However, I still see some use cases for device names in the path. Take for example: (A): /i440FX-pcihost/pci.0/e1000.05.0 vs (B): /pci.0/05.0 (removing both the root bridge driver name and the device driver name) If we attach a pci option rom to the device, create some ram to store the option rom and name the ram block $(PATH)/rom, with (A) we know more about the hierarchy to get to the actual devfn device, and we know the type of device that's in the slot. With (B), there's no robustness. If we migrated using (B), we could be stuffing a pc e1000 option rom into a ppc lsi895, just because it happened to live that the same place and have a ram block named rom. Including driver names increases the uniqueness of the path. Another example; if we have two drivers that create a vmstate with name "foo", plug one driver into slot 03.0 on the migration source, the other into slot 03.0 on the migration destination, what happens? It seems likely that the destination will try to load the vmstate from a different driver and fail in wonderful and bizarre ways. If we use (A), each path automatically has it's own namespace. ISA is also a good example even though it doesn't do hotplug. Given this set: /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300 versus this set: /pci.0.01.0/isa.0 /pci.0.01.0/isa.0/0x3f8 /pci.0.01.0/isa.0/0x378 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0/0x300 Which one has devices that are easier to uniquely identify? > > > For other buses, we need to make something up. > > > > > > Note that addressing by bus address rather than name is generally > > > useful, not just in the context of savevm. For instance, I'd appreciate > > > being able to say something like "device_del pci.0/04.0". > > > > And I prefer "device_del [.../]pci.0/e1000". Otherwise you need to dump > > the bus first before you can identify which device you want to remove. > > We can allow both. > > A bus address is sufficient to uniquely identify a device. A bus address (assuming it exists) is sufficient to uniquely identify a device, on a given VM. A bus address only identifies a location when comparing two separate VMs. > I see no reason to > require the driver name, or to include it in the canonical device address. Migration. Including the driver name extends our ability to uniquely identify a device across separate VMs. It's then up to the vmstate code to figure out whether the device are compatible for migrate state. > > > An easy way to get that is to reserve part of the name space for bus > > > addresses. If the path component starts with a letter, it's an ID or > > > driver name. If it starts with say '@', it's a bus address in > > > bus-specific syntax. The bus provides a method to look it up. > > > > I would prefer <driver>[@<bus-address>|.<instance-no>]. The former is > > set for buses that implement some to-be-defined device addressing > > service, the latter is the default on buses where that service is not > > available. > > If we have bus-address then I see no good reason to also add instance-no. > For busses that no natural address, we can define the address to be an > instance number. I agree, it should be a bug for any device with a bus address to have an instance number other than zero. Anything without a bus number can make use of instance numbers, and hopefully will never be hotplugged. > > > That way, we gain a useful feature, and avoid having an savevm-specific > > > "device path" that isn't recognized anywhere else. > > > > Agreed, we should find one solution for all use cases. > > I wasn't aware that there was any suggestion of a separate savevm-specific > path. The whole point of a device path is to uniquely identify a device > within a machine. There may be many different paths that identify the same > device. When given a device and asked to generate path, the result should be > the canonical address. IMO this should be the least volatile, and avoid > redundant information. Yep, I'm certainly aiming to not make this specific to savevm, but maybe the difference is that savevm does care about migration. If we take migration out of the picture, then I would tend to agree that we can remove driver names. A bus address uniquely identifies a given device on a given VM. But if we add migration back in, I think we can increase the robustness of the device path without reducing the stability by including the driver names. Most of my examples above are pathological cases that aren't going to happen. I think the driver name does provide useful information across VMs, but I'll go with the consensus whether to include it. BTW, find below output for a VM using the full driver name path versus no driver names. Alex (A) /i440FX-pcihost /i440FX-pcihost/pci.0/i440FX.00.0 /i440FX-pcihost/pci.0/PIIX3.01.0 /i440FX-pcihost/pci.0/cirrus-vga.02.0 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/mc146818rtc /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-serial.0x3f8 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-parallel.0x378 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/i8042 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/isa-fdc /i440FX-pcihost/pci.0/i82551.03.0 /i440FX-pcihost/pci.0/virtio-net-pci.04.0 /i440FX-pcihost/pci.0/e1000.05.0 /i440FX-pcihost/pci.0/rtl8139.06.0 /i440FX-pcihost/pci.0/pcnet.07.0 /i440FX-pcihost/pci.0/PIIX3.01.0/isa.0/ne2k_isa.0x300 /i440FX-pcihost/pci.0/piix3-ide.01.1 /i440FX-pcihost/pci.0/piix3-ide.01.1/ide.0/ide-drive.0 /i440FX-pcihost/pci.0/piix3-ide.01.1/ide.1/ide-drive.0 /i440FX-pcihost/pci.0/piix3-usb-uhci.01.2 /i440FX-pcihost/pci.0/PIIX4_PM.01.3 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.80 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.81 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.82 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.83 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.84 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.85 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.86 /i440FX-pcihost/pci.0/PIIX4_PM.01.3/i2c/smbus-eeprom.87 /i440FX-pcihost/pci.0/lsi53c895a.08.0/scsi.0/scsi-disk.0 /i440FX-pcihost/pci.0/lsi53c895a.08.0/scsi.0/scsi-disk.3 /i440FX-pcihost/pci.0/lsi53c895a.08.0 /i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0/scsi.0/scsi-disk.0 /i440FX-pcihost/pci.0/piix3-usb-uhci.01.2/usb.0/usb-storage.usb0 /i440FX-pcihost/pci.0/virtio-blk-pci.09.0 (B) /pci.0.00.0 /pci.0.01.0 /pci.0.02.0 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0/0x3f8 /pci.0.01.0/isa.0/0x378 /pci.0.01.0/isa.0 /pci.0.01.0/isa.0 /pci.0.03.0 /pci.0.04.0 /pci.0.05.0 /pci.0.06.0 /pci.0.07.0 /pci.0.01.0/isa.0/0x300 /pci.0.01.1 /pci.0.01.1/ide.0.0 /pci.0.01.1/ide.1.0 /pci.0.01.2 /pci.0.01.3 /pci.0.01.3/i2c/80 /pci.0.01.3/i2c/81 /pci.0.01.3/i2c/82 /pci.0.01.3/i2c/83 /pci.0.01.3/i2c/84 /pci.0.01.3/i2c/85 /pci.0.01.3/i2c/86 /pci.0.01.3/i2c/87 /pci.0.08.0/scsi.0/0 /pci.0.08.0/scsi.0/3 /pci.0.08.0 /pci.0.01.2/usb.0/usb0/scsi.0/0 /pci.0.01.2/usb.0/usb0 /pci.0.09.0
> 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
On Tue, 2010-06-15 at 22:55 +0100, Paul Brook wrote: > > 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. The host pci bridge has no identifying properties, so all we'd be printing is the pci bridge driver name, which we claim adds no value. Or are we saying that some driver names are useful while others aren't? > > 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. Driver name is sufficient to tell me that driver "foo" created this device on both the source and destination and hand off responsibility to driver "foo" to make the determination about whether it's compatible. I think it's reasonably safe to say we're not going to have two completely different drivers with name "foo"s within an instance of qemu and we're not going to turn over the name to a different driver and expect anything sane to happen wrt to migration. I disagree that this is a false sense of security, it gives me warm fuzzies. Please convince me otherwise. > > > > > 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. Makes sense to me ;) A bus address is unique only within the context of the VM. Within that VM I can figure out that the device at that bus address uses driver foo, so foo seems redundant in the device path. Now I try to apply that bus address to another VM, the migration target, and all I have is a location. I have to make a leap of faith that the device at that location is the same as the device at the same location on the migration source. Where are those warm fuzzies? > > > 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. 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. > If we include the device name, why aren't we also > including all other properties that would make the devices incompatible? Well, in some ways that'd be really nice, then we could rebuild the device on the fly in the target. But, I'm attempting to work within the existing migration code, because I'd really like something working better than it does today very soon. Therefore, we do want to reduce the path to only those elements that are stable and add value. I argue the driver name is both stable and adds value. Thanks, Alex
> > 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
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
* Paul Brook (paul@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
> * Paul Brook (paul@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
* Paul Brook (paul@codesourcery.com) wrote: > > * Paul Brook (paul@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
> > > > > 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
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
Alex Williamson <alex.williamson@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 the path component starts with a letter, it's an ID or >> > > driver name. If it starts with say '@', it's a bus address in >> > > bus-specific syntax. The bus provides a method to look it up. >> > >> > I would prefer <driver>[@<bus-address>|.<instance-no>]. The former is >> > set for buses that implement some to-be-defined device addressing >> > service, the latter is the default on buses where that service is not >> > available. >> >> If we have bus-address then I see no good reason to also add instance-no. >> For busses that no natural address, we can define the address to be an >> instance number. > > I agree, it should be a bug for any device with a bus address to have an > instance number other than zero. Anything without a bus number can make > use of instance numbers, and hopefully will never be hotplugged. > >> > > That way, we gain a useful feature, and avoid having an savevm-specific >> > > "device path" that isn't recognized anywhere else. >> > >> > Agreed, we should find one solution for all use cases. >> >> I wasn't aware that there was any suggestion of a separate savevm-specific >> path. The whole point of a device path is to uniquely identify a device >> within a machine. There may be many different paths that identify the same >> device. When given a device and asked to generate path, the result should be >> the canonical address. IMO this should be the least volatile, and avoid >> redundant information. Agree. [...]
Jan Kiszka <jan.kiszka@siemens.com> writes: > Markus Armbruster wrote: >> Jan Kiszka <jan.kiszka@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. [...]
On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: > Alex Williamson <alex.williamson@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
Alex Williamson wrote: > On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: >> Alex Williamson <alex.williamson@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 >
Alex Williamson <alex.williamson@redhat.com> writes: > On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: >> Alex Williamson <alex.williamson@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. [...]
Markus Armbruster wrote: > Alex Williamson <alex.williamson@redhat.com> writes: > >> On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: >>> Alex Williamson <alex.williamson@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
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.williamson@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
On Fri, 2010-06-18 at 16:03 +0200, Markus Armbruster wrote: > Alex Williamson <alex.williamson@redhat.com> writes: > > On Wed, 2010-06-16 at 10:23 +0200, Markus Armbruster wrote: > >> Alex Williamson <alex.williamson@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
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.williamson@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
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index 9ffdba7..e30df88 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -453,6 +453,7 @@ PropertyInfo qdev_prop_macaddr = { .name = "macaddr", .type = PROP_TYPE_MACADDR, .size = sizeof(MACAddr), + .flags = PROP_FLAG_PATH, .parse = parse_mac, .print = print_mac, }; @@ -496,6 +497,7 @@ PropertyInfo qdev_prop_pci_devfn = { .name = "pci-devfn", .type = PROP_TYPE_UINT32, .size = sizeof(uint32_t), + .flags = PROP_FLAG_PATH, .parse = parse_pci_devfn, .print = print_pci_devfn, }; diff --git a/hw/qdev.c b/hw/qdev.c index 36f29ea..dea44fe 100644 --- a/hw/qdev.c +++ b/hw/qdev.c @@ -120,6 +120,63 @@ DeviceState *qdev_create(BusState *bus, const char *name) return qdev_create_from_info(bus, info); } +static int qdev_strprint_parent_path(DeviceState *dev, char *buf, size_t len) +{ + int offset = 0; + + if (dev->parent_bus && dev->parent_bus->parent) + offset = qdev_strprint_parent_path(dev->parent_bus->parent, buf, len); + + offset += snprintf(buf + offset, len - offset, + "/%s", dev->parent_bus->name); + return offset; +} + +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=%s", + props->name, pbuf); + } + } + props++; + } + return offset; +} + +char *qdev_get_dev_path(DeviceState *dev) +{ + char buf[256] = ""; + int offset; + + if (!dev) + return NULL; + + offset = qdev_strprint_parent_path(dev, buf, sizeof(buf)); + + offset += qdev_strprint_path_props(dev, dev->parent_bus->info->props, + buf + offset, sizeof(buf) - offset); + + offset += snprintf(buf + offset, sizeof(buf) - offset, "/%s", + dev->info->name); + if (dev->id) + offset += snprintf(buf + offset, sizeof(buf) - offset, "-%s", dev->id); + + offset += qdev_strprint_path_props(dev, dev->info->props, + buf + offset, sizeof(buf) - offset); + return qemu_strdup(buf); +} + static void qdev_print_devinfo(DeviceInfo *info) { error_printf("name \"%s\", bus %s", diff --git a/hw/qdev.h b/hw/qdev.h index a44060e..2702384 100644 --- a/hw/qdev.h +++ b/hw/qdev.h @@ -96,6 +96,7 @@ struct PropertyInfo { const char *name; size_t size; enum PropertyType type; + int flags; int (*parse)(DeviceState *dev, Property *prop, const char *str); int (*print)(DeviceState *dev, Property *prop, char *dest, size_t len); }; @@ -201,6 +202,8 @@ extern PropertyInfo qdev_prop_netdev; extern PropertyInfo qdev_prop_vlan; extern PropertyInfo qdev_prop_pci_devfn; +#define PROP_FLAG_PATH (1<<0) + #define DEFINE_PROP(_name, _state, _field, _prop, _type) { \ .name = (_name), \ .info = &(_prop), \ @@ -280,6 +283,8 @@ void qdev_prop_set_defaults(DeviceState *dev, Property *props); void qdev_prop_register_global_list(GlobalProperty *props); void qdev_prop_set_globals(DeviceState *dev); +char *qdev_get_dev_path(DeviceState *dev); + /* This is a nasty hack to allow passing a NULL bus to qdev_create. */ extern struct BusInfo system_bus_info;
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. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- hw/qdev-properties.c | 2 ++ hw/qdev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++ hw/qdev.h | 5 ++++ 3 files changed, 64 insertions(+), 0 deletions(-)