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

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

[Halil Pasic]

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.

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)
-- 
2.13.5

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

[Cornelia Huck]

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?

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

[Halil Pasic]

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

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 <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?
> 
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 <borntraeger@de.ibm.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

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

[Cornelia Huck]

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.
> 
> 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(-)

Thanks, applied.