On Tue, Aug 21, 2012 at 2:26 PM, Yunkai Zhang <yunkai.me at gmail.com> wrote: > On Tue, Aug 21, 2012 at 1:48 PM, Yunkai Zhang <yunkai.me at gmail.com> wrote: >> On Tue, Aug 21, 2012 at 1:42 PM, MORITA Kazutaka >> <morita.kazutaka at lab.ntt.co.jp> wrote: >>> At Thu, 16 Aug 2012 22:38:21 +0800, >>> Yunkai Zhang wrote: >>>> >>>> After add '-F' flag, the help looks like: >>>> $ collie vdi check >>>> Usage: collie vdi check [-F] [-s snapshot] [-a address] [-p port] [-h] <vdiname> >>>> Options: >>>> -F, --force_repair force repair object's copies (dangerous) >>> >>> How about '-r, --repair'? >> >> >> Good for me. > > When I submit this patch, my new patch "collie: add self options to > collie's command" haven't been developed, so '-r, --repair' options > would be conflict with '-r, --raw' option. I found that '-r, --raw' option is also need by 'collie vdi list' command, so there exist name conflict. I would use '-R, --repair'. > > Since these two patches are dependent, I'll put them in a series in V2. > >> >> >>> >>>> fprintf(stderr, "Failed to read, %s\n", >>>> sd_strerror(rsp->result)); >>>> exit(EXIT_FAILURE); >>>> } >>>> - return buf; >>>> + >>>> + memcpy(sha1, (unsigned char *)&rsp->__pad[0], SHA1_LEN); >>> >>> Please define a member name instead of using __pad. >> >> OK. >> >>> >>> >>>> } >>>> >>>> -static void write_object_to(struct sd_vnode *vnode, uint64_t oid, void *buf) >>>> +static int do_repair(uint64_t oid, struct node_id *src, struct node_id *dest) >>>> { >>>> struct sd_req hdr; >>>> struct sd_rsp *rsp = (struct sd_rsp *)&hdr; >>>> + unsigned rlen, wlen; >>>> + char host[128]; >>>> int fd, ret; >>>> - unsigned wlen = SD_DATA_OBJ_SIZE, rlen = 0; >>>> - char name[128]; >>>> >>>> - addr_to_str(name, sizeof(name), vnode->nid.addr, 0); >>>> - fd = connect_to(name, vnode->nid.port); >>>> + addr_to_str(host, sizeof(host), dest->addr, 0); >>>> + >>>> + fd = connect_to(host, dest->port); >>>> if (fd < 0) { >>>> - fprintf(stderr, "failed to connect to %s:%"PRIu32"\n", >>>> - name, vnode->nid.port); >>>> - exit(EXIT_FAILURE); >>>> + fprintf(stderr, "Failed to connect\n"); >>>> + return SD_RES_EIO; >>>> } >>>> >>>> - sd_init_req(&hdr, SD_OP_WRITE_PEER); >>>> - hdr.epoch = sd_epoch; >>>> - hdr.flags = SD_FLAG_CMD_WRITE; >>>> - hdr.data_length = wlen; >>>> + sd_init_req(&hdr, SD_OP_REPAIR_OBJ_PEER); >>> >>> I don't think sending peer requests directly from outside sheeps is a >>> good idea. How about making the gateway node forward the requests? >> >> Ok, no problem. >> >> >>> >>>> >>>> + rlen = 0; >>>> + wlen = sizeof(*src); >>>> + >>>> + hdr.epoch = sd_epoch; >>>> hdr.obj.oid = oid; >>>> + hdr.data_length = wlen; >>>> + hdr.flags = SD_FLAG_CMD_WRITE; >>>> >>>> - ret = exec_req(fd, &hdr, buf, &wlen, &rlen); >>>> + ret = exec_req(fd, &hdr, src, &wlen, &rlen); >>>> close(fd); >>>> - >>>> if (ret) { >>>> - fprintf(stderr, "Failed to execute request\n"); >>>> - exit(EXIT_FAILURE); >>>> + fprintf(stderr, "Failed to repair oid:%"PRIx64"\n", oid); >>>> + return SD_RES_EIO; >>>> } >>>> - >>>> if (rsp->result != SD_RES_SUCCESS) { >>>> - fprintf(stderr, "Failed to read, %s\n", >>>> - sd_strerror(rsp->result)); >>>> - exit(EXIT_FAILURE); >>>> + fprintf(stderr, "Failed to repair oid:%"PRIx64", %s\n", >>>> + oid, sd_strerror(rsp->result)); >>>> + return rsp->result; >>>> } >>>> + >>>> + return SD_RES_SUCCESS; >>>> } >>>> >>>> -/* >>>> - * Fix consistency of the replica of oid. >>>> - * >>>> - * XXX: The fix is rather dumb, just read the first copy and write it >>>> - * to other replica. >>>> - */ >>>> -static void do_check_repair(uint64_t oid, int nr_copies) >>>> +static int do_check_repair(uint64_t oid, int nr_copies) >>>> { >>>> struct sd_vnode *tgt_vnodes[nr_copies]; >>>> - void *buf, *buf_cmp; >>>> - int i; >>>> + unsigned char sha1[SD_MAX_COPIES][SHA1_LEN]; >>>> + char host[128]; >>>> + int i, j; >>>> >>>> oid_to_vnodes(sd_vnodes, sd_vnodes_nr, oid, nr_copies, tgt_vnodes); >>>> - buf = read_object_from(tgt_vnodes[0], oid); >>>> - for (i = 1; i < nr_copies; i++) { >>>> - buf_cmp = read_object_from(tgt_vnodes[i], oid); >>>> - if (memcmp(buf, buf_cmp, SD_DATA_OBJ_SIZE)) { >>>> - free(buf_cmp); >>>> - goto fix_consistency; >>>> + for (i = 0; i < nr_copies; i++) { >>>> + get_obj_checksum_from(tgt_vnodes[i], oid, sha1[i]); >>>> + } >>>> + >>>> + for (i = 0; i < nr_copies; i++) { >>>> + for (j = (i + 1); j < nr_copies; j++) { >>>> + if (memcmp(sha1[i], sha1[j], SHA1_LEN)) >>>> + goto diff; >>>> } >>>> - free(buf_cmp); >>>> } >>>> - free(buf); >>>> - return; >>>> + return 0; >>>> >>>> -fix_consistency: >>>> - for (i = 1; i < nr_copies; i++) >>>> - write_object_to(tgt_vnodes[i], oid, buf); >>>> - fprintf(stdout, "fix %"PRIx64" success\n", oid); >>>> - free(buf); >>>> +diff: >>>> + fprintf(stderr, "Failed oid: %"PRIx64"\n", oid); >>>> + for (i = 0; i < nr_copies; i++) { >>>> + addr_to_str(host, sizeof(host), tgt_vnodes[i]->nid.addr, 0); >>>> + fprintf(stderr, ">> copy[%d], sha1: %s, from: %s:%d\n", >>>> + i, sha1_to_hex(sha1[i]), host, tgt_vnodes[i]->nid.port); >>>> + } >>>> + >>>> + if (!vdi_cmd_data.force_repair) >>>> + return -1; >>>> + >>>> + /* >>>> + * Force repair the consistency of oid's replica >>>> + * >>>> + * FIXME: this fix is rather dumb, it just read the >>>> + * first copy and write it to other replica, >>>> + */ >>>> + fprintf(stderr, ">> force repairing ...\n"); >>>> + addr_to_str(host, sizeof(host), tgt_vnodes[0]->nid.addr, >>>> + tgt_vnodes[0]->nid.port); >>>> + for (i = 1; i< nr_copies; i++) { >>> >>> Needs a space before '<'. >>> >>>> @@ -1466,18 +1474,22 @@ static int vdi_check(int argc, char **argv) >>>> goto out; >>>> } >>>> >>>> + printf("CHECKING VDI:%s ...\n", vdiname); >>>> ret = check_repair_vdi(vid); >>>> - if (ret != EXIT_SUCCESS) >>>> + if (ret != EXIT_SUCCESS) { >>>> + printf("%s\n", >>>> + vdi_cmd_data.force_repair?"FORCE REPAIRED":"FAILED"); >>> >>> Need spaces around '?' and ':'. >> >> Ok, I forgot to check it. >> >> I will give V2 later. >> >>> >>> Thanks, >>> >>> Kazutaka >> >> >> >> -- >> Yunkai Zhang >> Work at Taobao > > > > -- > Yunkai Zhang > Work at Taobao -- Yunkai Zhang Work at Taobao |