Daniel P. Berrangé <[email protected]> writes: > On Tue, Oct 27, 2026 at 03:54:03PM +0100, Markus Armbruster wrote: >> Marc-André Lureau <[email protected]> writes: >> >> > Hi >> > >> > On Fri, Oct 23, 2026 at 2:04 PM Daniel P. Berrangé <[email protected]> >> > wrote: >> >> >> >> On Tue, Oct 20, 2026 at 02:46:11PM +0400, Marc-André Lureau wrote: >> >> > PciDevfn is a bit special, since it accepts integer and string as >> >> > setter, but only output integer as getter. > > snip > >> > But I agree we >> > could do better. >> > >> > There are multiple options. We could keep declaring the property as >> > str instead. In this case, the getter should probably return a string >> > for consistency and correctness. This might surprise some existing >> > users though. >> >> I.e. ditch numeric suppport for numeric addresses, right? >> >> Incompatible change. Do we care? >> >> If we don't, we should be honest about it and mark qom-get unstable. >> >> > We could introduce new properties like `devfn-str` and `devfn-int` >> > that alias the same value. >> >> Compatibility break. If breaking compatibility is okay, I'd rather use >> the opportunity to get rid of cleverness, and go with your first option. >> >> > We could also introduce a new structured QAPI type. >> >> What kind of type do you have in mind? > > IMHO the root cause design flaw here was to overload 'addr' as a > way to set both the slot number and function number, as using > one property for two pieces of data is a well known anti-pattern > we caution against. > > IOW, we should have had 'slot' and 'function' as separate properties > from the start.
Fair. Our modeling of PCI is old and weird. What we call "pci-device" isn't a device, it's a function, where function 0 sort of stands in for the device. Its "addr" isn't a PCI address, it's the address of the function on its PCI bus, i.e. the tail of a PCI address. > From a CLI POV, we could easily change 'addr' to mean only the slot > number, since it is always parsed in string format without the > magic (slot << 4) | function encoding. Then add a 'function' prop In CLI, HMP and QMP string (not integer), value "5" is shorthand for "5.0", and means device 5, function 0. We could deprecate the .FN part in favor of a new property "function". Once we drop support for .FN from "addr", its name becomes even more misleading than it always was. > The challenge is the impact on QMP with strict typing, as ideally > int format would soley represent the slot number, but today the > int format is encoded (slot << 4) | function. So anyone relying > on int format in QMP would silently have a semantic change if we > altered what 'addr' means. That's a no-go. > Deprecating 'addr' entirely and adding brand new 'slot' and > 'function' properties would be a significant disruption to > a syntax which has been documented for years (decades). What about this: * No change to PCIDevice member @devfn: encodes slot and function as an integer. * New "pci-device" properties "slot" and "function". "slot" sets just the slot bits of @devfn, and "function" sets just the function bits. Both default to zero, so specifying just "slot" gives you PCI_DEVFN(slot, 0), which makes sense. * Deprecate "pci-device" property "addr" for QMP. We can keep it around for longer than the required grace period if that helps. * Declare it legacy for CLI and HMP, with no intent to remove it.
