[sheepdog] [PATCH v3 1/2] sheep: enhance STAT_RECOVERY for prividing information of recovery state

[Liu Yuan]

On Fri, Aug 02, 2013 at 04:52:05PM +0900, Hitoshi Mitake wrote:
> Current SD_OP_STAT_RECOVERY only provides an information that
> indicates the node is doing recovery or not. For providing more useful
> information about progress of recovery, this patch enhances this
> request.
> 
> The enhanced SD_OP_STAT_RECOVERY returns the information with struct
> recovery_state. The struct contains these information:
> 1. a flag which indicates a sheep process is in progress or not
> 2. state of recovery. With this information, collie can know which
>    phase of progress sheep is in.
> 3. Number of copied object during a recovery process.
> 4. Number of entire objects which should be copied during a recovery
>    process.
> 
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
> v3:
>  - make struct recovery_state a general mechanism for getting recovery
>    status. Ordinal "collie node recovery" uses struct recovery_state
>    for detecting recovery state instead of a result of request.
> 
> v2:
>  - don't use new variables for indicating the progress
>  - clean coding style
>  -- change names of struct recovery_info's members
>  -- fill_recovery_progress() -> get_recovery_progress()
> 
>  collie/node.c            |    8 +++++---
>  include/internal_proto.h |   13 +++++++++++++
>  sheep/ops.c              |    4 ++--
>  sheep/recovery.c         |   22 ++++++++++++++++------
>  sheep/sheep_priv.h       |    1 +
>  5 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/collie/node.c b/collie/node.c
> index 69229f4..4230af5 100644
> --- a/collie/node.c
> +++ b/collie/node.c
> @@ -132,17 +132,19 @@ static int node_recovery(int argc, char **argv)
>  	for (i = 0; i < sd_nodes_nr; i++) {
>  		char host[128];
>  		struct sd_req req;
> -		struct sd_rsp *rsp = (struct sd_rsp *)&req;
> +		struct recovery_state state;
>  
> +		memset(&state, 0, sizeof(state));
>  		addr_to_str(host, sizeof(host), sd_nodes[i].nid.addr, 0);
>  
>  		sd_init_req(&req, SD_OP_STAT_RECOVERY);
> +		req.data_length = sizeof(state);
>  
> -		ret = collie_exec_req(host, sd_nodes[i].nid.port, &req, NULL);
> +		ret = collie_exec_req(host, sd_nodes[i].nid.port, &req, &state);
>  		if (ret < 0)
>  			return EXIT_SYSFAIL;
>  
> -		if (rsp->result == SD_RES_NODE_IN_RECOVERY) {
> +		if (state.in_recovery) {
>  			addr_to_str(host, sizeof(host),
>  					sd_nodes[i].nid.addr, sd_nodes[i].nid.port);
>  			printf(raw_output ? "%d %s %d %d\n" : "%4d   %-20s%5d%11d\n",
> diff --git a/include/internal_proto.h b/include/internal_proto.h
> index 0061007..e42cd1b 100644
> --- a/include/internal_proto.h
> +++ b/include/internal_proto.h
> @@ -192,4 +192,17 @@ static inline __attribute__((used)) void __sd_epoch_format_build_bug_ons(void)
>  	BUILD_BUG_ON(sizeof(struct sd_node) != SD_NODE_SIZE);
>  }
>  
> +enum rw_state {
> +	RW_PREPARE_LIST, /* the recovery thread is preparing object list */
> +	RW_RECOVER_OBJ, /* the thread is recoering objects */
> +	RW_NOTIFY_COMPLETION, /* the thread is notifying recovery completion */
> +};
> +
> +struct recovery_state {
> +	uint32_t in_recovery;
> +	enum rw_state state;
> +	uint32_t nr_finished;
> +	uint32_t nr_total;
> +};
> +
>  #endif /* __INTERNAL_PROTO_H__ */
> diff --git a/sheep/ops.c b/sheep/ops.c
> index 5d7686c..c25aead 100644
> --- a/sheep/ops.c
> +++ b/sheep/ops.c
> @@ -390,8 +390,8 @@ static int local_stat_sheep(struct request *req)
>  static int local_stat_recovery(const struct sd_req *req, struct sd_rsp *rsp,
>  					void *data)
>  {
> -	if (node_in_recovery())
> -		return SD_RES_NODE_IN_RECOVERY;
> +	get_recovery_state(data);
> +	rsp->data_length = sizeof(struct recovery_state);
>  
>  	return SD_RES_SUCCESS;
>  }
> diff --git a/sheep/recovery.c b/sheep/recovery.c
> index ea3101a..43598a6 100644
> --- a/sheep/recovery.c
> +++ b/sheep/recovery.c
> @@ -11,12 +11,6 @@
>  
>  #include "sheep_priv.h"
>  
> -enum rw_state {
> -	RW_PREPARE_LIST, /* the recovery thread is preparing object list */
> -	RW_RECOVER_OBJ, /* the thread is recoering objects */
> -	RW_NOTIFY_COMPLETION, /* the thread is notifying recovery completion */
> -};
> -
>  /* base structure for the recovery thread */
>  struct recovery_work {
>  	uint32_t epoch;
> @@ -882,3 +876,19 @@ static void queue_recovery_work(struct recovery_info *rinfo)
>  
>  	queue_work(sys->recovery_wqueue, &rw->work);
>  }
> +
> +void get_recovery_state(struct recovery_state *state)
> +{
> +	struct recovery_info *rinfo = main_thread_get(current_rinfo);
> +
> +	if (!rinfo) {
> +		state->in_recovery = false;
> +		return;
> +	}
> +
> +	state->in_recovery = true;

->in_recovery isn't boolean, so assign it 1 or 0 instead of true or false.
Besides, use uint8_t for .in_recovery is good enough.

Thanks
Yuan