| Message ID | 20171004110109.16525-1-pasic@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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?
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
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>
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.
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)
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(-)