Re: [PATCH 02/13] s390_flic: add migration-enabled property

[Thomas Huth]

On 16/05/2024 16.42, Marc Hartmayer wrote:
> On Thu, May 09, 2024 at 07:00 PM +0200, Paolo Bonzini <pbonzini@redhat.com> 
> wrote:
> > Instead of mucking with css_migration_enabled(), add a property specific to
> > the FLIC device, similar to what is done for TYPE_S390_STATTRIB.
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   include/hw/s390x/s390_flic.h | 1 +
> >   hw/intc/s390_flic.c          | 6 +++++-
> >   hw/s390x/s390-virtio-ccw.c   | 1 +
> >   3 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
> > index 3907a13d076..bcb081def58 100644
> > --- a/include/hw/s390x/s390_flic.h
> > +++ b/include/hw/s390x/s390_flic.h
> > @@ -47,6 +47,7 @@ struct S390FLICState {
> >       /* to limit AdapterRoutes.num_routes for compat */
> >       uint32_t adapter_routes_max_batch;
> >       bool ais_supported;
> > +    bool migration_enabled;
> >   };
> >   
> >   
> > diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
> > index f4a848460b8..7f930800877 100644
> > --- a/hw/intc/s390_flic.c
> > +++ b/hw/intc/s390_flic.c
> > @@ -405,6 +405,8 @@ static void qemu_s390_flic_class_init(ObjectClass *oc, void 
> > *data)
> >   static Property s390_flic_common_properties[] = {
> >       DEFINE_PROP_UINT32("adapter_routes_max_batch", S390FLICState,
> >                          adapter_routes_max_batch, ADAPTER_ROUTES_MAX_GSI),
> > +    DEFINE_PROP_BOOL("migration-enabled", S390FLICState,
> > +                     migration_enabled, true),
> >       DEFINE_PROP_END_OF_LIST(),
> >   };
> >   
> > @@ -457,7 +459,9 @@ type_init(qemu_s390_flic_register_types)
> >   
> >   static bool adapter_info_so_needed(void *opaque)
> >   {
> > -    return css_migration_enabled();
> > +    S390FLICState *fs = S390_FLIC_COMMON(opaque);
> > +
> > +    return fs->migration_enabled;
> >   }
...
> This patch causes QEMU to crash when trying to save the domain state
> (e.g. using libvirt)

Oh, drat, that vmstate belongs to a ccw device, not to a flic device, so the 
"opaque" pointer in adapter_info_so_needed points to the wrong structure.

I guess the easiest fix is:

diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -459,7 +459,7 @@ type_init(qemu_s390_flic_register_types)

 static bool adapter_info_so_needed(void *opaque)
 {
-    S390FLICState *fs = S390_FLIC_COMMON(opaque);
+    S390FLICState *fs = s390_get_flic();

     return fs->migration_enabled;
 }

I'll send it as a proper patch...

 Thomas