[Sheepdog] [PATCH v4 2/2] sheep: abstract out store IO interface

[MORITA Kazutaka]

At Thu, 17 Nov 2011 22:26:48 +0800,
Liu Yuan wrote:
> 
> From: Liu Yuan <tailai.ly at taobao.com>
> 
> We need to abstract out store IO interface to adopt it to other IO store
> such as the coming 'Farm' store.
> 
> The open/read/write/close is cumbersome for a common kv-store to work with,
> but this interface request smallest changes to current sheep store code. It
> sucks but works as a kludge.
> 
> Don't get me wrong that I am writing an universal interface that will work well
> with different kinds of data stores, say, sql-store, non-sql store, unstructured store,
> the store that is not with local backing stroage, etc.
> 
> Simply I am *not* and I am always lost to foresee the future.
> 
> This interface is stupid but simply enough that costs me smallest changes to existing code
> to let Sheepdog work with current store implementation and the coming 'Farm' store.
> 
> I think those kind people who try to squeeze other useful stores into Sheepdog are at a better
> position to cook a more generic interface in the future.
> 
> - Why include length, offset that many kv stores don't need at all?
> 
> Okay, we'er trying to implement huge data size, so we need these to do partial object read/write.
> 
> - Why 'int fd' instead of a void *opaque for store object handle?
> 
> I suppose file is everything in UNIX philosophy and so fd can name everything and I hate type
> conversion and frown when I can't cscope what it means for one second.
> 
> And last, I am happy to see anybody prove me wrong and replace it with a more capable interface.
> 
> Signed-off-by: Liu Yuan <tailai.ly at taobao.com>
> ---
>  sheep/Makefile.am    |    2 +-
>  sheep/sheep_priv.h   |   20 +++++
>  sheep/simple_store.c |  169 +++++++++++++++++++++++++++++++++++++++++
>  sheep/store.c        |  205 ++++++++++++++++++++------------------------------
>  4 files changed, 273 insertions(+), 123 deletions(-)
>  create mode 100644 sheep/simple_store.c
> 
> diff --git a/sheep/Makefile.am b/sheep/Makefile.am
> index 55c35c7..d0d3be6 100644
> --- a/sheep/Makefile.am
> +++ b/sheep/Makefile.am
> @@ -24,7 +24,7 @@ INCLUDES		= -I$(top_builddir)/include -I$(top_srcdir)/include $(libcpg_CFLAGS) $
>  sbin_PROGRAMS		= sheep
>  
>  sheep_SOURCES		= sheep.c group.c sdnet.c store.c vdi.c work.c journal.c ops.c \
> -			  cluster/local.c strbuf.c
> +			  cluster/local.c strbuf.c simple_store.c
>  if BUILD_COROSYNC
>  sheep_SOURCES		+= cluster/corosync.c
>  endif
> diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h
> index cbacab3..914f7b5 100644
> --- a/sheep/sheep_priv.h
> +++ b/sheep/sheep_priv.h
> @@ -156,6 +156,26 @@ struct cluster_info {
>  	struct work_queue *recovery_wqueue;
>  };
>  
> +struct siocb {
> +	int fd;
> +	uint16_t flags;
> +	uint32_t epoch;
> +	void *buf;
> +	uint32_t length;
> +	uint64_t offset;
> +};
> +
> +struct store_driver {
> +	const char *driver_name;
> +	int (*init)(char *path);
> +	int (*open)(uint64_t oid, struct siocb *, int create);
> +	ssize_t (*write)(uint64_t oid, struct siocb *);
> +	ssize_t (*read)(uint64_t oid, struct siocb *);

How do you think about returning the result code instead of the number
of processed bytes, which was suggested by Christoph?

Other than this, looks okay to me. :)

Thanks,

Kazutaka