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 |