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. 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 |