| Message ID | 20200310134008.130038-4-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On 10.03.20 14:39, Janosch Frank wrote: > Migration is not yet supported. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 455ad31718..1ceb42c0b5 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -43,6 +43,9 @@ > #include "sysemu/sysemu.h" > #include "hw/s390x/pv.h" > #include <linux/kvm.h> > +#include "migration/blocker.h" > + > +static Error *pv_mig_blocker; > > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > { > @@ -323,15 +326,29 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) > > s390_pv_vm_disable(); > ms->pv = false; > + migrate_del_blocker(pv_mig_blocker); > + error_free_or_abort(&pv_mig_blocker); > } > > static int s390_machine_protect(S390CcwMachineState *ms) > { > + static Error *local_err; You have to initialize it to NULL. > int rc; > > + error_setg(&pv_mig_blocker, > + "protected VMs are currently not migrateable."); > + rc = migrate_add_blocker(pv_mig_blocker, &local_err); > + if (local_err) { > + error_report_err(local_err); > + error_free_or_abort(&pv_mig_blocker); > + return rc; > + } > + > /* Create SE VM */ > rc = s390_pv_vm_enable(); > if (rc) { > + error_report_err(local_err); > + error_free_or_abort(&pv_mig_blocker); Actually, migrate_del_blocker(pv_mig_blocker) as well, no? > return rc; > } > >
On 3/10/20 3:57 PM, David Hildenbrand wrote: > On 10.03.20 14:39, Janosch Frank wrote: >> Migration is not yet supported. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index 455ad31718..1ceb42c0b5 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -43,6 +43,9 @@ >> #include "sysemu/sysemu.h" >> #include "hw/s390x/pv.h" >> #include <linux/kvm.h> >> +#include "migration/blocker.h" >> + >> +static Error *pv_mig_blocker; >> >> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) >> { >> @@ -323,15 +326,29 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) >> >> s390_pv_vm_disable(); >> ms->pv = false; >> + migrate_del_blocker(pv_mig_blocker); >> + error_free_or_abort(&pv_mig_blocker); >> } >> >> static int s390_machine_protect(S390CcwMachineState *ms) >> { >> + static Error *local_err; > > You have to initialize it to NULL. That's a static variable, am I missing something? > >> int rc; >> >> + error_setg(&pv_mig_blocker, >> + "protected VMs are currently not migrateable."); >> + rc = migrate_add_blocker(pv_mig_blocker, &local_err); >> + if (local_err) { >> + error_report_err(local_err); >> + error_free_or_abort(&pv_mig_blocker); >> + return rc; >> + } >> + >> /* Create SE VM */ >> rc = s390_pv_vm_enable(); >> if (rc) { >> + error_report_err(local_err); >> + error_free_or_abort(&pv_mig_blocker); > > Actually, migrate_del_blocker(pv_mig_blocker) as well, no? > Ack >> return rc; >> } >> >> > >
On 10.03.20 16:02, Janosch Frank wrote: > On 3/10/20 3:57 PM, David Hildenbrand wrote: >> On 10.03.20 14:39, Janosch Frank wrote: >>> Migration is not yet supported. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>> index 455ad31718..1ceb42c0b5 100644 >>> --- a/hw/s390x/s390-virtio-ccw.c >>> +++ b/hw/s390x/s390-virtio-ccw.c >>> @@ -43,6 +43,9 @@ >>> #include "sysemu/sysemu.h" >>> #include "hw/s390x/pv.h" >>> #include <linux/kvm.h> >>> +#include "migration/blocker.h" >>> + >>> +static Error *pv_mig_blocker; >>> >>> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) >>> { >>> @@ -323,15 +326,29 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) >>> >>> s390_pv_vm_disable(); >>> ms->pv = false; >>> + migrate_del_blocker(pv_mig_blocker); >>> + error_free_or_abort(&pv_mig_blocker); >>> } >>> >>> static int s390_machine_protect(S390CcwMachineState *ms) >>> { >>> + static Error *local_err; >> >> You have to initialize it to NULL. > > That's a static variable, am I missing something? No, I was missing that :) Why is that a static variable? git grep "static Error *" will tell you that this is not common.
On 3/10/20 4:08 PM, David Hildenbrand wrote: > On 10.03.20 16:02, Janosch Frank wrote: >> On 3/10/20 3:57 PM, David Hildenbrand wrote: >>> On 10.03.20 14:39, Janosch Frank wrote: >>>> Migration is not yet supported. >>>> >>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>>> --- >>>> hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ >>>> 1 file changed, 17 insertions(+) >>>> >>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>>> index 455ad31718..1ceb42c0b5 100644 >>>> --- a/hw/s390x/s390-virtio-ccw.c >>>> +++ b/hw/s390x/s390-virtio-ccw.c >>>> @@ -43,6 +43,9 @@ >>>> #include "sysemu/sysemu.h" >>>> #include "hw/s390x/pv.h" >>>> #include <linux/kvm.h> >>>> +#include "migration/blocker.h" >>>> + >>>> +static Error *pv_mig_blocker; >>>> >>>> S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) >>>> { >>>> @@ -323,15 +326,29 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) >>>> >>>> s390_pv_vm_disable(); >>>> ms->pv = false; >>>> + migrate_del_blocker(pv_mig_blocker); >>>> + error_free_or_abort(&pv_mig_blocker); >>>> } >>>> >>>> static int s390_machine_protect(S390CcwMachineState *ms) >>>> { >>>> + static Error *local_err; >>> >>> You have to initialize it to NULL. >> >> That's a static variable, am I missing something? > > No, I was missing that :) Why is that a static variable? > > git grep "static Error *" will tell you that this is not common. > I'd guess I copied the other Error which is static... So I now removed the static and set it to NULL.
On 10.03.20 16:24, Janosch Frank wrote: > Migration is not yet supported. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > hw/s390x/s390-virtio-ccw.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 455ad31718..f18d98bb32 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -43,6 +43,9 @@ > #include "sysemu/sysemu.h" > #include "hw/s390x/pv.h" > #include <linux/kvm.h> > +#include "migration/blocker.h" > + > +static Error *pv_mig_blocker; > > S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) > { > @@ -323,15 +326,30 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) > > s390_pv_vm_disable(); > ms->pv = false; > + migrate_del_blocker(pv_mig_blocker); > + error_free_or_abort(&pv_mig_blocker); > } > > static int s390_machine_protect(S390CcwMachineState *ms) > { > + Error *local_err = NULL; > int rc; > > + error_setg(&pv_mig_blocker, > + "protected VMs are currently not migrateable."); > + rc = migrate_add_blocker(pv_mig_blocker, &local_err); > + if (local_err) { > + error_report_err(local_err); > + error_free_or_abort(&pv_mig_blocker); > + return rc; > + } > + > /* Create SE VM */ > rc = s390_pv_vm_enable(); > if (rc) { > + error_report_err(local_err); > + migrate_del_blocker(pv_mig_blocker); > + error_free_or_abort(&pv_mig_blocker); > return rc; > } > > Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 455ad31718..1ceb42c0b5 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -43,6 +43,9 @@ #include "sysemu/sysemu.h" #include "hw/s390x/pv.h" #include <linux/kvm.h> +#include "migration/blocker.h" + +static Error *pv_mig_blocker; S390CPU *s390_cpu_addr2state(uint16_t cpu_addr) { @@ -323,15 +326,29 @@ static void s390_machine_unprotect(S390CcwMachineState *ms) s390_pv_vm_disable(); ms->pv = false; + migrate_del_blocker(pv_mig_blocker); + error_free_or_abort(&pv_mig_blocker); } static int s390_machine_protect(S390CcwMachineState *ms) { + static Error *local_err; int rc; + error_setg(&pv_mig_blocker, + "protected VMs are currently not migrateable."); + rc = migrate_add_blocker(pv_mig_blocker, &local_err); + if (local_err) { + error_report_err(local_err); + error_free_or_abort(&pv_mig_blocker); + return rc; + } + /* Create SE VM */ rc = s390_pv_vm_enable(); if (rc) { + error_report_err(local_err); + error_free_or_abort(&pv_mig_blocker); return rc; }
Migration is not yet supported. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)