Re: [Qemu-devel] [PATCH COLO-Frame v10 19/38] COLO failover: Introduce state to record failover process

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* zhanghailiang (zhang.zhanghailiang@huawei.com) wrote:
> When handling failover, we do different things according to the different stage
> of failover process, here we introduce a global atomic variable to record the
> status of failover.
> 
> We add four failover status to indicate the different stage of failover process.
> You should use the helpers to get and set the value.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  include/migration/failover.h | 10 ++++++++++
>  migration/colo-failover.c    | 37 +++++++++++++++++++++++++++++++++++++
>  migration/colo.c             |  4 ++++
>  trace-events                 |  1 +
>  4 files changed, 52 insertions(+)
> 
> diff --git a/include/migration/failover.h b/include/migration/failover.h
> index 1785b52..882c625 100644
> --- a/include/migration/failover.h
> +++ b/include/migration/failover.h
> @@ -15,6 +15,16 @@
>  
>  #include "qemu-common.h"
>  
> +typedef enum COLOFailoverStatus {
> +    FAILOVER_STATUS_NONE = 0,
> +    FAILOVER_STATUS_REQUEST = 1, /* Request but not handled */
> +    FAILOVER_STATUS_HANDLING = 2, /* In the process of handling failover */
> +    FAILOVER_STATUS_COMPLETED = 3, /* Finish the failover process */
> +} COLOFailoverStatus;

OK - there's a couple of typo's later, but other than those:

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> +
> +void failover_init_state(void);
> +int failover_set_state(int old_state, int new_state);
> +int failover_get_state(void);
>  void failover_request_active(Error **errp);
>  
>  #endif
> diff --git a/migration/colo-failover.c b/migration/colo-failover.c
> index e3897c6..ae06c16 100644
> --- a/migration/colo-failover.c
> +++ b/migration/colo-failover.c
> @@ -14,22 +14,59 @@
>  #include "migration/failover.h"
>  #include "qmp-commands.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qemu/error-report.h"
> +#include "trace.h"
>  
>  static QEMUBH *failover_bh;
> +static COLOFailoverStatus failover_state;
>  
>  static void colo_failover_bh(void *opaque)
>  {
> +    int old_state;
> +
>      qemu_bh_delete(failover_bh);
>      failover_bh = NULL;
> +    old_state = failover_set_state(FAILOVER_STATUS_REQUEST,
> +                                   FAILOVER_STATUS_HANDLING);
> +    if (old_state != FAILOVER_STATUS_REQUEST) {
> +        error_report(" Unkown error for failover, old_state=%d", old_state);

Typo 'Unkown'

> +        return;
> +    }
>      /*TODO: Do failover work */
>  }
>  
>  void failover_request_active(Error **errp)
>  {
> +   if (failover_set_state(FAILOVER_STATUS_NONE, FAILOVER_STATUS_REQUEST)
> +         != FAILOVER_STATUS_NONE) {
> +        error_setg(errp, "COLO failover is already actived");
> +        return;
> +    }
>      failover_bh = qemu_bh_new(colo_failover_bh, NULL);
>      qemu_bh_schedule(failover_bh);
>  }
>  
> +void failover_init_state(void)
> +{
> +    failover_state = FAILOVER_STATUS_NONE;
> +}
> +
> +int failover_set_state(int old_state, int new_state)
> +{
> +    int old;
> +
> +    old = atomic_cmpxchg(&failover_state, old_state, new_state);;

Typo double ;;

> +    if (old == old_state) {
> +        trace_colo_failover_set_state(new_state);
> +    }
> +    return old;
> +}
> +
> +int failover_get_state(void)
> +{
> +    return atomic_read(&failover_state);
> +}
> +
>  void qmp_x_colo_lost_heartbeat(Error **errp)
>  {
>      if (get_colo_mode() == COLO_MODE_UNKNOWN) {
> diff --git a/migration/colo.c b/migration/colo.c
> index 64daee9..7732f60 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -194,6 +194,8 @@ static void colo_process_checkpoint(MigrationState *s)
>      int64_t current_time, checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>      int fd, ret = 0;
>  
> +    failover_init_state();
> +
>      /* Dup the fd of to_dst_file */
>      fd = dup(qemu_get_fd(s->to_dst_file));
>      if (fd == -1) {
> @@ -310,6 +312,8 @@ void *colo_process_incoming_thread(void *opaque)
>      migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>                        MIGRATION_STATUS_COLO);
>  
> +    failover_init_state();
> +
>      fd = dup(qemu_get_fd(mis->from_src_file));
>      if (fd < 0) {
>          ret = -errno;
> diff --git a/trace-events b/trace-events
> index c98bc13..61e89c7 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1502,6 +1502,7 @@ rdma_start_outgoing_migration_after_rdma_source_init(void) ""
>  colo_vm_state_change(const char *old, const char *new) "Change '%s' => '%s'"
>  colo_ctl_put(const char *msg, uint64_t value) "Send '%s' cmd, value: %" PRIu64""
>  colo_ctl_get(const char *msg) "Receive '%s' cmd"
> +colo_failover_set_state(int new_state) "new state %d"
>  
>  # kvm-all.c
>  kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK