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;
}
const VMStateDescription vmstate_adapter_info_so = {
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index feabc173eb3..1383e47eeb5 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -1174,6 +1174,7 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
static GlobalProperty compat[] = {
{ TYPE_S390_STATTRIB, "migration-enabled", "off", },
+ { TYPE_S390_FLIC_COMMON, "migration-enabled", "off", },
};
ccw_machine_2_10_class_options(mc);
--
2.45.0On 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;
> }
>
> const VMStateDescription vmstate_adapter_info_so = {
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index feabc173eb3..1383e47eeb5 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -1174,6 +1174,7 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
> S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
> static GlobalProperty compat[] = {
> { TYPE_S390_STATTRIB, "migration-enabled", "off", },
> + { TYPE_S390_FLIC_COMMON, "migration-enabled", "off", },
> };
>
> ccw_machine_2_10_class_options(mc);
> --
> 2.45.0
>
>
This patch causes QEMU to crash when trying to save the domain state
(e.g. using libvirt)
$ cat dom.xml
<domain type='kvm'>
<name>bug</name>
<memory unit='KiB'>1048576</memory>
<vcpu placement='static'>2</vcpu>
<os>
<type arch='s390x' machine='s390-ccw-virtio-9.1'>hvm</type>
<kernel>/var/lib/libvirt/images/hades/vmlinux-s390x</kernel>
<initrd>/var/lib/libvirt/images/hades/ramdisk-s390x</initrd>
</os>
<devices>
<emulator>/usr/bin/qemu-system-s390x</emulator>
<console type='pty'>
<target type='sclp' port='0'/>
</console>
</devices>
</domain>
$ virsh create dom.xml
Domain 'bug' created from dom.xml
$ virsh save bug data
error: Failed to save domain 'bug' to data
error: operation failed: domain is not running
$ coredumpctl gdb
(gdb) bt
#0 object_dynamic_cast_assert (obj=0x2aa364aedd0, typename=typename@entry=0x2aa3491bd56 "s390-flic", file=file@entry=0x2aa34920c7a "/root/git/qemu/include/hw/s390x/s390_flic.h", line=line@entry=42, func=func@entry=0x2aa34a4b964 <__func__.14> "S390_FLIC_COMMON")
at ../qom/object.c:923
#1 0x000002aa3459b518 in S390_FLIC_COMMON (obj=<optimized out>) at /root/git/qemu/include/hw/s390x/s390_flic.h:42
#2 adapter_info_so_needed (opaque=<optimized out>) at ../hw/intc/s390_flic.c:462
#3 0x000002aa348b24dc in vmstate_section_needed (vmsd=0x2aa34c80fe0 <vmstate_adapter_info_so>, opaque=<optimized out>) at ../migration/vmstate.c:330
#4 vmstate_subsection_save (f=0x2aa36602bd0, vmsd=0x2aa34c80f78 <vmstate_adapter_info>, opaque=0x2aa364aedd0, vmdesc=0x0) at ../migration/vmstate.c:528
#5 vmstate_save_state_v (f=f@entry=0x2aa36602bd0, vmsd=0x2aa34c80f78 <vmstate_adapter_info>, opaque=opaque@entry=0x2aa364aedd0, vmdesc=vmdesc@entry=0x0, version_id=version_id@entry=1, errp=0x0) at ../migration/vmstate.c:443
#6 0x000002aa348b2886 in vmstate_save_state (f=0x2aa36602bd0, vmsd=<optimized out>, opaque=0x2aa364aedd0, vmdesc_id=0x0) at ../migration/vmstate.c:341
#7 vmstate_save_state_v (f=f@entry=0x2aa36602bd0, vmsd=0x2aa34c80e50 <vmstate_adapter_routes>, opaque=opaque@entry=0x2aa364aedd0, vmdesc=vmdesc@entry=0x0, version_id=version_id@entry=1, errp=0x0) at ../migration/vmstate.c:401
#8 0x000002aa348b2886 in vmstate_save_state (f=0x2aa36602bd0, vmsd=<optimized out>, opaque=0x2aa364aedd0, vmdesc_id=0x0) at ../migration/vmstate.c:341
#9 vmstate_save_state_v (f=0x2aa36602bd0, vmsd=0x2aa34c6cdf0 <vmstate_virtio_ccw_dev>, opaque=<optimized out>, vmdesc=0x0, version_id=version_id@entry=1, errp=0x0) at ../migration/vmstate.c:401
#10 0x000002aa348b2d7e in vmstate_save_state (f=<optimized out>, vmsd=<optimized out>, opaque=<optimized out>, vmdesc_id=<optimized out>) at ../migration/vmstate.c:341
#11 0x000002aa345c9726 in virtio_save (vdev=0x2aa364afe20, f=0x2aa36602bd0) at ../hw/virtio/virtio.c:2808
#12 0x000002aa348b23de in vmstate_save_state_v (f=f@entry=0x2aa36602bd0, vmsd=0x2aa34c80cd0 <vmstate_virtio_console>, opaque=<optimized out>, vmdesc=vmdesc@entry=0x2aa36602280, version_id=version_id@entry=3, errp=0x3ff73efb438) at ../migration/vmstate.c:408
#13 0x000002aa348b2dbe in vmstate_save_state_with_err (f=f@entry=0x2aa36602bd0, vmsd=<optimized out>, opaque=<optimized out>, vmdesc_id=vmdesc_id@entry=0x2aa36602280, errp=errp@entry=0x3ff73efb438) at ../migration/vmstate.c:347
#14 0x000002aa344993ce in vmstate_save (f=f@entry=0x2aa36602bd0, se=se@entry=0x2aa365cac80, vmdesc=vmdesc@entry=0x2aa36602280, errp=<optimized out>, errp@entry=0x3ff73efb438) at ../migration/savevm.c:1037
#15 0x000002aa3449cb80 in qemu_savevm_state_complete_precopy_non_iterable (f=f@entry=0x2aa36602bd0, in_postcopy=<optimized out>, in_postcopy@entry=false, inactivate_disks=false, inactivate_disks@entry=true) at ../migration/savevm.c:1554
#16 0x000002aa3449d15a in qemu_savevm_state_complete_precopy (f=0x2aa36602bd0, iterable_only=iterable_only@entry=false, inactivate_disks=false) at ../migration/savevm.c:1630
#17 0x000002aa3448ca00 in migration_completion_precopy (s=0x2aa3625d1f0, current_active_state=0x3ff73efb67c) at ../migration/migration.c:2710
#18 migration_completion (s=0x2aa3625d1f0) at ../migration/migration.c:2774
#19 migration_iteration_run (s=0x2aa3625d1f0) at ../migration/migration.c:3198
#20 migration_thread (opaque=opaque@entry=0x2aa3625d1f0) at ../migration/migration.c:3464
#21 0x000002aa3483bc12 in qemu_thread_start (args=<optimized out>) at ../util/qemu-thread-posix.c:541
#22 0x000003ff91bac3fa in start_thread () at /lib64/libc.so.6
#23 0x000003ff91c2bb18 in thread_start () at /lib64/libc.so.6
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
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
On 09/05/2024 19.00, Paolo Bonzini 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(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
(BTW: It's really good that Daniel's patch series is going to mark the old
machine types as deprecated, too ... migration stuff has been so hacky in
the 2.x days...)
> 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;
> }
>
> const VMStateDescription vmstate_adapter_info_so = {
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index feabc173eb3..1383e47eeb5 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -1174,6 +1174,7 @@ static void ccw_machine_2_9_class_options(MachineClass *mc)
> S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
> static GlobalProperty compat[] = {
> { TYPE_S390_STATTRIB, "migration-enabled", "off", },
> + { TYPE_S390_FLIC_COMMON, "migration-enabled", "off", },
> };
>
> ccw_machine_2_10_class_options(mc);
© 2016 - 2024 Red Hat, Inc.