Re: [PULL 0/4] machine development tool

[Peter Xu]

On Mon, Mar 18, 2024 at 08:08:29PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> On 08.03.24 06:47, Peter Xu wrote:
> > On Thu, Mar 07, 2024 at 12:06:59PM +0300, Maksim Davydov wrote:
> > > 
> > > On 3/6/24 04:57, Peter Xu wrote:
> > > > On Tue, Mar 05, 2024 at 03:43:41PM +0100, Markus Armbruster wrote:
> > > > > Peter Maydell<peter.maydell@linaro.org>  writes:
> > > > > 
> > > > > > On Mon, 4 Mar 2024 at 13:52, Maksim 
> > > > > > Davydov<davydov-max@yandex-team.ru>  wrote:
> > > > > > > The following changes since commit 
> > > > > > > e1007b6bab5cf97705bf4f2aaec1f607787355b8:
> > > > > > > 
> > > > > > >     Merge tag 'pull-request-2024-03-01' 
> > > > > > > ofhttps://gitlab.com/thuth/qemu  into staging (2024-03-01 
> > > > > > > 10:14:32 +0000)
> > > > > > > 
> > > > > > > are available in the Git repository at:
> > > > > > > 
> > > > > > >     https://gitlab.com/davydov-max/qemu.git  
> > > > > > > tags/pull-compare-mt-2024-03-04
> > > > > > > 
> > > > > > > for you to fetch changes up to 
> > > > > > > 7693a2e8518811a907d73a85807ee71dac8fabcb:
> > > > > > > 
> > > > > > >     scripts: add script to compare compatibility properties 
> > > > > > > (2024-03-04 14:10:53 +0300)
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > > Please note. This is the first pull request from me.
> > > > > > > My public GPG key is available here
> > > > > > > https://keys.openpgp.org/vks/v1/by-fingerprint/CDB5BEEF8837142579F5CDFE8E927E10F72F78D4
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > > scripts: add a new script for machine development
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > > Hi; I would prefer this to go through some existing submaintainer
> > > > > > tree if possible, please.
> > > > > Migration?  QOM?  Not sure.  Cc'ing the maintainers anyway.
> > > > Yeah this seems like migration relevant.. however now I'm slightly 
> > > > confused
> > > > on when this script should be useful.
> > > > 
> > > > According to:
> > > > 
> > > > https://lore.kernel.org/qemu-devel/20240222153912.646053-5-davydov-max@yandex-team.ru/
> > > > 
> > > >           This script runs QEMU to obtain compat_props of machines and
> > > >           default values of different types of drivers to produce 
> > > > comparison
> > > >           table. This table can be used to compare machine types to 
> > > > choose
> > > >           the most suitable machine or compare binaries to be sure that
> > > >           migration to the newer version will save all device
> > > >           properties. Also the json or csv format of this table can be 
> > > > used
> > > >           to check does a new machine affect the previous ones by 
> > > > comparing
> > > >           tables with and without the new machine.
> > > > 
> > > > In regards to "choose the most suitable machine": why do you need to 
> > > > choose
> > > > a machine?
> > > > 
> > > > If it's pretty standalone setup, shouldn't we always try to use the 
> > > > latest
> > > > machine type if possible (as normally compat props are only used to keep
> > > > compatible with old machine types, and the default should always be
> > > > preferred). If it's a cluster setup, IMHO it should depend on the oldest
> > > > QEMU version that plans to be supported.  I don't see how such 
> > > > comparison
> > > > helps yet in either of the cases..
> > > > 
> > > > In regards to "compare binaries to be sure that migration to the newer
> > > > version will save all device properties": do we even support migrating
> > > > _between_ machine types??
> > > > 
> > > > Sololy relying on compat properties to detect device compatibility is 
> > > > also
> > > > not reliable.  For example, see VMStateField.field_exists() or 
> > > > similarly,
> > > > VMStateDescription.needed(), which are hooks that device can provide to
> > > > dynamically decide what device state to be saved/loaded.  Such things 
> > > > are
> > > > not reflected in compat properties, so even if compat properties of all
> > > > devices are the same between two machine types, it may not mean that the
> > > > migration stream will always be compatible.
> > > > 
> > > > Thanks,
> > > 
> > > In fact, the last commit describes the meaning of this series best. 
> > > Perhaps
> > > it should have been moved to the cover letter:
> > > Often, many teams have their own "machines" inherited from "upstream" to
> > > manage default values of devices. This is done because of the limitations
> > > imposed by the compatibility requirements or the desire to help their
> > > customers better configure their devices. And since machine type has
> > > a hard-to-read structure, it is very easy to make a mistake when
> > > transferring
> > > default values from one machine to another. For example, when some 
> > > property
> > > is set for the entire abstract class x86_64-cpu (which will be applied to
> > > all
> > > models), and then rolled back for a specific model. The situation is about
> > > the same with changing the default values of devices: if the value changes
> > > in the description of the device itself, then you need to make sure that
> > > nothing changes when using the current machine.
> > > Therefore, there was a desire to make a dev tool that will help quickly
> > > expand
> > > the definition of a machine or compare several machines from different
> > > binary
> > > files. It can be used when rebasing to a new version of qemu and/or for
> > > automatic tests.
> > 
> > OK, thanks.
> > 
> > So is it a migration compatibility issue that you care (migrating VMs from
> > your old downstream binary to new downstream binary should always succeed),
> > or perhaps you care more on making sure the features you wanted, i.e., you
> > want to make sure some specific devices that you care will have the
> > properties that you expect?
> 
> Actually both things.
> 
> 1. We need a tool to analyze, what exactly changes between MT-s. Do we want 
> to move on new upstream MT or not, how much it is different from our 
> downstream MT and so on.
> 2. It also could be used to check, that new MT is correctly defined (not 
> breaking old MT's)
> 
> > 
> > I think compat properties are mostly used for migration purposes, but
> > indeed it can also be used to keep old behaviors of devices, even if the
> > migration could succed with/without such a compat property entry.
> > 
> > If it's about migration, I'd like to know whether vmstate-static-checker.py
> > could also help your case (under scripts/), perhaps in a better way,
> > because it directly observes the VMSD structures (which is the ultimate
> > form on wire, after all these compat properties applied to the devices).
> 
> Hmm, vmstate-static-checker.py checks a concrete device configuration. So 
> it's a different thing.

I don't think so - 'qemu -dump-vmstate' should dump all device states that
it ever supports.  Feel free to have a look at dump_vmstate_json_to_file(),
or just try give it a dump.

> 
> > 
> > If it's not about migration, then maybe it's more QOM-relevant, and if so I
> > don't have a strong opinion. It seems still make some sense to have a tool
> > simply dump the QOM tree for a machine type with all properties and compare
> > them between machines with some binaries.  For that I'll leave that to
> > Markus to decide.
> 
> Markus ACKed :)

I didn't see Markus acked all the patches yet, but if so that's okay then.
Even if so, I think what Peter Maydell suggested is then this series should
go through the QOM tree, rather than a separate pull.

Thanks,

-- 
Peter Xu