On Sat, Apr 7, 2012 at 1:06 AM, MORITA Kazutaka <morita.kazutaka at gmail.com>wrote: > 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. > Thanks for the comments. I tested the patch with vdi tree and graph. It shows as the follow example. It seems right to me, please give the detailed comments about this, thanks. The base VDI named as "v1" and it has 4 snapshots. v1---(you are here)-+-[2012-04-12 10:59] |-[2012-04-12 10:59] |-[2012-04-12 11:00] `-[2012-04-12 11:02] digraph G { node [shape = "box", fontname = "Courier"]; "0" [shape = "ellipse", label = "root"]; "0" -> "709128"; "709128" [ group = "v1", label = "Name: v1\nTag: 0\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:15", color="red" ]; "709128" -> "709129"; "709129" [ group = "v1", label = "Name: v1\nTag: 1\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:30" ]; "709128" -> "70912a"; "70912a" [ group = "v1", label = "Name: v1\nTag: 2\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:53" ]; "709128" -> "70912b"; "70912b" [ group = "v1", label = "Name: v1\nTag: 3\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 11:00:47" ]; "709128" -> "70912c"; "70912c" [ group = "v1", label = "Name: v1\nTag: 4\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 11:02:45" ]; } > > > > > 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. > I tested it and found it can not work well with alignment 64/32. #define SD_INODE_HEADER_SIZE (sizeof(struct sheepdog_inode) - \ sizeof(uint32_t) * MAX_DATA_OBJS) With the alignment 64/32, can not get correct header size. Perhaps the 'packed' is better. I have fixed the other bugs in comments and will submit the patch again. Thanks Haiting > > > Thanks, > > Kazutaka > -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.wpkg.org/pipermail/sheepdog/attachments/20120412/dad3372b/attachment.html> |