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

[Christian Borntraeger]

On 10/04/2017 01:10 PM, Cornelia Huck wrote:
> On Wed,  4 Oct 2017 13:01:09 +0200
> Halil Pasic <address@hidden> 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 <address@hidden>
>> Reported-by: Thomas Huth <address@hidden>
>> ---
>>  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?
> 
FWIW, I have now a scripts that tests all kind of possible combinations.
As things are not broken without this patch in the sense of migration fails,
it is just that the 2.11 machine does not migrate the state, while the 2.10
machine does. 

The script does survive all combinations (2.11 with 2.10+our scheduled fixes 
for 2.10.1)
with this patch on top of 2.11

Tested-by: Christian Borntraeger <address@hidden>
Acked-by: Christian Borntraeger <address@hidden>