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

[Liu Yuan]

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.

Thanks
Yuan