[sheepdog] [PATCH] sheep: split create_vdi_obj() into 4 functions

Liu Yuan namei.unix at gmail.com
Thu Aug 22 11:00:24 CEST 2013


On Thu, Aug 22, 2013 at 04:57:30PM +0800, Liu Yuan wrote:
> On Thu, Aug 22, 2013 at 04:53:48PM +0900, MORITA Kazutaka wrote:
> > From: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > 
> > create_vdi_obj() is very complex because it contains four kinds of
> > operations:
> > 
> >  - create fresh vdi
> >  - clone vdi
> >  - snapshot vdi
> >  - rebase vdi (rollback)
> > 
> > Splitting them apart make the code much easier to read.
> > 
> > This patch is just a cleanup and doesn't change its logic at all.
> > 
> > Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> > ---
> >  sheep/ops.c        |   10 +-
> >  sheep/sheep_priv.h |    4 +-
> >  sheep/vdi.c        |  356 ++++++++++++++++++++++++++++++++++++++--------------
> >  3 files changed, 277 insertions(+), 93 deletions(-)
> > 
> > diff --git a/sheep/ops.c b/sheep/ops.c
> > index 4c0f975..76ff611 100644
> > --- a/sheep/ops.c
> > +++ b/sheep/ops.c
> > @@ -67,6 +67,10 @@ static int cluster_new_vdi(struct request *req)
> >  	struct sd_rsp *rsp = &req->rp;
> >  	uint32_t vid;
> >  	int ret;
> > +	struct timeval tv;
> > +
> > +	gettimeofday(&tv, NULL);
> > +
> >  	struct vdi_iocb iocb = {
> >  		.name = req->data,
> >  		.data_len = hdr->data_length,
> > @@ -75,12 +79,16 @@ static int cluster_new_vdi(struct request *req)
> >  		.create_snapshot = !!hdr->vdi.snapid,
> >  		.nr_copies = hdr->vdi.copies ? hdr->vdi.copies :
> >  				sys->cinfo.nr_copies,
> > +		.time = (uint64_t) tv.tv_sec << 32 | tv.tv_usec * 1000,
> >  	};
> >  
> >  	if (hdr->data_length != SD_MAX_VDI_LEN)
> >  		return SD_RES_INVALID_PARMS;
> >  
> > -	ret = vdi_create(&iocb, &vid);
> > +	if (iocb.create_snapshot)
> > +		ret = sd_snapshot_create(&iocb, &vid);
> > +	else
> > +		ret = sd_vdi_create(&iocb, &vid);
> >  
> >  	rsp->vdi.vdi_id = vid;
> >  	rsp->vdi.copies = iocb.nr_copies;
> > diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h
> > index 35c067e..b78afc8 100644
> > --- a/sheep/sheep_priv.h
> > +++ b/sheep/sheep_priv.h
> > @@ -171,6 +171,7 @@ struct vdi_iocb {
> >  	uint32_t snapid;
> >  	bool create_snapshot;
> >  	int nr_copies;
> > +	uint64_t time;
> >  };
> >  
> >  struct vdi_info {
> > @@ -276,7 +277,8 @@ int get_max_copy_number(void);
> >  int get_req_copy_number(struct request *req);
> >  int add_vdi_state(uint32_t vid, int nr_copies, bool snapshot);
> >  int vdi_exist(uint32_t vid);
> > -int vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid);
> > +int sd_vdi_create(struct vdi_iocb *iocb, uint32_t *new_vid);
> > +int sd_snapshot_create(struct vdi_iocb *iocb, uint32_t *new_vid);
> 
> sd_{vdi,snapshot}_create share almost the same logic, how about merge them
> into one function? Either ask sd_{vdi,snapshot}_create call the shared helper
> or use single sd_vdi_create() function

Since we have 'vdi_' as the namespace for functions in sheep/vdi.c. So it better
to call it use vdi_ prefix instead of sd_.

Thanks
Yuan



More information about the sheepdog mailing list