On Mon, Apr 22, 2013 at 08:10:58PM +0800, Liu Yuan wrote: > On 04/22/2013 08:00 PM, Stefan Hajnoczi wrote: > > On Mon, Apr 22, 2013 at 02:59:10PM +0800, Liu Yuan wrote: > >> +static coroutine_fn int > >> +sd_co_is_allocated(BlockDriverState *bs, int64_t sector_num, int > >> nb_sectors, > >> + int *pnum) > >> +{ > >> + BDRVSheepdogState *s = bs->opaque; > >> + SheepdogInode *inode = &s->inode; > >> + unsigned long start = sector_num * BDRV_SECTOR_SIZE / > >> SD_DATA_OBJ_SIZE, idx, > >> + end = DIV_ROUND_UP((sector_num + nb_sectors) * > >> + BDRV_SECTOR_SIZE, SD_DATA_OBJ_SIZE); > > > > Please put the variable declarations on separate lines, it's very easy > > to miss "idx". > > Okay. > > > > >> + int ret = 1; > >> + > >> + for (idx = start; idx <= end; idx++) { > > > > Should this be idx < end? Otherwise you are checking one beyond the > > last SD_DATA_OBJ_SIZE. > > No. the end is index of last object, not index of last object + 1.
Imagine sector_num = 0 and nb_sectors = SD_DATA_OBJ_SIZE / BDRV_SECTOR_SIZE. start = 0 end = 1 You don't want object 1, only object 0.