<div class="gmail_extra"><br><br>
<div class="gmail_quote">On Tue, Apr 24, 2012 at 1:00 AM, MORITA Kazutaka <span dir="ltr"><<a href="mailto:morita.kazutaka@gmail.com" target="_blank">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 Mon, 23 Apr 2012 11:37:11 +0800,<br>
<div>
<div class="h5">HaiTing Yao wrote:<br>><br>> On Sat, Apr 21, 2012 at 4:56 AM, MORITA Kazutaka<br>> <<a href="mailto:morita.kazutaka@gmail.com">morita.kazutaka@gmail.com</a>>wrote:<br>><br>> > At Fri, 20 Apr 2012 14:40:26 +0800,<br>
> >  <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             |   33 ++++++++++++++++++++++-----------<br>> > >  include/sheepdog_proto.h |    6 ++++--<br>> > >  sheep/vdi.c              |   18 ++++++++++--------<br>
> > >  4 files changed, 37 insertions(+), 22 deletions(-)<br>> ><br>> > Your patch breaks the relationship between VDI snapshots.<br>> > For example:<br>> > ==<br>> >  $ collie cluster format -c 1<br>
> ><br>> >  $ qemu-img create sheepdog:test 4G<br>> >  $ qemu-img snapshot -c tag1 sheepdog:test<br>> >  $ qemu-img snapshot -c tag2 sheepdog:test<br>> >  $ qemu-img snapshot -c tag3 sheepdog:test<br>
> ><br>> >  $ qemu-img create sheepdog:test2 4G<br>> >  $ qemu-img snapshot -c tag1 sheepdog:test2<br>> >  $ qemu-img snapshot -c tag2 sheepdog:test2<br>> >  $ qemu-io -c "write 0 512" sheepdog:test2:1<br>
> >  $ qemu-img snapshot -c tag3 sheepdog:test2<br>> ><br>> >  $ collie vdi tree<br>> > ==<br>> ><br>> > and the expected output is:<br>> > ==<br>> >  test---[2012-04-21 05:51]---[2012-04-21 05:51]---[2012-04-21<br>
> > 05:51]---(you are here)<br>> >  test2---[2012-04-21 05:51]-+-[2012-04-21 05:51]---[2012-04-21 05:51]<br>> >                             `-[2012-04-21 05:51]---(you are here)<br>> > ==<br>> ><br>
><br>> I think you have applied my block/sheepdog.c patch for qemu project. There<br>> is still something to do. qemu-img is still having some problem with object<br>> cache when there is snapshot. The work is on the progress by Liuyuan as I<br>
> knew. Perhaps you can shutdown object cache temporarily by this:<br><br></div></div>No, the cache is not a problem here.<br>
<div class="im"><br><br>><br>> void do_io_request(struct work *work)<br>><br>>         dprintf("%x, %" PRIx64" , %u\n", opcode, oid, epoch);<br>>        + hdr->flags = hdr->flags & ~SD_FLAG_CMD_CACHE;<br>
>         if (hdr->flags & SD_FLAG_CMD_RECOVERY)<br>><br>> Then the display like this<br>><br>> [wujue.yht@v134092.sqa.cm4 ~]$ clv<br>>   Name        Id    Size    Used  Shared    Creation time   VDI id  Tag<br>
>   v1           0  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:45   709128<br>>   img1         0  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:46   9e028f<br>> s img1         1  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:46   9e0290  img1-sn1<br>
> s img1         2  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:47   9e0291  img1-sn2<br>> s img1         3  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:47   9e0292  img1-sn3<br>>   img2         0  1.0 GB  0.0 MB  0.0 MB 2012-04-23 10:47   9e0442<br>
> [wujue.yht@v134092.sqa.cm4 ~]$ ./collie vdi tree<br>> v1---(you are here)<br>> img1---(you are here)-+-[2012-04-23 10:46]<br>>                       |-[2012-04-23 10:47]<br>>                       `-[2012-04-23 10:47]<br>
> img2---(you are here)<br>> The snapshots will not display on one line, because all of snapshots belong<br>> to base VDI now and there is no direct relation between snapshots. I am not<br>> sure of this. Do we need display them on one line?<br>
><br>> Because the base VDI never be changed, the postion identified by 'you are<br>> here' always follows base VDI after my change. Do we expect this?<br><br></div>The problem is that your patch doesn't set VDI relationships<br>
correctly.  'parent_vdi_id' in struct sheepdog_inode should be the<br>source vdi id, and 'child_vdi_id' should contain children vdi.  If you<br>create a new vdi (or snapshot) B from A, A is a parent of B.<br>
</blockquote>
<div> </div>
<div>I used collie tool to test and get this result.</div>
<div> </div>
<div>Before my modification, the relation as belowe</div>
<div> </div>
<div>[wujue.yht@v134092.sqa.cm4 ~]$ collie vdi list<br>  Name        Id    Size    Used  Shared    Creation time   VDI id  Tag<br>s v1           1  1.0 GB   20 MB  0.0 MB 2012-04-24 11:23   709128  <br>s v1           2  1.0 GB  0.0 MB   20 MB 2012-04-24 11:24   709129  <br>
  v1           3  1.0 GB  0.0 MB   20 MB 2012-04-24 11:24   70912a  </div>
