[v1] s390x: Protected Virtualization support

[PATCH 11/15] RFC: s390x: Exit on vcpu reset error

Posted by Janosch Frank 6 years, 2 months ago

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);
     }
 }
 
-- 
2.20.1

Re: [PATCH 11/15] RFC: s390x: Exit on vcpu reset error

Posted by David Hildenbrand 6 years, 2 months ago

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>

-- 

Thanks,

David / dhildenb

Re: [PATCH 11/15] RFC: s390x: Exit on vcpu reset error

Posted by Janosch Frank 6 years, 2 months ago

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?

Re: [PATCH 11/15] RFC: s390x: Exit on vcpu reset error

Posted by David Hildenbrand 6 years, 2 months ago

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.

-- 

Thanks,

David / dhildenb