Re: [PATCH v7 54/65] qdev: adjust PciDevfn declared type

[Daniel P. Berrangé <berrange@xxxxxxxxxx>]

On Tue, Oct 27, 2026 at 03:54:03PM +0100, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@xxxxxxxxxx> writes:
> 
> > Hi
> >
> > On Fri, Oct 23, 2026 at 2:04 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> 
> > 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.

>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

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. 

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).


With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|