At Tue, 27 Mar 2012 13:53:51 +0800, yaohaiting.wujue at gmail.com wrote: > > From: HaiTing Yao <wujue.yht at taobao.com> > > When create snapshot for source VDI, the new created VDI used as source > VDI, and the old source VDI used as snapshot. This flow make users > confused about VDI and snapshot relation. The snapshot metadata maybe is > stored on multi-VDI, so need read multi VDIs inode to get snapshot list. > > When create snapshot, we does not need change new created VDI to source > VDI. The source VDI just need use snapshot VDI ID as its object data ID. > > Show one example. > > Before modification: > > Name Id Size Used Shared Creation time VDI id Tag > s v1 1 64 MB 20 MB 0.0 MB 2012-03-26 16:55 709128 > s v1 2 64 MB 0.0 MB 20 MB 2012-03-26 16:56 709129 sn3 > v1 3 64 MB 0.0 MB 20 MB 2012-03-26 16:56 70912a > > After modification: > > Name Id Size Used Shared Creation time VDI id Tag > v1 0 64 MB 20 MB 0.0 MB 2012-03-27 11:06 709128 > s v1 1 64 MB 0.0 MB 20 MB 2012-03-27 11:06 709129 > s v1 2 64 MB 0.0 MB 20 MB 2012-03-27 11:07 70912a sn3 > > Signed-off-by: HaiTing Yao <wujue.yht at taobao.com> > --- > collie/common.c | 2 +- > collie/vdi.c | 30 +++++++++++++++++++----------- > include/sheepdog_proto.h | 6 +++--- > sheep/vdi.c | 18 ++++++++++-------- > 4 files changed, 33 insertions(+), 23 deletions(-) It seems that 'collie vdi tree' and 'collie vdi graph' don't work correctly with this patch. Other comments are below. > > diff --git a/collie/common.c b/collie/common.c > index f4301c4..636b821 100644 > --- a/collie/common.c > +++ b/collie/common.c > @@ -13,7 +13,7 @@ > > int is_current(struct sheepdog_inode *i) > { > - return !i->snap_ctime; > + return !i->snap_id; > } > > char *size_to_str(uint64_t _size, char *str, int str_size) > diff --git a/collie/vdi.c b/collie/vdi.c > index e0678a6..8a30c99 100644 > --- a/collie/vdi.c > +++ b/collie/vdi.c > @@ -90,7 +90,9 @@ static void print_vdi_list(uint32_t vid, char *name, char *tag, uint32_t snapid, > for (idx = 0; idx < MAX_DATA_OBJS; idx++) { > if (!i->data_vdi_id[idx]) > continue; > - if (is_data_obj_writeable(i, idx)) > + if (!i->parent_vdi_id) > + my_objs++; > + else if ((!i->snap_id) && (i->io_vdi_id == i->data_vdi_id[idx])) Use is_data_obj_writeable(). > my_objs++; > else > cow_objs++; > @@ -511,7 +513,7 @@ out: > static int vdi_snapshot(int argc, char **argv) > { > char *vdiname = argv[optind++]; > - uint32_t vid; > + uint32_t vid, own_vid; > int ret; > char buf[SD_INODE_HEADER_SIZE]; > struct sheepdog_inode *inode = (struct sheepdog_inode *)buf; > @@ -528,20 +530,26 @@ static int vdi_snapshot(int argc, char **argv) > return EXIT_FAILURE; > } > > - ret = sd_read_object(vid_to_vdi_oid(vid), inode, SD_INODE_HEADER_SIZE, 0); > - if (ret != SD_RES_SUCCESS) { > - fprintf(stderr, "Failed to read an inode header\n"); > + ret = do_vdi_create(vdiname, inode->vdi_size, vid, &own_vid, 1); > + > + if (ret < 0) { > + fprintf(stderr, "Failed to write VDI %s\n", vdiname); > return EXIT_FAILURE; > } > > if (vdi_cmd_data.snapshot_tag[0]) { > - ret = sd_write_object(vid_to_vdi_oid(vid), 0, vdi_cmd_data.snapshot_tag, > + ret = sd_read_object(vid_to_vdi_oid(own_vid), inode, SD_INODE_HEADER_SIZE, 0); > + if (ret != SD_RES_SUCCESS) { > + fprintf(stderr, "Failed to read an inode header\n"); > + return EXIT_FAILURE; > + } > + ret = sd_write_object(vid_to_vdi_oid(own_vid), 0, vdi_cmd_data.snapshot_tag, > SD_MAX_VDI_TAG_LEN, > offsetof(struct sheepdog_inode, tag), > 0, inode->nr_copies, 0); > } > > - return do_vdi_create(vdiname, inode->vdi_size, vid, NULL, 1); > + return ret; > } > > static int vdi_clone(int argc, char **argv) > @@ -1150,7 +1158,7 @@ static int vdi_write(int argc, char **argv) > remain -= ret; > } > > - inode->data_vdi_id[idx] = inode->vdi_id; > + inode->data_vdi_id[idx] = inode->io_vdi_id; > oid = vid_to_data_oid(inode->data_vdi_id[idx], idx); > ret = sd_write_object(oid, old_oid, buf, len, offset, flags, > inode->nr_copies, create); > @@ -1161,9 +1169,9 @@ static int vdi_write(int argc, char **argv) > } > > if (create) { > - ret = sd_write_object(vid_to_vdi_oid(vid), 0, &vid, sizeof(vid), > - SD_INODE_HEADER_SIZE + sizeof(vid) * idx, 0, > - inode->nr_copies, 0); > + ret = sd_write_object(vid_to_vdi_oid(vid), 0, &inode->io_vdi_id, > + sizeof(inode->io_vdi_id), SD_INODE_HEADER_SIZE + sizeof(vid) * idx, > + 0, inode->nr_copies, 0); > if (ret) { > ret = EXIT_FAILURE; > goto out; > diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h > index 2d0d5ec..a4d4093 100644 > --- a/include/sheepdog_proto.h > +++ b/include/sheepdog_proto.h > @@ -176,7 +176,6 @@ struct sheepdog_inode { > char name[SD_MAX_VDI_LEN]; > char tag[SD_MAX_VDI_TAG_LEN]; > uint64_t ctime; > - uint64_t snap_ctime; > uint64_t vm_clock_nsec; > uint64_t vdi_size; > uint64_t vm_state_size; > @@ -186,9 +185,10 @@ struct sheepdog_inode { > uint32_t snap_id; > uint32_t vdi_id; > uint32_t parent_vdi_id; > + uint32_t io_vdi_id; What do you mean by 'io_'? Isn't there a better name? > uint32_t child_vdi_id[MAX_CHILDREN]; > uint32_t data_vdi_id[MAX_DATA_OBJS]; > -}; > +} __attribute__((packed)); Align to 64 bit instead of using the 'packed' attribute. Thanks, Kazutaka |