On 05/30/2013 08:23 PM, Kai Zhang wrote: > Signed-off-by: Kai Zhang <kyle at zelin.io> > --- > collie/farm/farm.c | 184 ++++++++++++++++++++++++++++++++------------ > collie/farm/farm.h | 12 +-- > collie/farm/object_tree.c | 15 +--- > collie/farm/trunk.c | 15 +--- > 4 files changed, 146 insertions(+), 80 deletions(-) > > diff --git a/collie/farm/farm.c b/collie/farm/farm.c > index cb4aaa9..bb7b36c 100644 > --- a/collie/farm/farm.c > +++ b/collie/farm/farm.c > @@ -29,6 +29,16 @@ struct vdi_entry { > }; > static LIST_HEAD(last_vdi_list); > > +struct snapshot_work { > + uint64_t oid; > + int nr_copies; > + unsigned char sha1[SHA1_LEN]; > + struct strbuf *trunk_entries; > + struct work work; > +}; > +struct work_queue *wq; > +static uatomic_bool work_error; > + > static struct vdi_entry *find_vdi(const char *name) > { > struct vdi_entry *vdi; > @@ -186,39 +196,6 @@ static int notify_vdi_add(uint32_t vdi_id, uint32_t nr_copies) > return ret; > } > > -static int fill_trunk_entry(uint64_t oid, int nr_copies, > - void *buf, size_t size, void *data) > -{ > - int ret = -1; > - > - struct strbuf *trunk_entries = data; > - struct trunk_entry new_entry = {}; > - struct sha1_file_hdr hdr = { .priv = 0 }; > - struct strbuf object_strbuf = STRBUF_INIT; > - > - memcpy(hdr.tag, TAG_DATA, TAG_LEN); > - hdr.size = size; > - > - strbuf_add(&object_strbuf, buf, size); > - strbuf_insert(&object_strbuf, 0, &hdr, sizeof(hdr)); > - > - if (sha1_file_write((void *)object_strbuf.buf, > - object_strbuf.len, > - new_entry.sha1) != 0) > - goto out; > - > - new_entry.oid = oid; > - new_entry.nr_copies = nr_copies; > - strbuf_add(trunk_entries, &new_entry, sizeof(struct trunk_entry)); > - > - ret = 0; > -out: > - if (ret) > - fprintf(stderr, "Fail to fill trunk entry\n."); > - strbuf_release(&object_strbuf); > - return ret; > -} > - > int farm_init(const char *path) > { > int ret = -1; > @@ -240,14 +217,78 @@ bool farm_contain_snapshot(uint32_t idx, const char *tag) > return (get_trunk_sha1(idx, tag, trunk_sha1) == 0); > } > > +static void save_object_main(struct work *work) > +{ > + void *sha1_buf, *data_buf; > + size_t sha1_size, data_size; > + struct snapshot_work *sw; > + struct sha1_file_hdr *hdr; > + > + if (uatomic_is_true(&work_error)) > + return; > + > + sw = container_of(work, struct snapshot_work, work); > + data_size = get_objsize(sw->oid); > + sha1_size = data_size + sizeof(struct sha1_file_hdr); > + hdr = sha1_buf = xmalloc(sha1_size); > + data_buf = (char *)sha1_buf + sizeof(struct sha1_file_hdr); > + > + if (sd_read_object(sw->oid, data_buf, data_size, 0, true) < 0) > + goto error; > + > + memcpy(hdr->tag, TAG_DATA, TAG_LEN); > + hdr->size = data_size; > + > + if (sha1_file_write(sha1_buf, sha1_size, sw->sha1) < 0) > + goto error; > + > + goto out; > +error: > + fprintf(stderr, "Fail to save object, oid %"PRIu64"\n", > + sw->oid); > + uatomic_set_true(&work_error); > +out: > + free(sha1_buf); > +} It is more clear to put error handling after the normal return as: out: clean_up_something(); return success; err: clean_up_something(); return false; Even though sometimes we duplicate some statements in the clean_up_something(). GCC might do smarter optimization to remove the duplicate statements sometimes, if not, we just trade several bytes to get a better readability. > + > +static void save_object_done(struct work *work) > +{ > + struct trunk_entry entry; > + struct snapshot_work *sw = container_of(work, struct snapshot_work, > + work); > + > + if (uatomic_is_true(&work_error)) > + goto out; > + > + entry.oid = sw->oid; > + entry.nr_copies = sw->nr_copies; > + memcpy(entry.sha1, sw->sha1, SHA1_LEN); > + strbuf_add(sw->trunk_entries, &entry, sizeof(struct trunk_entry)); > +out: > + free(sw); > +} > + > +static int queue_save_snapshot_work(uint64_t oid, int nr_copies, void *data) > +{ > + struct snapshot_work *sw = xzalloc(sizeof(struct snapshot_work)); > + struct strbuf *trunk_entries = data; > + sw->oid = oid; > + sw->nr_copies = nr_copies; > + sw->trunk_entries = trunk_entries; > + sw->work.fn = save_object_main; We sometimes use 'main' to mean it is running in the main thread. So we'd better have save_object_main as do_save_object. So load_object_main -> do_load_object. By the way, maybe we can read sha1 of object first to save duplicate transfer of the objects that maps to the same sha1. Thanks, Yuan |