Re: [Qemu-devel] [PATCHv4 0/5] Clean up compatibility mode handling

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 4268 bytes --]

On Tue, May 30, 2017 at 01:14:16AM +0200, Greg Kurz wrote:
> On Fri, 26 May 2017 15:23:14 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> [...]
> > 
> > 
> > Changes since v3:
> >   * Backwards compatible -cpu handling now removes compat= option from
> >     options passed on to the cpu, so it doesn't trigger further warnings
> 
> This seems to also have another interesting effect.
> 
> getset_compat_deprecated() could be called either during CPU realization from:
> 
> object_property_parse()
> {
>     Visitor *v = string_input_visitor_new(string);
>     object_property_set(obj, v, name, errp);
>     ...
> }
> 
> or during a QOM set operation from:
> 
> void object_property_set_qobject(Object *obj, QObject *value,
>                                  const char *name, Error **errp)
> {
>     Visitor *v;
> 
>     v = qobject_input_visitor_new(value);
>     object_property_set(obj, v, name, errp);
>     ...
> }
> 
> or similarly during a QOM get operation with a QObject output visitor.
> 
> The realization path no longer exists with patch 2, so you don't need
> to implement a null string input visitor anymore.

s/patch 2/patch 3/?

Is that true though?  It shouldn't get called through that path in
practice, because we strip the compat property from the cpu object
properties.  But it could get called if either a) the user explicitly
creates a cpu object with -device CPU,compat=whatever or b) if the
user uses the compat= property with a machine type other than pseries.

Of course the user *shouldn't* do either of those things, but
providing a meaningful error if they do is pretty much the whole
purpose of this getter/setter method.

> 
> This means that patch 1 is no longer needed if I get things right but
> you probably want Markus to second that.
> 
> >   * Add a migration fix make cpu_synchronize_state() safe in post_load
> >     handlers, which in turn fixes a bug in 5/5.
> >   * A number of bugfixes and other tweaks suggested by feedback on v2.
> > 
> > Changes since RFCv2:
> >   * Many patches dropped, since they're already merged
> >   * Rebased, fixed conflicts
> >   * Restored support for backwards migration (wasn't as complicated as
> >     I thought)
> >   * Updated final patch's description to more accurately reflect the
> >     logic
> > 
> > Changes since RFCv1:
> >   * Change CAS logic to prefer compatibility modes over raw mode
> >   * Simplified by giving up on half-hearted attempts to maintain
> >     backwards migration
> >   * Folded migration stream changes into a single patch
> >   * Removed some preliminary patches which are already merged
> > 
> > David Gibson (4):
> >   migration: Mark CPU states dirty before incoming migration/loadvm
> >   pseries: Move CPU compatibility property to machine
> >   pseries: Reset CPU compatibility mode
> >   ppc: Rework CPU compatibility testing across migration
> > 
> > Greg Kurz (1):
> >   qapi: add explicit null to string input and output visitors
> > 
> >  cpus.c                       |   9 ++++
> >  hw/ppc/spapr.c               |   8 +++-
> >  hw/ppc/spapr_cpu_core.c      |  62 +++++++++++++++++++++-----
> >  hw/ppc/spapr_hcall.c         |   8 ++--
> >  include/hw/ppc/spapr.h       |  12 +++--
> >  include/sysemu/cpus.h        |   1 +
> >  include/sysemu/hax.h         |   1 +
> >  include/sysemu/hw_accel.h    |  10 +++++
> >  include/sysemu/kvm.h         |   1 +
> >  kvm-all.c                    |  10 +++++
> >  migration/savevm.c           |   2 +
> >  qapi/string-input-visitor.c  |  11 +++++
> >  qapi/string-output-visitor.c |  14 ++++++
> >  target/i386/hax-all.c        |  10 +++++
> >  target/ppc/compat.c          | 102 +++++++++++++++++++++++++++++++++++++++++++
> >  target/ppc/cpu.h             |   5 ++-
> >  target/ppc/machine.c         |  72 ++++++++++++++++++++++++++++--
> >  target/ppc/translate_init.c  |  86 +++++++++++-------------------------
> >  18 files changed, 340 insertions(+), 84 deletions(-)
> > 
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]