Re: [Qemu-devel] [PATCH 1/1] s390x/css: fix css migration compat handling

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 10/04/2017 01:10 PM, Cornelia Huck wrote:
> On Wed,  4 Oct 2017 13:01:09 +0200
> Halil Pasic <pasic@linux.vnet.ibm.com> wrote:
> 
>> Commit e996583eb3 ("s390x/css: activate ChannelSubSys migration",
>> 2017-07-11) was supposed to enable css migration for virtio-ccw
>> machines starting 2.10, but it ended up effectively enabling it
>> only for 2.10 as the registration of the appropriate VMStateDescription
>> happens in ccw_machine_2_10_instance_options which does not get
>> called for machines more recent than 2_10.
>>
>> Let us move the corresponding chunk of code (which conditionally enables
>> the migration based on the value of the corresponding class property) to
>> a ccw_init, which is called for each virtio-ccw machine instance.
> 
> s/a//
> 
>>
>> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> Reported-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  hw/s390x/s390-virtio-ccw.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index fafbc6d4fe..3b5dfdb48d 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -313,6 +313,9 @@ static void ccw_init(MachineState *machine)
>>          ret = css_create_css_image(VIRTUAL_CSSID, true);
>>      }
>>      assert(ret == 0);
>> +    if (css_migration_enabled()) {
>> +        css_register_vmstate();
>> +    }
>>  
>>      /* Create VirtIO network adapters */
>>      s390_create_virtio_net(BUS(css_bus), "virtio-net-ccw");
>> @@ -731,9 +734,6 @@ DEFINE_CCW_MACHINE(2_11, "2.11", true);
>>  static void ccw_machine_2_10_instance_options(MachineState *machine)
>>  {
>>      ccw_machine_2_11_instance_options(machine);
>> -    if (css_migration_enabled()) {
>> -        css_register_vmstate();
>> -    }
>>  }
>>  
>>  static void ccw_machine_2_10_class_options(MachineClass *mc)
> 
> Looks sane.
> 
> Out of curiosity: Which machine types did you test migration for?
> 
In context of this patch. I've double checked with gdb that
css_register_vmstate is called for 2.11, 2.10 and that it is not called
for 2.9 and 2.8.

I did not do any actual migrations before sending the patch.  The mean
thing is that the migration between 2.11 and 2.11 does not fail
(necessarily) it's "just" that the stuff described by vmstate_css won't
get migrated (AFAIR that means we get the defaults instead).

Regards,
Halil