[sheepdog] [PATCH 1/3] md: make vdisk management dynamic

[MORITA Kazutaka]

At Tue,  3 Sep 2013 11:06:53 +0800,
Liu Yuan wrote:
> 
> diff --git a/sheep/md.c b/sheep/md.c
> index d51a7c6..f94fd29 100644
> --- a/sheep/md.c
> +++ b/sheep/md.c
> @@ -13,98 +13,116 @@
>  
>  #include "sheep_priv.h"
>  
> -#define MD_DEFAULT_VDISKS 128
> -#define MD_MAX_VDISK (MD_MAX_DISK * MD_DEFAULT_VDISKS)
> +#define MD_VDISK_SIZE ((uint64_t)1*1024*1024*1024) /* 1G */
> +
> +#define NONE_EXIST_PATH "/all/disks/are/broken/,ps/əʌo7/!"

The path may exist, no?  How about using "" as a non-existent path?

> +static inline uint64_t vnode_hash(struct rb_node *node)
> +{
> +	struct vdisk *v;
>  
> -		nr++;
> -	}
> +	v = rb_entry(node, struct vdisk, rb);
> +	return v->hash;
> +}
>  
> -	return nr;
> +static inline struct vdisk *vnode_vdisk(struct rb_node *node)
> +{
> +	return rb_entry(node, struct vdisk, rb);
>  }
>  
> -static inline int disks_to_vdisks(const struct disk *ds, int nmds, struct vdisk *vds)
> +static struct vdisk *find_right_close(struct rb_node *node, uint64_t hash)
>  {
> -	int nr_vdisks = 0;
> +	struct vdisk *v;
>  
> -	for (int i = 0; i < nmds; i++)
> -		nr_vdisks += disk_to_vdisks(ds, i, vds + nr_vdisks);
> +	if (hash < vnode_hash(node)) {
> +		if (!node->rb_left)
> +			return vnode_vdisk(node);
>  
> -	xqsort(vds, nr_vdisks, vdisk_cmp);
> +		v = find_right_close(node->rb_left, hash);
> +		return v->hash - hash > vnode_hash(node) - hash ?
> +			vnode_vdisk(node) : v;
> +	} else if (hash > vnode_hash(node)) {
> +		if (!node->rb_right)
> +			return vnode_vdisk(rb_first(&md.vroot));
> +		return find_right_close(node->rb_right, hash);
> +	} else {
> +		return vnode_vdisk(node);
> +	}
> +}
>  
> -	return nr_vdisks;
> +/* If v1_hash < oid <= v2_hash, then oid is resident in v2 */
> +static struct vdisk *oid_to_vdisk(uint64_t oid)
> +{
> +	uint64_t hash = sd_hash_oid(oid);
> +
> +	return find_right_close(md.vroot.rb_node, hash);
>  }

I think we will implement a similar function when we change node
management from a fixed array to rbtree.  How about adding a helper
function in rbtree.h like as follows?  Then, the above code will be
much cleaner.

/*
 * Search for a value in the rbtree.  When the key is not found in the rbtree,
 * this returns the next greater node if one exists.
 */
#define rb_nsearch(root, key, member, compar)				\
({									\
	struct rb_node *__n = (root)->rb_node;				\
	typeof(key) __ret = NULL, __data;				\
									\
	while (__n) {							\
		__data = rb_entry(__n, typeof(*key), member);		\
		int __cmp = compar(key, __data);			\
									\
		if (__cmp < 0) {					\
			__ret = __data;					\
			__n = __n->rb_left;				\
		} else if (__cmp > 0)					\
			__n = __n->rb_right;				\
		else {							\
			__ret = __data;					\
			break;						\
		}							\
	}								\
	__ret;								\
})


>  
> -static inline struct vdisk *oid_to_vdisk(uint64_t oid)
> +static void create_vdisks(struct disk *disk)
>  {
> -	return oid_to_vdisk_from(md_vds, md_nr_vds, oid);
> +	uint64_t hval = sd_hash(disk->path, strlen(disk->path));
> +
> +	for (int i = 0; i < vdisk_number(disk); i++) {

GCC can know that vdisk_number(disk) returns a constant number while
looping?  I think it's better to save the number of vdisks to a
temporary variable:

	nr = vdisk_number(disk);
	for (int i = 0; i < nr; i++) {

Thanks,

Kazutaka