[Sheepdog] [PATCH v5 01/17] sheep: hide some minor store layout aware operations

[MORITA Kazutaka]

At Fri, 30 Dec 2011 21:06:56 +0800,
Liu Yuan wrote:
> 
> From: Liu Yuan <tailai.ly at taobao.com>
> 
> We need this to decouple the sheep from kv-store.
> 
> Signed-off-by: Liu Yuan <tailai.ly at taobao.com>
> ---
>  sheep/store.c |   57 ++++++++++++++++++++++++++++++++++++++-------------------
>  1 files changed, 38 insertions(+), 19 deletions(-)
> 
> diff --git a/sheep/store.c b/sheep/store.c
> index 56e3cd2..3323fbc 100644
> --- a/sheep/store.c
> +++ b/sheep/store.c
> @@ -57,6 +57,14 @@ static int obj_cmp(const void *oid1, const void *oid2)
>  	return 0;
>  }
>  
> +static void get_store_dir(struct strbuf *buf, int epoch)
> +{
> +	if (!strcmp(store.name, "simple"))
> +		strbuf_addf(buf, "%s%08u", obj_path, epoch);
> +	else /* XXX assume other store doesn't need epoch/obj pattern */
> +		strbuf_addf(buf, "%s", obj_path);
> +}
> +

It is not good to make a conditional branch based on the drvier name
here.

How about adding stat and jrnl_begin/end/recover to the store driver
interface?  Then, we can remove get_store_dir().

>  int stat_sheep(uint64_t *store_size, uint64_t *store_free, uint32_t epoch)
>  {
>  	struct statvfs vs;
> @@ -65,22 +73,27 @@ int stat_sheep(uint64_t *store_size, uint64_t *store_free, uint32_t epoch)
>  	struct dirent *d;
>  	uint64_t used = 0;
>  	struct stat s;
> -	char path[1024], store_dir[1024];
> +	char path[1024];
> +	struct strbuf store_dir = STRBUF_INIT;
>  
>  	ret = statvfs(mnt_path, &vs);
> -	if (ret)
> -		return SD_RES_EIO;
> +	if (ret) {
> +		ret = SD_RES_EIO;
> +		goto out;
> +	}
>  
> -	snprintf(store_dir, sizeof(store_dir), "%s%08u", obj_path, epoch);
> -	dir = opendir(store_dir);
> -	if (!dir)
> -		return SD_RES_EIO;
> +	get_store_dir(&store_dir, epoch);
> +	dir = opendir(store_dir.buf);
> +	if (!dir) {
> +		ret = SD_RES_EIO;
> +		goto out;
> +	}
>  
>  	while ((d = readdir(dir))) {
>  		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
>  			continue;
>  
> -		snprintf(path, sizeof(path), "%s/%s", store_dir, d->d_name);
> +		snprintf(path, sizeof(path), "%s/%s", store_dir.buf, d->d_name);
>  
>  		ret = stat(path, &s);
>  		if (ret)
> @@ -90,11 +103,13 @@ int stat_sheep(uint64_t *store_size, uint64_t *store_free, uint32_t epoch)
>  	}
>  
>  	closedir(dir);
> +	ret = SD_RES_SUCCESS;
>  
>  	*store_size = (uint64_t)vs.f_frsize * vs.f_bfree + used;
>  	*store_free = (uint64_t)vs.f_frsize * vs.f_bfree;
> -
> -	return SD_RES_SUCCESS;
> +out:
> +	strbuf_release(&store_dir);
> +	return ret;
>  }
>  
>  static int merge_objlist(uint64_t *list1, int nr_list1,
> @@ -411,11 +426,12 @@ out:
>  
>  int update_epoch_store(uint32_t epoch)
>  {
> -	char new[1024];
> -
> -	snprintf(new, sizeof(new), "%s%08u/", obj_path, epoch);
> -	mkdir(new, def_dmode);
> +	if (!strcmp(store.name, "simple")) {
> +		char new[1024];
>  
> +		snprintf(new, sizeof(new), "%s%08u/", obj_path, epoch);
> +		mkdir(new, def_dmode);
> +	}
>  	return 0;
>  }

IIUC, the farm driver updates epoch in farm_begin_recover().  Why do
the two drivers update it in the different places?

Thanks,

Kazutaka