Re: [Qemu-devel] [PATCH] block: Change bdrv_commit to handle multiple sectors at once

[Christoph Hellwig]

On Fri, Jul 16, 2010 at 06:17:36PM +0200, Kevin Wolf wrote:
> +    buf = qemu_malloc(2048 * BDRV_SECTOR_SIZE);

Please add a COMMIT_BUF_SIZE #define instead of the hardcoded 2048 in
various places.

>      for (i = 0; i < total_sectors;) {
> +        if (drv->bdrv_is_allocated(bs, i, 2048, &n)) {
>  
> +            if (bdrv_read(bs, i, buf, n) != 0) {
> +                ret = -EIO;
> +                goto ro_cleanup;
> +            }
> +
> +            if (bdrv_write(bs->backing_hd, i, buf, n) != 0) {
> +                ret = -EIO;
> +                goto ro_cleanup;
> +            }
>          }
> +        i += n;

Maybe it's just me, but I'd prefer n getting a more descriptive name
(e.g. sector) and moving the increment of it into the for loop, e.g.

        for (sector = 0; sector < total_sectors; sector += n) {
                if (!drv->bdrv_is_allocated(bs, i, 2048, &n))
                        continue;
                ...
        }

Otherwise this looks good to me.