diff mbox series

[v12,01/21] migration: Set error state in case of error

Message ID 20180425112723.1111-2-quintela@redhat.com
State New
Headers show
Series Multifd | expand

Commit Message

Juan Quintela April 25, 2018, 11:27 a.m. UTC 
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Comments

Dr. David Alan Gilbert May 2, 2018, 3:53 p.m. UTC | #1 
* Juan Quintela (quintela@redhat.com) wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/ram.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 0e90efa092..2ae560ea80 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -415,10 +415,20 @@ struct {
>      int count;
>  } *multifd_send_state;
>  
> -static void terminate_multifd_send_threads(Error *errp)
> +static void terminate_multifd_send_threads(Error *err)
>  {
>      int i;
>  
> +    if (err) {
> +        MigrationState *s = migrate_get_current();
> +        migrate_set_error(s, err);
> +        if (s->state == MIGRATION_STATUS_SETUP ||
> +            s->state == MIGRATION_STATUS_ACTIVE) {

Can you explain/add comment why these only set it in some states?
For example what about PRE_SWITCHOVER, DEVICE or postcopy?

Dave

> +            migrate_set_state(&s->state, s->state,
> +                              MIGRATION_STATUS_FAILED);
> +        }
> +    }
> +
>      for (i = 0; i < multifd_send_state->count; i++) {
>          MultiFDSendParams *p = &multifd_send_state->params[i];
>  
> @@ -515,10 +525,20 @@ struct {
>      int count;
>  } *multifd_recv_state;
>  
> -static void terminate_multifd_recv_threads(Error *errp)
> +static void terminate_multifd_recv_threads(Error *err)
>  {
>      int i;
>  
> +    if (err) {
> +        MigrationState *s = migrate_get_current();
> +        migrate_set_error(s, err);
> +        if (s->state == MIGRATION_STATUS_SETUP ||
> +            s->state == MIGRATION_STATUS_ACTIVE) {
> +            migrate_set_state(&s->state, s->state,
> +                              MIGRATION_STATUS_FAILED);
> +        }
> +    }
> +
>      for (i = 0; i < multifd_recv_state->count; i++) {
>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
>  
> -- 
> 2.17.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Juan Quintela May 9, 2018, 8:15 a.m. UTC | #2 
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/ram.c | 24 ++++++++++++++++++++++--
>>  1 file changed, 22 insertions(+), 2 deletions(-)
>> 
>> diff --git a/migration/ram.c b/migration/ram.c
>> index 0e90efa092..2ae560ea80 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -415,10 +415,20 @@ struct {
>>      int count;
>>  } *multifd_send_state;
>>  
>> -static void terminate_multifd_send_threads(Error *errp)
>> +static void terminate_multifd_send_threads(Error *err)
>>  {
>>      int i;
>>  
>> +    if (err) {
>> +        MigrationState *s = migrate_get_current();
>> +        migrate_set_error(s, err);
>> +        if (s->state == MIGRATION_STATUS_SETUP ||
>> +            s->state == MIGRATION_STATUS_ACTIVE) {
>
> Can you explain/add comment why these only set it in some states?
> For example what about PRE_SWITCHOVER, DEVICE or postcopy?

This were the ones needed when I did the patch? O:-)

Ok, looking at all of them:

    MIGRATION_STATUS_NONE = 0,

We can't be on that state

    MIGRATION_STATUS_SETUP = 1,
There

MIGRATION_STATUS_CANCELLING = 2,
    MIGRATION_STATUS_CANCELLED = 3,

We don't want to touch in the case of that two.

    MIGRATION_STATUS_ACTIVE = 4,

There.

    MIGRATION_STATUS_POSTCOPY_ACTIVE = 5,
    MIGRATION_STATUS_POSTCOPY_PAUSED = 6,

We are not supporing those yet.

    MIGRATION_STATUS_COMPLETED = 7,

This is impossible (TM)

    MIGRATION_STATUS_FAILED = 8,

We don't want to change.

    MIGRATION_STATUS_COLO = 9,

Not contemplated yet
    MIGRATION_STATUS_PRE_SWITCHOVER = 10,
    MIGRATION_STATUS_DEVICE = 11,

Needed.
    MIGRATION_STATUS__MAX = 12,

Irrelelevant.

So, I agree with your suggestion.

Later, Juan.


> Dave
>
>> +            migrate_set_state(&s->state, s->state,
>> +                              MIGRATION_STATUS_FAILED);
>> +        }
>> +    }
>> +
>>      for (i = 0; i < multifd_send_state->count; i++) {
>>          MultiFDSendParams *p = &multifd_send_state->params[i];
>>  
>> @@ -515,10 +525,20 @@ struct {
>>      int count;
>>  } *multifd_recv_state;
>>  
>> -static void terminate_multifd_recv_threads(Error *errp)
>> +static void terminate_multifd_recv_threads(Error *err)
>>  {
>>      int i;
>>  
>> +    if (err) {
>> +        MigrationState *s = migrate_get_current();
>> +        migrate_set_error(s, err);
>> +        if (s->state == MIGRATION_STATUS_SETUP ||
>> +            s->state == MIGRATION_STATUS_ACTIVE) {
>> +            migrate_set_state(&s->state, s->state,
>> +                              MIGRATION_STATUS_FAILED);
>> +        }
>> +    }
>> +
>>      for (i = 0; i < multifd_recv_state->count; i++) {
>>          MultiFDRecvParams *p = &multifd_recv_state->params[i];
>>  
>> -- 
>> 2.17.0
>> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff mbox series

Patch

diff --git a/migration/ram.c b/migration/ram.c
index 0e90efa092..2ae560ea80 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -415,10 +415,20 @@  struct {
     int count;
 } *multifd_send_state;
 
-static void terminate_multifd_send_threads(Error *errp)
+static void terminate_multifd_send_threads(Error *err)
 {
     int i;
 
+    if (err) {
+        MigrationState *s = migrate_get_current();
+        migrate_set_error(s, err);
+        if (s->state == MIGRATION_STATUS_SETUP ||
+            s->state == MIGRATION_STATUS_ACTIVE) {
+            migrate_set_state(&s->state, s->state,
+                              MIGRATION_STATUS_FAILED);
+        }
+    }
+
     for (i = 0; i < multifd_send_state->count; i++) {
         MultiFDSendParams *p = &multifd_send_state->params[i];
 
@@ -515,10 +525,20 @@  struct {
     int count;
 } *multifd_recv_state;
 
-static void terminate_multifd_recv_threads(Error *errp)
+static void terminate_multifd_recv_threads(Error *err)
 {
     int i;
 
+    if (err) {
+        MigrationState *s = migrate_get_current();
+        migrate_set_error(s, err);
+        if (s->state == MIGRATION_STATUS_SETUP ||
+            s->state == MIGRATION_STATUS_ACTIVE) {
+            migrate_set_state(&s->state, s->state,
+                              MIGRATION_STATUS_FAILED);
+        }
+    }
+
     for (i = 0; i < multifd_recv_state->count; i++) {
         MultiFDRecvParams *p = &multifd_recv_state->params[i];