<div> </div>
<div>vdi ID 709128<br>its parent ID 0<br>its 1st child id 709129</div>
<div> </div>
<div>vdi ID 709129<br>its parent ID 709128<br>its 1st child id 70912a</div>
<div> </div>
<div>vdi ID 70912a<br>its parent ID 709129</div>
<div>no child</div>
<div> </div>
<div>After my modification, the relation as belowe. All of snapshots belong to base VDI. Would you mind give the details you expected, thanks.</div>
<div> </div>
<div>[wujue.yht@v134092.sqa.cm4 ~]$ collie vdi list<br>  Name        Id    Size    Used  Shared    Creation time   VDI id  Tag<br> v1           1  1.0 GB   20 MB  0.0 MB 2012-04-24 11:23   709128  <br>s v1           2  1.0 GB  0.0 MB   20 MB 2012-04-24 11:24   709129  <br>
s v1           3  1.0 GB  0.0 MB   20 MB 2012-04-24 11:24   70912a </div>
<div> </div>
<div>vdi ID 709128<br>its parent ID 0<br>its 1st child id 709129<br>its 2nd child id 70912a</div>
<div><br>vdi ID 709129<br>its parent ID 709128</div>
<div>no child</div>
<div> </div>
<div>vdi ID 70912a<br>its parent ID 709128 
<div>no child</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>> BTW, since we change the inode structure, how about increse the<br>> SD_PROTO_VER?<br><br></div>If we would merge this change, we must increment SD_PROTO_VER.  But<br>I'm still not sure this change is really necessary.  As I said before,<br>
I'd like to avoid changing the sheepdog_inode structure if possible.<br>If we don't show VDI id in the output of 'collie vdi list', there is<br>no confusing issue you want to fix, no?<br><br>Thanks,<br><br>
Kazutaka<br></blockquote>
<div> </div>
<div>Besides confusing, there is some other conditions we need to consider.</div>
<div> </div>
<div>1, find_first_vdi() looks up from the right side of the hash point. The new created VDI bacomes the base VDI, so we must begin looking up from right side. We find the next zero from the hash point at bitmap, and use the next zero as the right side edge. Then we can not clear bitmap anytime, because we need the next zero as the right side point. The 1 at bitmap does not mean the VDI ID is used, we must check the inode object and find its name is null or not. Without the patch, we must do like this and did like this. With the patch, base VDI is never changed so we can look up from the left side.</div>

<div> </div>
<div>2, Before the patch, every VDI have one child at most in inode. It is hard to control children maximum, and can not delete any inode object of snapshots. If we miss one, the tree list is broken because parent/child relation is one by one.</div>

<div> </div>
<div>I do not thik this patch is good enough for these problems, and perhaps we have better soultions.</div>
<div> </div>
<div>Thanks </div>
<div>Haiting</div>
<div> </div>
<div>    </div></div><br></div>