[sheepdog] [PATCH v3 1/3] sheep: handle VID overflow correctly

[Hitoshi Mitake]

At Fri, 27 Feb 2015 16:55:41 +0800,
Liu Yuan wrote:
> 
> On Fri, Feb 27, 2015 at 04:51:21PM +0800, Liu Yuan wrote:
> > On Fri, Feb 27, 2015 at 01:46:14PM +0900, Hitoshi Mitake wrote:
> > > Current sheep cannot handle a case like this:
> > > 1. iterate snapshot creation and let latest working VDI have VID 0xffffff
> > > 2. create one more snapshot
> > > 
> > > This problem comes from invalid usage of find_next_zero_bit() of sheep.
> > > Current sheep checks its VID bitmap with find_next_zero_bit(). But the
> > > function has a subtle point we must care about. If we check a bitmap
> > > and whose right most bit is 1, it returns a number of the bit though
> > > the bit is not 0. It means
> > >     /* the right most bit of sys->vdi_inuse is 1*/
> > >     find_next_zero_bit(sys->vdi_inuse, SD_NR_VDIS, 0);
> > > will return SD_NR_VDIS - 1 (not SD_NR_VDIS, of course). So it is not
> > > possible to check the right most bit is used or not by simply calling
> > > find_next_zero_bit(). So the existing code cannot handle the case of
> > > overflow.
> > 
> > I did a small test, it looks like following:
> > 
> > int main()
> > {
> > 	long i = 0b111111;
> > 	printf("%lu\n", find_next_zero_bit(&i, 6, 5));
> > 	printf("%lu\n", find_next_zero_bit(&i, 6, 0));
> > }
> > 
> > It mimics what you described. But the result is
> > 
> > yliu at ubuntu-trusty:~/sheepdog$ ./a.out 
> > 6
> > 6
> > 
> > which means find_next_zero_bit does return SD_NR_VDIS in your case. Am I missing
> > anything?
> > 
> 
> find_next_zero_bit return SD_NR_VDIS to indicate no valid bit is found, no? For
> our case, [0, SD_NR_VDIS) is valid range, so it is safe to use SD_NR_VDIS to
> indicate the no-zero-bit case.

My local test had something wrong. 0xffffff can be used as a valid VID.

Thanks,
Hitoshi

> 
> Thanks
> Yuan
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> https://lists.wpkg.org/mailman/listinfo/sheepdog