On Wed, May 15, 2024 at 05:03:44PM +0100, Daniel P. Berrangé wrote: > Above all, I'm failing to see why there's a compelling reason > for virtio_gpu to diverge from our long standing practice of > adding a named property flag "virtio_scanout_vmstate_fix" > on the machine class, and then setting it in machine types > which need it.
The reason to introduce that is definitely avoid introducing fields / properties in similar cases in which case all the fields may represent the same thing ("return true if MC is older than xxx version"). Especially when such change is not bound to a new feature so in which case it won't make sense to allow user to even control that propoerty, even if we exported this "x-virtio-scanout-fix" property, but now we must export it because compat fields require it. However I think agree that having upstream specific MC versions in VMSD checks is kind of unwanted. I think the major problem is we don't have that extra machine type abstract where we can have simply a number showing the release of QEMU, then we can map that number to whatever upstream/downstream machine types. E.g.: Release No. Upstream version Downstream version 50 9.0 Y.0 51 9.1 52 9.2 Y.1 ... Then downstream is not mapping to 9.0/... but the release no. Then here instead of hard code upstream MC versions we can already provide similar helpers like: machine_type_newer_than_50() Then device code can use it without polluting that with upstream MC versioning. Downstream will simply work if downstream MCs are mapped alright to the release no. when rebase. But I'm not sure whether it'll be even worthwhile.. the majority will still be that the VMSD change is caused by a new feature, and exporting that property might in most cases be wanted. In all cases, for now I agree it's at least easier to stick with the simple way. Thanks, -- Peter Xu