[sheepdog] [PATCH v3 1/5] change accessing of inode->data_vdi_id[] to function

[MORITA Kazutaka]

At Thu, 24 Oct 2013 17:46:18 +0800,
Robin Dong wrote:
> 
> We need to modify all the direct accessing of data_vdi_id[] to common function
> because the following change of index in inode.
> 
> Signed-off-by: Robin Dong <sanbai at taobao.com>
> ---
>  dog/cluster.c            |    7 ++--
>  dog/common.c             |    7 +---
>  dog/vdi.c                |   78 +++++++++++++++++----------------------------
>  include/sheepdog_proto.h |    6 +++-
>  lib/Makefile.am          |    2 +-
>  lib/sd_inode.c           |   13 ++++++++
>  sheep/vdi.c              |    7 ++--
>  sheepfs/volume.c         |   15 +++-----
>  8 files changed, 65 insertions(+), 70 deletions(-)
>  create mode 100644 lib/sd_inode.c
> 
> diff --git a/dog/cluster.c b/dog/cluster.c
> index c2f97ad..62b78d0 100644
> --- a/dog/cluster.c
> +++ b/dog/cluster.c
> @@ -246,6 +246,7 @@ static void fill_object_tree(uint32_t vid, const char *name, const char *tag,
>  			     const struct sd_inode *i, void *data)
>  {
>  	uint64_t vdi_oid = vid_to_vdi_oid(vid), vmstate_oid;
> +	uint32_t vdi_id;
>  	int nr_objs, nr_vmstate_object;
>  
>  	/* ignore active vdi */
> @@ -258,9 +259,9 @@ static void fill_object_tree(uint32_t vid, const char *name, const char *tag,
>  	/* fill data object id */
>  	nr_objs = count_data_objs(i);
>  	for (uint64_t idx = 0; idx < nr_objs; idx++) {
> -		if (i->data_vdi_id[idx]) {
> -			uint64_t oid = vid_to_data_oid(i->data_vdi_id[idx],
> -						       idx);
> +		vdi_id = sd_inode_get_vdi(i, idx);
> +		if (vdi_id) {
> +			uint64_t oid = vid_to_data_oid(vdi_id, idx);
>  			object_tree_insert(oid, i->nr_copies, i->copy_policy);
>  		}
>  	}
> diff --git a/dog/common.c b/dog/common.c
> index 5e7ce2e..5e4ed21 100644
> --- a/dog/common.c
> +++ b/dog/common.c
> @@ -136,7 +136,7 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
>  	struct sd_req req;
>  	struct sd_rsp *rsp = (struct sd_rsp *)&req;
>  	static DECLARE_BITMAP(vdi_inuse, SD_NR_VDIS);
> -	unsigned int rlen = sizeof(vdi_inuse);
> +	unsigned int rlen;
>  
>  	sd_init_req(&req, SD_OP_READ_VDIS);
>  	req.data_length = sizeof(vdi_inuse);
> @@ -165,10 +165,7 @@ int parse_vdi(vdi_parser_func_t func, size_t size, void *data)
>  			continue;
>  
>  		if (size > SD_INODE_HEADER_SIZE) {
> -			rlen = count_data_objs(&i) * sizeof(i.data_vdi_id[0]);
> -			if (rlen > size - SD_INODE_HEADER_SIZE)
> -				rlen = size - SD_INODE_HEADER_SIZE;
> -
> +			rlen = size - SD_INODE_HEADER_SIZE;

Do we need this change?  The original code means

 rlen = min(count_data_objs(&i) * sizeof(i.data_vdi_id[0]),
            size - SD_INODE_HEADER_SIZE);

and this reduced the number of read size.  This improved performance
of vdi listing when there are many small vdis.

>  			ret = sd_read_object(oid, ((char *)&i) + SD_INODE_HEADER_SIZE,
>  					     rlen, SD_INODE_HEADER_SIZE, true);
>  


> diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h
> index 7138608..d393460 100644
> --- a/include/sheepdog_proto.h
> +++ b/include/sheepdog_proto.h
> @@ -235,6 +235,10 @@ struct sheepdog_vdi_attr {
>  	char value[SD_MAX_VDI_ATTR_VALUE_LEN];
>  };
>  
> +uint32_t sd_inode_get_vdi(const struct sd_inode *inode, int idx);

What we get is the vdi id.  I think sd_inode_get_vdi_id() or
sd_inode_get_vid() is better.

Thanks,

Kazutaka