<br><br>
<div class="gmail_quote">On Sat, Apr 7, 2012 at 1:06 AM, MORITA Kazutaka <span dir="ltr"><<a href="mailto:morita.kazutaka@gmail.com">morita.kazutaka@gmail.com</a>></span> wrote:<br>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">At Tue, 27 Mar 2012 13:53:51 +0800,<br>
<div>
<div class="h5"><a href="mailto:yaohaiting.wujue@gmail.com">yaohaiting.wujue@gmail.com</a> wrote:<br>><br>> From: HaiTing Yao <<a href="mailto:wujue.yht@taobao.com">wujue.yht@taobao.com</a>><br>><br>> When create snapshot for source VDI, the new created VDI used as source<br>
> VDI, and the old source VDI used as snapshot. This flow make users<br>> confused about VDI and snapshot relation. The snapshot metadata maybe is<br>> stored on multi-VDI, so need read multi VDIs inode to get snapshot list.<br>
><br>> When create snapshot, we does not need change new created VDI to source<br>> VDI. The source VDI just need use snapshot VDI ID as its object data ID.<br>><br>> Show one example.<br>><br>> Before modification:<br>
><br>> Name Id Size Used Shared Creation time VDI id Tag<br>> s v1 1 64 MB 20 MB 0.0 MB 2012-03-26 16:55 709128<br>> s v1 2 64 MB 0.0 MB 20 MB 2012-03-26 16:56 709129 sn3<br>
> v1 3 64 MB 0.0 MB 20 MB 2012-03-26 16:56 70912a<br>><br>> After modification:<br>><br>> Name Id Size Used Shared Creation time VDI id Tag<br>> v1 0 64 MB 20 MB 0.0 MB 2012-03-27 11:06 709128<br>
> s v1 1 64 MB 0.0 MB 20 MB 2012-03-27 11:06 709129<br>> s v1 2 64 MB 0.0 MB 20 MB 2012-03-27 11:07 70912a sn3<br>><br>> Signed-off-by: HaiTing Yao <<a href="mailto:wujue.yht@taobao.com">wujue.yht@taobao.com</a>><br>
> ---<br>> collie/common.c | 2 +-<br>> collie/vdi.c | 30 +++++++++++++++++++-----------<br>> include/sheepdog_proto.h | 6 +++---<br>> sheep/vdi.c | 18 ++++++++++--------<br>
> 4 files changed, 33 insertions(+), 23 deletions(-)<br><br></div></div>It seems that 'collie vdi tree' and 'collie vdi graph' don't work<br>correctly with this patch.<br></blockquote>
<div> </div>
<div>Thanks for the comments.</div>
<div> </div>
<div>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. </div>
<div> </div>
<div>The base VDI named as "v1" and it has 4 snapshots.</div>
<div> </div>
<div>v1---(you are here)-+-[2012-04-12 10:59]<br> |-[2012-04-12 10:59]<br> |-[2012-04-12 11:00]<br> `-[2012-04-12 11:02]</div>
<div> </div>
<div>digraph G {<br> node [shape = "box", fontname = "Courier"];</div>
<div> "0" [shape = "ellipse", label = "root"];</div>
<div> "0" -> "709128";<br> "709128" [<br> group = "v1",<br> label = "Name: v1\nTag: 0\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:15",<br>
color="red"<br> ];</div>
<div> "709128" -> "709129";<br> "709129" [<br> group = "v1",<br> label = "Name: v1\nTag: 1\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:30"<br>
];</div>
<div> "709128" -> "70912a";<br> "70912a" [<br> group = "v1",<br> label = "Name: v1\nTag: 2\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 10:59:53"<br>
];</div>
<div> "709128" -> "70912b";<br> "70912b" [<br> group = "v1",<br> label = "Name: v1\nTag: 3\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 11:00:47"<br>
];</div>
<div> "709128" -> "70912c";<br> "70912c" [<br> group = "v1",<br> label = "Name: v1\nTag: 4\nSize: 1.0 GB\nDate: 2012-04-12\nTime: 11:02:45"<br>
];</div>
<div>}</div>
<div> </div>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote">
<div class="im"><br>><br>> diff --git a/collie/common.c b/collie/common.c<br>> index f4301c4..636b821 100644<br>> --- a/collie/common.c<br>> +++ b/collie/common.c<br>> @@ -13,7 +13,7 @@<br>><br>> int is_current(struct sheepdog_inode *i)<br>
> {<br>> - return !i->snap_ctime;<br>> + return !i->snap_id;<br>> }<br>><br>> char *size_to_str(uint64_t _size, char *str, int str_size)<br>> diff --git a/collie/vdi.c b/collie/vdi.c<br>
> index e0678a6..8a30c99 100644<br>> --- a/collie/vdi.c<br>> +++ b/collie/vdi.c<br>> @@ -90,7 +90,9 @@ static void print_vdi_list(uint32_t vid, char *name, char *tag, uint32_t snapid,<br>> for (idx = 0; idx < MAX_DATA_OBJS; idx++) {<br>
> if (!i->data_vdi_id[idx])<br>> continue;<br>> - if (is_data_obj_writeable(i, idx))<br>> + if (!i->parent_vdi_id)<br>> + my_objs++;<br>
> + else if ((!i->snap_id) && (i->io_vdi_id == i->data_vdi_id[idx]))<br><br></div>Use is_data_obj_writeable().<br>
<div>
<div class="h5"><br>> my_objs++;<br>> else<br>> cow_objs++;<br>> @@ -511,7 +513,7 @@ out:<br>> static int vdi_snapshot(int argc, char **argv)<br>> {<br>
> char *vdiname = argv[optind++];<br>> - uint32_t vid;<br>> + uint32_t vid, own_vid;<br>> int ret;<br>> char buf[SD_INODE_HEADER_SIZE];<br>> struct sheepdog_inode *inode = (struct sheepdog_inode *)buf;<br>
> @@ -528,20 +530,26 @@ static int vdi_snapshot(int argc, char **argv)<br>> return EXIT_FAILURE;<br>> }<br>><br>> - ret = sd_read_object(vid_to_vdi_oid(vid), inode, SD_INODE_HEADER_SIZE, 0);<br>
> - if (ret != SD_RES_SUCCESS) {<br>> - fprintf(stderr, "Failed to read an inode header\n");<br>> + ret = do_vdi_create(vdiname, inode->vdi_size, vid, &own_vid, 1);<br>> +<br>
> + if (ret < 0) {<br>> + fprintf(stderr, "Failed to write VDI %s\n", vdiname);<br>> return EXIT_FAILURE;<br>> }<br>><br>> if (vdi_cmd_data.snapshot_tag[0]) {<br>
> - ret = sd_write_object(vid_to_vdi_oid(vid), 0, vdi_cmd_data.snapshot_tag,<br>> + ret = sd_read_object(vid_to_vdi_oid(own_vid), inode, SD_INODE_HEADER_SIZE, 0);<br>> + if (ret != SD_RES_SUCCESS) {<br>
> + fprintf(stderr, "Failed to read an inode header\n");<br>> + return EXIT_FAILURE;<br>> + }<br>> + ret = sd_write_object(vid_to_vdi_oid(own_vid), 0, vdi_cmd_data.snapshot_tag,<br>
> SD_MAX_VDI_TAG_LEN,<br>> offsetof(struct sheepdog_inode, tag),<br>> 0, inode->nr_copies, 0);<br>> }<br>
><br>> - return do_vdi_create(vdiname, inode->vdi_size, vid, NULL, 1);<br>> + return ret;<br>> }<br>><br>> static int vdi_clone(int argc, char **argv)<br>> @@ -1150,7 +1158,7 @@ static int vdi_write(int argc, char **argv)<br>
> remain -= ret;<br>> }<br>><br>> - inode->data_vdi_id[idx] = inode->vdi_id;<br>> + inode->data_vdi_id[idx] = inode->io_vdi_id;<br>> oid = vid_to_data_oid(inode->data_vdi_id[idx], idx);<br>
> ret = sd_write_object(oid, old_oid, buf, len, offset, flags,<br>> inode->nr_copies, create);<br>> @@ -1161,9 +1169,9 @@ static int vdi_write(int argc, char **argv)<br>
> }<br>><br>> if (create) {<br>> - ret = sd_write_object(vid_to_vdi_oid(vid), 0, &vid, sizeof(vid),<br>> - SD_INODE_HEADER_SIZE + sizeof(vid) * idx, 0,<br>
> - inode->nr_copies, 0);<br>> + ret = sd_write_object(vid_to_vdi_oid(vid), 0, &inode->io_vdi_id,<br>> + sizeof(inode->io_vdi_id), SD_INODE_HEADER_SIZE + sizeof(vid) * idx,<br>
> + 0, inode->nr_copies, 0);<br>> if (ret) {<br>> ret = EXIT_FAILURE;<br>> goto out;<br>> diff --git a/include/sheepdog_proto.h b/include/sheepdog_proto.h<br>
> index 2d0d5ec..a4d4093 100644<br>> --- a/include/sheepdog_proto.h<br>> +++ b/include/sheepdog_proto.h<br>> @@ -176,7 +176,6 @@ struct sheepdog_inode {<br>> char name[SD_MAX_VDI_LEN];<br>> char tag[SD_MAX_VDI_TAG_LEN];<br>
> uint64_t ctime;<br>> - uint64_t snap_ctime;<br>> uint64_t vm_clock_nsec;<br>> uint64_t vdi_size;<br>> uint64_t vm_state_size;<br>> @@ -186,9 +185,10 @@ struct sheepdog_inode {<br>
> uint32_t snap_id;<br>> uint32_t vdi_id;<br>> uint32_t parent_vdi_id;<br>> + uint32_t io_vdi_id;<br><br></div></div>What do you mean by 'io_'? Isn't there a better name?<br>
<div class="im"><br><br>> uint32_t child_vdi_id[MAX_CHILDREN];<br>> uint32_t data_vdi_id[MAX_DATA_OBJS];<br>> -};<br>> +} __attribute__((packed));<br><br></div>Align to 64 bit instead of using the 'packed' attribute.<br>
</blockquote>
<div> </div>
<div>I tested it and found it can not work well with alignment 64/32. </div>
<div> </div>
<div>#define SD_INODE_HEADER_SIZE (sizeof(struct sheepdog_inode) - \<br> sizeof(uint32_t) * MAX_DATA_OBJS)</div>
<div> </div>
<div>With the alignment 64/32, can not get correct header size. Perhaps the 'packed' is better.</div>
<div> </div>
<div>I have fixed the other bugs in comments and will submit the patch again.</div>
<div> </div>
<div>Thanks</div>
<div>Haiting</div>
<blockquote style="BORDER-LEFT:#ccc 1px solid;MARGIN:0px 0px 0px 0.8ex;PADDING-LEFT:1ex" class="gmail_quote"><br><br>Thanks,<br><br>Kazutaka<br></blockquote></div><br>