Re: [PATCH v2 00/21] qom: introduce property flags to track external user input

[Zhao Liu <zhao1.liu@intel.com>]

> So the main thing that pushes us into using QOM for internal properties
> is the machine type compatibility code. eg where we bulk set stuff using
> 
>     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
> 
> That logic is all QOM based. Using QOM isn't our exclusive approach, as
> we have machine types sometimes setting object fields directly. eg
> 
>   static void pc_i440fx_machine_7_0_options(MachineClass *m)
>   {
>     PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
>     pc_i440fx_machine_7_1_options(m);
>     pcmc->enforce_amd_1tb_hole = false;
>     compat_props_add(m->compat_props, hw_compat_7_0, hw_compat_7_0_len);
>     compat_props_add(m->compat_props, pc_compat_7_0, pc_compat_7_0_len);
>   }
> 
> but that only works for properties against the machine type, not compat
> properties against devices, since we have no direct access to the other
> classes/instances.

Right, and setting fields directly is only possible for machine-level
state, not for device properties created dynamically during
realize/init. So QOM-based compat properties are indeed inescapable
for devices.

> If we want to be able to control hardware compat, without exposing
> something as a user facing tunable, then internal-only QOM props seems
> inescapable.

Agreed.

> I do still wonder if we genuinely need internal-only QOM props for
> machine type compat ?
> 
> Whether using a public 'x-' prefixed property or an internal only
> property, we're constrained by the need to retain the behaviour
> semantics for as long as the machine type exists. I don't see an
> internal only property giving us significantly more freedom here.
> We can already rename a 'x-' property however we want with no
> notice, as long as the machine type doesn't change behaviour.

Hmm, I think x- prefix is already semantically overloaded. Looking at
the current codebase, x- carries two very different meanings:

 - "unstable but user-facing feature" - e.g. x-vga, x-igd-opregion
   (documented in docs/igd-assign.rst with user-facing examples),
   x-migration-multifd-transfer (documented in
   docs/devel/vfio-migration.rst).

 - "internal compat switch to fix bug" - e.g. x-buggy-eim,
   x-pci-express-writeable-slt-bug.

These two categories have fundamentally different contracts - the
former should be settable by users, the latter should not. But today,
nothing prevents a user from writing something like:

"-device intel-iommu,x-buggy-eim=false"

QEMU will happily accept it.

An INTERNAL flag can reject this at property set time with a clear
error.

And management tools use qom-list-properties to discover settable
properties (though this series hasn't covered this but it's
possible) - they currently have no way to distinguish "x-vga"
(user-facing) from "x-pci-express-writeable-slt-bug" (internal
bug-compat). An INTERNAL flag could give a clean boundary for
introspection.

> I don't think it does. Code evolution is painful as long as the machine
> type using the prop needs to exist with fixed semantics, whether it is
> internal or public with x- prefix.

I agree the machine type lifetime constraint doesn't change.

But experimental x- or normal (external) properties are by default
exposed to users, allowing them to set values via -device or other
entry points. This effectively treats the property as an API.

Once it becomes an API, any modification to the property must consider
whether there are external dependencies. Just as with the series
(removing v2.6 & v2.7 PC machines) I referenced in cover letter,
justifying whether a property can be directly removed becomes very
complicated and unpredictable - some are definitely in use, while some
are merely "potentially" in use.

Let me take the recently added compatibility properties as another
example - they are all x86 CPU properties:
 * x-l1-cache-per-thread: it's useful for external user to tune L1
   cache topology (though now we have smp-cache, before smp-cache,
   it was useful).
 * x-consistent-cache: this actually is a fix and should be
   internal-only.

While both are compatibility properties and share the x- prefix,
their purposes differ, reflecting the confusion around x- prefix I
mentioned earlier. It's also expected that distinct strategies will
be required when genuinely considering the removal of these two
categories of properties in the future. :-(

So the advantage of distinguishing between internal and external
properties is that modifications or even removal of internal
properties can be done directly without worrying about external
consumers. External properties, as potential API objects, require
explicit deprecation before they can be dropped.

Although it doesn't directly help with modifying existing properties,
any newly added internal property won't increase such API concerns
going forward.

So I'm not positioning INTERNAL as a replacement for x- prefix -
rather, it complements x- by providing runtime enforcement, clean
introspection, and preventing accidental ABI commitment that a naming
convention alone cannot. x- remains appropriate for
unstable-but-user-facing features.

Thanks,
Zhao