| Message ID | 20191120114334.2287-12-frankja@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: Protected Virtualization support | expand |
On 20.11.19 12:43, Janosch Frank wrote: > If a vcpu is not properly reset it might be better to just end the VM. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > target/s390x/kvm.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c > index 190400df55..0210b54157 100644 > --- a/target/s390x/kvm.c > +++ b/target/s390x/kvm.c > @@ -418,11 +418,13 @@ static void kvm_s390_reset_vcpu(S390CPU *cpu, unsigned long type) > if (kvm_vcpu_ioctl(cs, KVM_S390_VCPU_RESET, type)) { > error_report("CPU reset type %ld failed on CPU %i", > type, cs->cpu_index); > + exit(1); > } > return; > } > if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) { > error_report("Initial CPU reset failed on CPU %i", cs->cpu_index); > + exit(1); > } > } > > According to the comment in include/qapi/error.h "Please don't error_setg(&error_fatal, ...), use error_report() and exit(), because that's more obvious." This is the right thing to do. ... and it's a fairly pathological thing to happen either way. Reviewed-by: David Hildenbrand <david@redhat.com>
On 11/21/19 1:14 PM, David Hildenbrand wrote: > On 20.11.19 12:43, Janosch Frank wrote: >> If a vcpu is not properly reset it might be better to just end the VM. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> target/s390x/kvm.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >> index 190400df55..0210b54157 100644 >> --- a/target/s390x/kvm.c >> +++ b/target/s390x/kvm.c >> @@ -418,11 +418,13 @@ static void kvm_s390_reset_vcpu(S390CPU *cpu, unsigned long type) >> if (kvm_vcpu_ioctl(cs, KVM_S390_VCPU_RESET, type)) { >> error_report("CPU reset type %ld failed on CPU %i", >> type, cs->cpu_index); >> + exit(1); >> } >> return; >> } >> if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) { >> error_report("Initial CPU reset failed on CPU %i", cs->cpu_index); >> + exit(1); >> } >> } >> >> > > According to the comment in include/qapi/error.h > > "Please don't error_setg(&error_fatal, ...), use error_report() and > exit(), because that's more obvious." > > This is the right thing to do. > > ... and it's a fairly pathological thing to happen either way. > > Reviewed-by: David Hildenbrand <david@redhat.com> > Do we want to have that separate or should I squash it into the reset changes?
On 21.11.19 13:19, Janosch Frank wrote: > On 11/21/19 1:14 PM, David Hildenbrand wrote: >> On 20.11.19 12:43, Janosch Frank wrote: >>> If a vcpu is not properly reset it might be better to just end the VM. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> target/s390x/kvm.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c >>> index 190400df55..0210b54157 100644 >>> --- a/target/s390x/kvm.c >>> +++ b/target/s390x/kvm.c >>> @@ -418,11 +418,13 @@ static void kvm_s390_reset_vcpu(S390CPU *cpu, unsigned long type) >>> if (kvm_vcpu_ioctl(cs, KVM_S390_VCPU_RESET, type)) { >>> error_report("CPU reset type %ld failed on CPU %i", >>> type, cs->cpu_index); >>> + exit(1); >>> } >>> return; >>> } >>> if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) { >>> error_report("Initial CPU reset failed on CPU %i", cs->cpu_index); >>> + exit(1); >>> } >>> } >>> >>> >> >> According to the comment in include/qapi/error.h >> >> "Please don't error_setg(&error_fatal, ...), use error_report() and >> exit(), because that's more obvious." >> >> This is the right thing to do. >> >> ... and it's a fairly pathological thing to happen either way. >> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> > > Do we want to have that separate or should I squash it into the reset > changes? > > I' keep it separated.
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c index 190400df55..0210b54157 100644 --- a/target/s390x/kvm.c +++ b/target/s390x/kvm.c @@ -418,11 +418,13 @@ static void kvm_s390_reset_vcpu(S390CPU *cpu, unsigned long type) if (kvm_vcpu_ioctl(cs, KVM_S390_VCPU_RESET, type)) { error_report("CPU reset type %ld failed on CPU %i", type, cs->cpu_index); + exit(1); } return; } if (kvm_vcpu_ioctl(cs, KVM_S390_INITIAL_RESET, NULL)) { error_report("Initial CPU reset failed on CPU %i", cs->cpu_index); + exit(1); } }
If a vcpu is not properly reset it might be better to just end the VM. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- target/s390x/kvm.c | 2 ++ 1 file changed, 2 insertions(+)