At Thu, 17 Nov 2011 22:26:47 +0800, Liu Yuan wrote: > > From: Liu Yuan <tailai.ly at taobao.com> > > Let jrnl_perform just concentrate on journeling stuff, not intrude in store IO. > This would make store IO interface abstracting easier and cleaner. > > Signed-off-by: Liu Yuan <tailai.ly at taobao.com> > --- > sheep/journal.c | 70 ++++++++++++++++------------------------ > sheep/sheep_priv.h | 5 ++- > sheep/store.c | 89 ++++++++++++++++++++++++++++++++++++++-------------- > 3 files changed, 96 insertions(+), 68 deletions(-) > > diff --git a/sheep/journal.c b/sheep/journal.c > index beff25c..790ebb6 100644 > --- a/sheep/journal.c > +++ b/sheep/journal.c > @@ -189,68 +189,54 @@ static int jrnl_apply_to_target_object(struct jrnl_descriptor *jd) > return res; > } > > -static int jrnl_commit_data(struct jrnl_descriptor *jd) > -{ > - int ret = 0; > - ssize_t retsize; > - struct jrnl_head *head = (struct jrnl_head *) &jd->head; > - > - retsize = pwrite64(jd->target_fd, jd->data, head->size, head->offset); > - if (retsize != head->size) { > - if (errno == ENOSPC) > - ret = SD_RES_NO_SPACE; > - else > - ret = SD_RES_EIO; > - } > - > - return ret; > -} > - > /* > * We cannot use this function for concurrent write operations > */ > -int jrnl_perform(int fd, void *buf, size_t count, off_t offset, > +struct jrnl_descriptor *jrnl_begin(void *buf, size_t count, off_t offset, > const char *path, const char *jrnl_dir) > { > int ret; > - struct jrnl_descriptor jd; > - > - memset(&jd, 0, sizeof(jd)); > - jd.target_fd = fd; > + struct jrnl_descriptor *jd = xzalloc(sizeof(*jd)); > > - jd.head.offset = offset; > - jd.head.size = count; > - strcpy(jd.head.target_path, path); > + jd->head.offset = offset; > + jd->head.size = count; > + strcpy(jd->head.target_path, path); > > - jd.data = buf; > + jd->data = buf; > > - ret = jrnl_create(&jd, jrnl_dir); > + ret = jrnl_create(jd, jrnl_dir); > if (ret) > - goto out; > + goto err; > > - ret = jrnl_write_header(&jd); > + ret = jrnl_write_header(jd); > if (ret) > - goto out; > + goto err; > > - ret = jrnl_write_data(&jd); > + ret = jrnl_write_data(jd); > if (ret) > - goto out; > + goto err; > > - ret = jrnl_write_end_mark(&jd); > + ret = jrnl_write_end_mark(jd); > if (ret) > - goto out; > + goto err; > + return jd; > +err: > + free(jd); > + return NULL; > +} > > - ret = jrnl_commit_data(&jd); > - if (ret) > - goto out; > +int jrnl_end(struct jrnl_descriptor * jd) > +{ > + int ret = 0; > + if (!jd) > + return ret; > > - ret = jrnl_close(&jd); > + ret = jrnl_close(jd); > if (ret) > - goto out; > - > - ret = jrnl_remove(&jd); > + goto err; > > -out: > + ret = jrnl_remove(jd); > +err: > return ret; > } > > diff --git a/sheep/sheep_priv.h b/sheep/sheep_priv.h > index 56bcbd1..cbacab3 100644 > --- a/sheep/sheep_priv.h > +++ b/sheep/sheep_priv.h > @@ -260,8 +260,9 @@ int do_process_main(struct sd_op_template *op, const struct sd_req *req, > struct sd_rsp *rsp, void *data); > > /* Journal */ > -int jrnl_perform(int fd, void *buf, size_t count, off_t offset, > - const char *path, const char *jrnl_dir); > +struct jrnl_descriptor *jrnl_begin(void *buf, size_t count, off_t offset, > + const char *path, const char *jrnl_dir); > +int jrnl_end(struct jrnl_descriptor * jd); > int jrnl_recover(const char *jrnl_dir); > > static inline int is_myself(uint8_t *addr, uint16_t port) > diff --git a/sheep/store.c b/sheep/store.c > index c142bc3..f022d55 100644 > --- a/sheep/store.c > +++ b/sheep/store.c > @@ -624,6 +624,7 @@ static int store_write_obj_fd(int fd, struct request *req, uint32_t epoch) > struct sd_obj_req *hdr = (struct sd_obj_req *)&req->rq; > uint64_t oid = hdr->oid; > int ret = SD_RES_SUCCESS; > + void *jd = NULL; > > if (hdr->flags & SD_FLAG_CMD_TRUNCATE) { > ret = ftruncate(fd, hdr->offset + hdr->data_length); > @@ -638,10 +639,19 @@ static int store_write_obj_fd(int fd, struct request *req, uint32_t epoch) > > snprintf(path, sizeof(path), "%s%08u/%016" PRIx64, obj_path, > epoch, oid); > - ret = jrnl_perform(fd, req->data, hdr->data_length, > + jd = jrnl_begin(req->data, hdr->data_length, > hdr->offset, path, jrnl_path); > - if (ret) > - return ret; > + if (!jd) > + return SD_RES_EIO; > + ret = pwrite64(fd, req->data, hdr->data_length, hdr->offset); > + if (ret != hdr->data_length) { > + if (errno == ENOSPC) > + return SD_RES_NO_SPACE; > + eprintf("%m\n"); > + jrnl_end(jd); > + return SD_RES_EIO; > + } > + jrnl_end(jd); > } else { > ret = pwrite64(fd, req->data, hdr->data_length, hdr->offset); > if (ret != hdr->data_length) { > @@ -651,7 +661,6 @@ static int store_write_obj_fd(int fd, struct request *req, uint32_t epoch) > return SD_RES_EIO; > } > } > - > return SD_RES_SUCCESS; > } > > @@ -1090,20 +1099,30 @@ int remove_epoch(int epoch) > int set_cluster_ctime(uint64_t ct) > { > int fd, ret; > + void *jd; > > fd = open(config_path, O_DSYNC | O_WRONLY); > if (fd < 0) > - return -1; > + return SD_RES_EIO; > > - ret = jrnl_perform(fd, &ct, sizeof(ct), > - offsetof(struct sheepdog_config, ctime), > - config_path, jrnl_path); > + jd = jrnl_begin(&ct, sizeof(ct), > + offsetof(struct sheepdog_config, ctime), > + config_path, jrnl_path); > + if (!jd) { > + ret = SD_RES_EIO; > + goto err; > + } > + ret = pwrite64(fd, &ct, sizeof(ct), offsetof(struct sheepdog_config, ctime)); > + if (ret != sizeof(ct)) { > + ret = SD_RES_EIO; > + jrnl_end(jd); > + goto err; > + } > + ret = SD_RES_SUCCESS; > + jrnl_end(jd); > +err: The following code may be a bit cleaner. == if (ret != sizeof(ct)) ret = SD_RES_EIO; else ret = SD_RES_SUCCESS; jrnl_end(jd); == > close(fd); > - > - if (ret) > - return -1; > - > - return 0; > + return ret; > } > > uint64_t get_cluster_ctime(void) > @@ -2125,20 +2144,31 @@ int read_epoch(uint32_t *epoch, uint64_t *ct, > int set_cluster_copies(uint8_t copies) > { > int fd, ret; > + void *jd; > > fd = open(config_path, O_DSYNC | O_WRONLY); > if (fd < 0) > return SD_RES_EIO; > > - ret = jrnl_perform(fd, &copies, sizeof(copies), > - offsetof(struct sheepdog_config, copies), > - config_path, jrnl_path); > - close(fd); > - > - if (ret != 0) > - return SD_RES_EIO; > + jd = jrnl_begin(&copies, sizeof(copies), > + offsetof(struct sheepdog_config, copies), > + config_path, jrnl_path); > + if (!jd) { > + ret = SD_RES_EIO; > + goto err; > + } > > - return SD_RES_SUCCESS; > + ret = pwrite64(fd, &copies, sizeof(copies), offsetof(struct sheepdog_config, copies)); > + if (ret != sizeof(copies)) { > + ret = SD_RES_EIO; > + jrnl_end(jd); > + goto err; > + } > + jrnl_end(jd); > + ret = SD_RES_SUCCESS; > +err: Same here. > + close(fd); > + return ret; > } > > int get_cluster_copies(uint8_t *copies) > @@ -2162,17 +2192,28 @@ int get_cluster_copies(uint8_t *copies) > int set_cluster_flags(uint16_t flags) > { > int fd, ret = SD_RES_EIO; > + void *jd; > > fd = open(config_path, O_DSYNC | O_WRONLY); > if (fd < 0) > goto out; > > - ret = jrnl_perform(fd, &flags, sizeof(flags), > + jd = jrnl_begin(&flags, sizeof(flags), > offsetof(struct sheepdog_config, flags), > config_path, jrnl_path); > - if (ret != 0) > + if (!jd) { > ret = SD_RES_EIO; > - > + goto err; > + } > + ret = pwrite64(fd, &flags, sizeof(flags), offsetof(struct sheepdog_config, flags)); > + if (ret != sizeof(flags)) { > + ret = SD_RES_EIO; > + jrnl_end(jd); > + goto err; > + } > + jrnl_end(jd); > + ret = SD_RES_SUCCESS; > +err: And here. Thanks, Kazutaka |