qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [sheepdog] [PATCH] sheepdog: implement .bdrv_co_is_allo


From: MORITA Kazutaka
Subject: Re: [Qemu-devel] [sheepdog] [PATCH] sheepdog: implement .bdrv_co_is_allocated
Date: Mon, 22 Apr 2013 15:01:39 +0900
User-agent: Wanderlust/2.15.9 (Almost Unreal) Emacs/23.2 Mule/6.0 (HANACHIRUSATO)

At Thu, 18 Apr 2013 19:48:52 +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 * SECTOR_SIZE / SD_DATA_OBJ_SIZE, idx,

It looks better to use BDRV_SECTOR_SIZE .  I'd suggest preparing
another patch to replace all the SECTOR_SIZE with BDRV_SECTOR_SIZE.

> +                  end = start + (nb_sectors * SECTOR_SIZE) / 
> SD_DATA_OBJ_SIZE;

Using 'start' to calculate 'end' is wrong because 'start' may be
rounded down.

> +
> +    for (idx = start; idx <= end; idx++) {
> +        if (inode->data_vdi_id[idx] == 0) {
> +            break;
> +        }
> +    }
> +    if (idx == start) {
> +        *pnum = SD_DATA_OBJ_SIZE / SECTOR_SIZE;

Isn't it better to set the longest length of the unallocated sectors?

> +        return 0;
> +    }
> +
> +    *pnum = (idx - start) * SD_DATA_OBJ_SIZE / SECTOR_SIZE;
> +    return 1;
> +}
> +

Thanks,

Kazutaka



reply via email to

[Prev in Thread] Current Thread [Next in Thread]