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

[Hitoshi Mitake]

At Thu, 1 Aug 2013 12:32:51 +0800,
Liu Yuan wrote:
> 
> On Thu, Aug 01, 2013 at 01:03:08PM +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_progress. The struct contains these information:
> > 1. state of recovery. With this information, collie can know which
> >    phase of progress sheep is in.
> > 2. Number of copied object during a recovery process.
> > 3. Number of entire objects which should be copied during a recovery
> >    process.
> > 
> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > ---
> > 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()
> > 
> >  include/internal_proto.h |   12 ++++++++++++
> >  sheep/ops.c              |   11 ++++++++---
> >  sheep/recovery.c         |   19 +++++++++++++------
> >  sheep/sheep_priv.h       |    1 +
> >  4 files changed, 34 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/internal_proto.h b/include/internal_proto.h
> > index 0061007..9fab5eb 100644
> > --- a/include/internal_proto.h
> > +++ b/include/internal_proto.h
> > @@ -192,4 +192,16 @@ 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_progress {
> > +	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..60da83f 100644
> > --- a/sheep/ops.c
> > +++ b/sheep/ops.c
> > @@ -390,10 +390,15 @@ 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;
> > +	if (!node_in_recovery())
> > +		return SD_RES_SUCCESS;
> >  
> > -	return SD_RES_SUCCESS;
> > +	if (req->data_length == sizeof(struct recovery_progress)) {
> 
> This check is not necessary if we get a more generic name, such as
> recovery_state, which is easily to extend to get the information more than
> progress status. So return recovery_state become a must-call function for
> local_stat_recovery.

OK, it seems that renaming recovery_progress -> recovery_state and let
it have a flag for indicating in recovery or not would be simpler.

I'll do this in v3.

Thanks,
Hitoshi