[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] block: Change bdrv_commit to handle multiple se
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH] block: Change bdrv_commit to handle multiple sectors at once |
Date: |
Mon, 19 Jul 2010 11:37:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100430 Fedora/3.0.4-2.fc12 Thunderbird/3.0.4 |
Am 16.07.2010 20:16, schrieb 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;
> ...
> }
So you mean i should be renamed, not n, right? I'll change these points
in v2.
Kevin