Re: [Qemu-devel] [PATCH 08/27] block/parallels: _co_writev callback for Parallels format

[Stefan Hajnoczi]

On Wed, Mar 11, 2015 at 01:28:02PM +0300, Denis V. Lunev wrote:
> +static int64_t allocate_cluster(BlockDriverState *bs, int64_t sector_num)
> +{
> +    BDRVParallelsState *s = bs->opaque;
> +    uint32_t idx, offset, tmp;
> +    int64_t pos;
> +    int ret;
> +
> +    idx = sector_num / s->tracks;
> +    offset = sector_num % s->tracks;
> +
> +    if (idx >= s->catalog_size) {
> +        return -EINVAL;
> +    }
> +    if (s->catalog_bitmap[idx] != 0) {
> +        return (uint64_t)s->catalog_bitmap[idx] * s->off_multiplier + offset;
> +    }
> +
> +    pos = bdrv_getlength(bs->file) >> BDRV_SECTOR_BITS;
> +    bdrv_truncate(bs->file, (pos + s->tracks) << BDRV_SECTOR_BITS);
> +    s->catalog_bitmap[idx] = pos / s->off_multiplier;
> +
> +    tmp = cpu_to_le32(s->catalog_bitmap[idx]);
> +
> +    ret = bdrv_pwrite_sync(bs->file,
> +            sizeof(ParallelsHeader) + idx * sizeof(tmp), &tmp, sizeof(tmp));

What is the purpose of the sync?

> +    if (ret < 0) {
> +        return ret;
> +    }
> +    return (uint64_t)s->catalog_bitmap[idx] * s->off_multiplier + offset;
> +}

This function is missing error handling.  If the catalog bitmap update
cannot be written to file then our in-memory copy should also be
reverted back to 0 (unallocated).

pgp20E5jg1c3_.pgp
Description: PGP signature