Re: [Qemu-devel] [PATCH 02/17] cow: make writes go at a less indecent speed

[Fam Zheng]

On Wed, 07/03 16:34, Paolo Bonzini wrote:
> Only sync once per write, rather than once per sector.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  block/cow.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/block/cow.c b/block/cow.c
> index 204451e..133e596 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -106,7 +106,7 @@ static int cow_open(BlockDriverState *bs, QDict *options, 
> int flags)
>   * XXX(hch): right now these functions are extremely inefficient.
>   * We should just read the whole bitmap we'll need in one go instead.
>   */
> -static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum)
> +static inline int cow_set_bit(BlockDriverState *bs, int64_t bitnum, bool 
> *first)
Why flush _before first_ write, rather than (more intuitively) flush
_after last_ write? And personally I think "bool sync" makes a better
signature than "bool *first", although it's not that critical as
cow_update_bitmap is the only caller.
>  {
>      uint64_t offset = sizeof(struct cow_header_v2) + bitnum / 8;
>      uint8_t bitmap;
> @@ -117,9 +117,21 @@ static inline int cow_set_bit(BlockDriverState *bs, 
> int64_t bitnum)
>         return ret;
>      }
>  
> +    if (bitmap & (1 << (bitnum % 8))) {
> +        return 0;
> +    }
> +
> +    if (*first) {
> +        ret = bdrv_flush(bs->file);
> +        if (ret < 0) {
> +           return ret;
> +        }
> +        *first = false;
> +    }
> +
>      bitmap |= (1 << (bitnum % 8));
>  
> -    ret = bdrv_pwrite_sync(bs->file, offset, &bitmap, sizeof(bitmap));
> +    ret = bdrv_pwrite(bs->file, offset, &bitmap, sizeof(bitmap));
>      if (ret < 0) {
>         return ret;
>      }
> @@ -181,9 +193,10 @@ static int cow_update_bitmap(BlockDriverState *bs, 
> int64_t sector_num,
>  {
>      int error = 0;
>      int i;
> +    bool first = true;
>  
>      for (i = 0; i < nb_sectors; i++) {
> -        error = cow_set_bit(bs, sector_num + i);
> +        error = cow_set_bit(bs, sector_num + i, &first);
>          if (error) {
>              break;
>          }
> -- 
> 1.8.2.1
> 
> 
> 

-- 
Fam