diff mbox series

[v8,03/15] s390x: protvirt: Add migration blocker

Message ID 20200310134008.130038-4-frankja@linux.ibm.com
State New
Headers show
Series s390x: Protected Virtualization support | expand

Commit Message

Janosch Frank March 10, 2020, 1:39 p.m. UTC
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(+)

Comments

David Hildenbrand March 10, 2020, 2:57 p.m. UTC | #1
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;
>      }
>  
>
Janosch Frank March 10, 2020, 3:02 p.m. UTC | #2
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;
>>      }
>>  
>>
> 
>
David Hildenbrand March 10, 2020, 3:08 p.m. UTC | #3
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.
Janosch Frank March 10, 2020, 3:14 p.m. UTC | #4
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.
David Hildenbrand March 10, 2020, 3:26 p.m. UTC | #5
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 mbox series

Patch

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