[sheepdog] [PATCH v2 2/2] collie: add a new option --progress to "node recovery" for showing recovery progress

[Liu Yuan]

On Thu, Aug 01, 2013 at 01:03:09PM +0900, Hitoshi Mitake wrote:
> This patch adds a new option --progress (or -P) to the node recovery
> subcommand. With this subcommand, users can show a progress of
> recovery process.
> 
> Example:
> $ sudo collie node recovery --progress
>  99.7 % [==============================================>] 7047 / 7068
> 
> The denominator (7068 in the above case) indicates a number of entire
> object which should be checked. The numerator (7047 in the above case)
> indicates a number of objects which is already checked or copied.
> 
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> ---
> v2:
>  - make this feature as an option of "node recovery", not a new subcommand
>  - clean coding style
>  -- renaming recovery_progress_unit() -> get_recovery_progress()
> 
>  collie/node.c |  110 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 108 insertions(+), 2 deletions(-)
> 
> diff --git a/collie/node.c b/collie/node.c
> index 69229f4..a1392b0 100644
> --- a/collie/node.c
> +++ b/collie/node.c
> @@ -13,6 +13,7 @@
>  
>  static struct node_cmd_data {
>  	bool all_nodes;
> +	bool recovery_progress;
>  } node_cmd_data;
>  
>  static void cal_total_vdi_size(uint32_t vid, const char *name, const char *tag,
> @@ -120,10 +121,111 @@ static int node_info(int argc, char **argv)
>  	return EXIT_SUCCESS;
>  }
>  
> +/*
> + * get_recovery_progress()
> + *
> + * Returned values:
> + * -1 ... request failed
> + *  0 ... recovery ended
> + *  1 ... recovery is continuing
> + */
> +static bool get_recovery_progress(struct recovery_progress *prog)
> +{

bool means true or false.

> +	int ret;
> +	struct sd_req req;
> +	struct sd_rsp *rsp = (struct sd_rsp *)&req;
> +
> +	sd_init_req(&req, SD_OP_STAT_RECOVERY);
> +	req.data_length = sizeof(*prog);
> +
> +	ret = collie_exec_req(sdhost, sdport, &req, prog);
> +	if (ret < 0) {
> +		fprintf(stderr, "Failed to execute request\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	switch (rsp->result) {
> +	case SD_RES_SUCCESS:
> +		ret = 0;
> +		break;
> +	case SD_RES_NODE_IN_RECOVERY:
> +		ret = 1;
> +		break;
> +	default:
> +		fprintf(stderr, "obtaining recovery progress fail: %s\n",
> +			sd_strerror(ret));
> +		ret = -1;
> +		break;
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +static int node_recovery_progress(void)
> +{
> +	int status, prev_status = -2;
> +
> +	/*
> +	 * prev_status is required for expressing state transition, and -2
> +	 * indicates the previous state is not initialized
> +	 */
> +
> +	/*
> +	 * ToDos
> +	 *
> +	 * 1. Calculate size of actually copied objects.
> +	 *    For doing this, not so trivial changes for recovery process are
> +	 *    required.
> +	 *
> +	 * 2. Print remaining physical time.
> +	 *    Even if it is not so acculate, it is helpful for administrators.
> +	 */
> +
> +	do {
> +		struct recovery_progress prog;
> +
> +		status = get_recovery_progress(&prog);
> +		if (status != 1) {
> +			if (status == 0 && prev_status != -2)
> +				/* not an immediate completion */
> +				show_progress(prog.nr_total, prog.nr_total,
> +					true);
> +
> +			break;
> +		}
> +

I can't understand what you are doing here. Just have get_recovery_progress()
return true:node_in_recovery, false:node_not_in_recovery, isn't enough?

Thanks
Yuan