At Thu, 17 Nov 2011 18:56:38 +0800,
Liu Yuan wrote:
>
> On 11/17/2011 06:52 PM, Liu Yuan wrote:
>
> > On 11/17/2011 06:27 PM, MORITA Kazutaka wrote:
> >
> >> I think this is still not generic... If this is specific one to
> >> simple_store, move it to simple_store.c.
> >>
> >>
> >
> >
> > I use siocb for interface functions internal information passing *and*
> > for higher level code to specify flags, length, buf, offset to store.rw
> > too.
> >
> > This is just a dirty hack, enabling me to make small changes to the
> > previous code to work with simple store correctly.
> >
> > My intention is that siocb is not specific to simple store. we need a
> > channel to send information to store.open(), then it can use it
> > communicate with store.read/write/close.
> >
> > So I think siocb would be globally seen by higher level code.
> >
> > if we don't use siocb, then what do you suggest to use for higher level
> > information (for e.g, flags, length, buf, offset) to store's interfaces?
> >
> > We can't simple pass a void *ptr, without knowing the internal structure
> > for it.
If you pass &ptr in store.open() and get a store driver specific
opaque, you don't need to know the internal structure.
> >
> > and even if later, siocb get changed, we don't need to change current
> > (oid, void *priv) interface.
> >
> > This is my own thought, maybe there is better idea.
> >
> >
> >> Remove the 'static' modifier in store.c if you want to use obj_path in
> >> this file.
> >>
> >
> >
> > Umm, obj_path is not static in my repo...I guess some patch remove it
> > already. I'll rebase it.
> >
> >
> >>> + ret = store_write_last_sector(oid, &fd);
> >>
> >> store_write_last_sector() expects struct siocb in the second argument.
> >>
> >
> >
> > Good catch, it is my carelessness.
> >
> >>
> >> I think we should define opaque as 'void *', get a store_driver
> >> specific opaque in the argument of store.open(), and pass it to
> >> store.read()/write()/close(). This is also what Christoph suggested.
> >> Am i wrong?
> >>
> >
> >
> > Like above, so it is totally opaque, where can we pass flags, length,
> > offset to store driver?
> >
>
>
> And more, length, offset is critical for later partial data object
> read/write, if we support huge data size. So we can't assume an
> interface just need (key, value).
What I thought was:
struct store_driver {
const char *driver_name;
int (*init)(char *path);
int (*open)(uint32_t epoch, uint64_t oid, int create, void **priv);
int (*write)(uint64_t oid, void *buf, int len, off_t offset, void *priv);
int (*read)(uint64_t oid, void *buf, int len, off_t offset, void *priv);
int (*close)(void *priv);
};
On error, open/write/read returns an error code. open() also returns
the driver specific data in the last argument. read()/write() checks
whether data is fully read/written inside the method.
If you want to use siocb (I'm not sure this is the right way):
struct siocb {
uint32_t epoch;
void *buf;
uint32_t length;
uint64_t offset;
int create
};
struct store_driver {
const char *driver_name;
int (*init)(char *path);
int (*open)(uint64_t oid, struct siocb *cb, void **priv);
int (*write)(uint64_t oid, struct siocb *cb, void *priv);
int (*read)(uint64_t oid, struct siocb *cb, void *priv);
int (*close)(void *priv);
};
'fd' shouldn't be included in siocb because it is simple_store specific.
Thanks,
Kazutaka
|
|