[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