[sheepdog] [PATCH] sheep: cache sha1 digest for read-only objects

[Liu Yuan]

On 05/14/2013 03:10 PM, MORITA Kazutaka wrote:
> The contents of read-only objects are immutable.  We can cache their
> sha1 digests as extended attributes for the next sha1 calculation.
> With this patch, I can complete 'collie vdi check' of a 4 GB snapshot
> volume in 0.2 seconds.
> 
> Signed-off-by: MORITA Kazutaka <morita.kazutaka at lab.ntt.co.jp>
> ---
> 
> This patch is against my "use hash for vdi check and object recovery" series.
> 
>  sheep/farm/farm.c   |  5 -----
>  sheep/plain_store.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/sheep/farm/farm.c b/sheep/farm/farm.c
> index c31c501..38586f5 100644
> --- a/sheep/farm/farm.c
> +++ b/sheep/farm/farm.c
> @@ -98,11 +98,6 @@ static int farm_init(void)
>  	if (create_directory(obj_path) < 0)
>  		goto err;
>  
> -	if (!is_xattr_enabled(obj_path)) {
> -		sd_eprintf("xattrs are not enabled on %s", obj_path);
> -		goto err;
> -	}
> -
>  	if (snap_init() < 0)
>  		goto err;
>  
> diff --git a/sheep/plain_store.c b/sheep/plain_store.c
> index 3b71467..5f0a131 100644
> --- a/sheep/plain_store.c
> +++ b/sheep/plain_store.c
> @@ -12,6 +12,7 @@
>  #include <fcntl.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
> +#include <sys/xattr.h>
>  #include <unistd.h>
>  #include <libgen.h>
>  
> @@ -220,6 +221,12 @@ int default_init(void)
>  	int ret;
>  
>  	sd_dprintf("use plain store driver");
> +
> +	if (!is_xattr_enabled(obj_path)) {
> +		sd_eprintf("xattrs are not enabled on %s", obj_path);
> +		return SD_RES_EIO;
> +	}
> +
>  	ret = for_each_obj_path(make_stale_dir);
>  	if (ret != SD_RES_SUCCESS)
>  		return ret;
> @@ -476,6 +483,41 @@ int default_remove_object(uint64_t oid)
>  	return SD_RES_SUCCESS;
>  }
>  
> +#define SHA1NAME "user.obj.sha1"
> +
> +static int get_sha1_cache(uint64_t oid, uint32_t epoch, uint8_t *sha1)
> +{
> +	char path[PATH_MAX];
> +
> +	if (!oid_is_readonly(oid))
> +		return -1;
> +
> +	if (default_exist(oid))
> +		get_obj_path(oid, path);
> +	else
> +		get_stale_obj_path(oid, epoch, path);
> +
> +	if (getxattr(path, SHA1NAME, sha1, SHA1_LEN) != SHA1_LEN)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static int set_sha1_cache(uint64_t oid, uint32_t epoch, const uint8_t *sha1)
> +{
> +	char path[PATH_MAX];
> +
> +	if (!oid_is_readonly(oid))
> +		return -1;
> +

Stale object isn't marked readonly yet. So your patch won't be useful
for recovery?

> +	if (default_exist(oid))
> +		get_obj_path(oid, path);
> +	else
> +		get_stale_obj_path(oid, epoch, path);
> +
> +	return setxattr(path, SHA1NAME, sha1, SHA1_LEN, 0);

setxattr can fail, need failure handling.

> +}
> +
>  int default_get_hash(uint64_t oid, uint32_t epoch, uint8_t *sha1)
>  {
>  	int ret;
> @@ -485,6 +527,11 @@ int default_get_hash(uint64_t oid, uint32_t epoch, uint8_t *sha1)
>  	uint64_t offset = 0;
>  	uint32_t length;
>  
> +	if (get_sha1_cache(oid, epoch, sha1) == 0) {
> +		sd_dprintf("use cached sha1 digest %s", sha1_to_hex(sha1));
> +		return SD_RES_SUCCESS;
> +	}
> +
>  	length = get_objsize(oid);
>  	buf = malloc(length);
>  	if (buf == NULL)
> @@ -512,6 +559,8 @@ int default_get_hash(uint64_t oid, uint32_t epoch, uint8_t *sha1)
>  	sd_dprintf("the message digest of %"PRIx64" at epoch %d is %s", oid,
>  		   epoch, sha1_to_hex(sha1));
>  
> +	set_sha1_cache(oid, epoch, sha1);

Better name as get/set_object_sha1() and put oid_is_readonly() check in
the default_get_hash() will make the code better readability.

Thanks,
Yuan