| Message ID | 1435742217-62246-1-git-send-email-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
Christian Borntraeger <borntraeger@de.ibm.com> wrote: First of all Reviewed-by: Juan Quintela <quintela@redhat.com> For the patch. But one said that, I don't agree with the commint text. > The section footer changes commit f68945d42bab ("Add a protective > section footer") and commit 37fb569c0198 ("Disable section footers > on older machine types") broke migration for any non-versioned > machines. If broke migration for 2.4 -> 2.3 for machines that don't care about compatibility. If they care, they are versioned O:-) Right now ppc & x86. I guess that s390 and arm will follow in due curse. > While one can argue that section footer should be enabled > explicitely for new versions instead of disabled for old ones, > this pinpoints to a problem of s390-ccw-machines: it needs to > be versioned to be compatible with future changes in common > code data structures such as section footers. It is done explicitely the other way around. New way is the default way. If we did it in a special way in the past, we add a switch. If at some point we remove the old machine type for any reason, we don't have to change anything on $latest. Basically the idea here is that (until now) only x86 cared about backwards compatibility, so when a migration changed happened because it was good for any reason, normal devices do changes, and then x86 try to fix the pieces after the fact. That is going to continue, just that now more architectures care, and then we should detect this kind of problems much earlier. > Let's introduce a version scheme for s390-ccw-virtio machines. > We will use the old s390-ccw-virtio name as alias to the latest > version as all existing libvirt XML for the ccw type were expanded > by libvirt to that name. > > The only downside of this patch is, that the old alias s390-ccw > will no longer be available as machines can have only one alias, > but it should not really matter. Should we change to a list? Later, Juan.
Am 01.07.2015 um 11:56 schrieb Juan Quintela: > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > > First of all > > Reviewed-by: Juan Quintela <quintela@redhat.com> > > For the patch. > > But one said that, I don't agree with the commint text. So let's just drop this sentence >> While one can argue that section footer should be enabled >> explicitely for new versions instead of disabled for old ones, And rephrase to " This pinpoints to a problem of s390-ccw-machines: it needs to be versioned to allow common code changes to add compat handling. " Conny, want me to resend or can you fixup the patch description when taking this patch? > > >> The section footer changes commit f68945d42bab ("Add a protective >> section footer") and commit 37fb569c0198 ("Disable section footers >> on older machine types") broke migration for any non-versioned >> machines. > > If broke migration for 2.4 -> 2.3 for machines that don't care about > compatibility. If they care, they are versioned O:-) Right now ppc & > x86. I guess that s390 and arm will follow in due curse. yes. That is what my 2nd sentence says: we are not versioned and that is the main issue to solve. >> this pinpoints to a problem of s390-ccw-machines: it needs to >> be versioned to be compatible with future changes in common >> code data structures such as section footers. > > It is done explicitely the other way around. New way is the default > way. If we did it in a special way in the past, we add a switch. If at > some point we remove the old machine type for any reason, we don't have > to change anything on $latest. > > Basically the idea here is that (until now) only x86 cared about > backwards compatibility, so when a migration changed happened because it > was good for any reason, normal devices do changes, and then x86 try to > fix the pieces after the fact. That is going to continue, just that now > more architectures care, and then we should detect this kind of problems > much earlier. > >> Let's introduce a version scheme for s390-ccw-virtio machines. >> We will use the old s390-ccw-virtio name as alias to the latest >> version as all existing libvirt XML for the ccw type were expanded >> by libvirt to that name. >> >> The only downside of this patch is, that the old alias s390-ccw >> will no longer be available as machines can have only one alias, >> but it should not really matter. > > Should we change to a list? list of aliases? Why not, we would use it.
Christian Borntraeger <borntraeger@de.ibm.com> wrote: > Am 01.07.2015 um 11:56 schrieb Juan Quintela: >> Christian Borntraeger <borntraeger@de.ibm.com> wrote: >> >> First of all >> >> Reviewed-by: Juan Quintela <quintela@redhat.com> >> >> For the patch. >> >> But one said that, I don't agree with the commint text. > > So let's just drop this sentence >>> While one can argue that section footer should be enabled >>> explicitely for new versions instead of disabled for old ones, > > And rephrase to > > " > This pinpoints to a problem of s390-ccw-machines: it needs to > be versioned to allow common code changes to add compat handling. > " > > > > Conny, want me to resend or can you fixup the patch description when > taking this patch? > >> >> >>> The section footer changes commit f68945d42bab ("Add a protective >>> section footer") and commit 37fb569c0198 ("Disable section footers >>> on older machine types") broke migration for any non-versioned >>> machines. >> >> If broke migration for 2.4 -> 2.3 for machines that don't care about >> compatibility. If they care, they are versioned O:-) Right now ppc & >> x86. I guess that s390 and arm will follow in due curse. > > yes. That is what my 2nd sentence says: we are not versioned and that is > the main issue to solve. You do it, so you are well. Thanks. >> was good for any reason, normal devices do changes, and then x86 try to >> fix the pieces after the fact. That is going to continue, just that now >> more architectures care, and then we should detect this kind of problems >> much earlier. >> >>> Let's introduce a version scheme for s390-ccw-virtio machines. >>> We will use the old s390-ccw-virtio name as alias to the latest >>> version as all existing libvirt XML for the ccw type were expanded >>> by libvirt to that name. >>> >>> The only downside of this patch is, that the old alias s390-ccw >>> will no longer be available as machines can have only one alias, >>> but it should not really matter. >> >> Should we change to a list? > > list of aliases? Why not, we would use it. Anyone for machine definitions reading this? More people need/wants it? Later, Juan
On Wed, 1 Jul 2015 11:16:57 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > The section footer changes commit f68945d42bab ("Add a protective > section footer") and commit 37fb569c0198 ("Disable section footers > on older machine types") broke migration for any non-versioned > machines. > > While one can argue that section footer should be enabled > explicitely for new versions instead of disabled for old ones, > this pinpoints to a problem of s390-ccw-machines: it needs to > be versioned to be compatible with future changes in common > code data structures such as section footers. > > Let's introduce a version scheme for s390-ccw-virtio machines. > We will use the old s390-ccw-virtio name as alias to the latest > version as all existing libvirt XML for the ccw type were expanded > by libvirt to that name. > > The only downside of this patch is, that the old alias s390-ccw > will no longer be available as machines can have only one alias, > but it should not really matter. > > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Juan Quintela <quintela@redhat.com> > Cc: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> > Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) Adapted the commit message and applied (with minor tweaks) to my s390-next branch at git://github.com/cohuck/qemu s390-next I'll probably send a pull request including this patch tomorrow, unless someone has further comments.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 47f323b..90bb159 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -159,9 +159,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) MachineClass *mc = MACHINE_CLASS(oc); NMIClass *nc = NMI_CLASS(oc); - mc->name = "s390-ccw-virtio"; - mc->alias = "s390-ccw"; - mc->desc = "VirtIO-ccw based S390 machine"; mc->init = ccw_init; mc->block_default_type = IF_VIRTIO; mc->no_cdrom = 1; @@ -171,7 +168,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) mc->no_sdcard = 1; mc->use_sclp = 1; mc->max_cpus = 255; - mc->is_default = 1; mc->hot_add_cpu = ccw_hot_add_cpu; nc->nmi_monitor_handler = s390_nmi; } @@ -228,6 +224,7 @@ static inline void s390_machine_initfn(Object *obj) static const TypeInfo ccw_machine_info = { .name = TYPE_S390_CCW_MACHINE, .parent = TYPE_MACHINE, + .abstract = true, .instance_size = sizeof(S390CcwMachineState), .instance_init = s390_machine_initfn, .class_init = ccw_machine_class_init, @@ -237,9 +234,26 @@ static const TypeInfo ccw_machine_info = { }, }; +static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data) +{ + MachineClass *mc = MACHINE_CLASS(oc); + + mc->name = "s390-ccw-virtio-2.4"; + mc->alias = "s390-ccw-virtio"; + mc->desc = "VirtIO-ccw based S390 machine v2.4"; + mc->is_default = 1; +} + +static const TypeInfo ccw_machine_2_4_info = { + .name = TYPE_S390_CCW_MACHINE "2.4", + .parent = TYPE_S390_CCW_MACHINE, + .class_init = ccw_machine_2_4_class_init, +}; + static void ccw_machine_register_types(void) { type_register_static(&ccw_machine_info); + type_register_static(&ccw_machine_2_4_info); } type_init(ccw_machine_register_types)
The section footer changes commit f68945d42bab ("Add a protective section footer") and commit 37fb569c0198 ("Disable section footers on older machine types") broke migration for any non-versioned machines. While one can argue that section footer should be enabled explicitely for new versions instead of disabled for old ones, this pinpoints to a problem of s390-ccw-machines: it needs to be versioned to be compatible with future changes in common code data structures such as section footers. Let's introduce a version scheme for s390-ccw-virtio machines. We will use the old s390-ccw-virtio name as alias to the latest version as all existing libvirt XML for the ccw type were expanded by libvirt to that name. The only downside of this patch is, that the old alias s390-ccw will no longer be available as machines can have only one alias, but it should not really matter. Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Cc: Juan Quintela <quintela@redhat.com> Cc: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)