I just took a quick look at the flush code.
Il 13/06/2012 16:36, Dong Xu Wang ha scritto:
>
> +bool add_cow_cache_set_writethrough(BlockDriverState *bs, AddCowCache *c,
> + bool enable)
> +{
> + bool old = c->writethrough;
> +
> + if (!old && enable) {
> + add_cow_cache_flush(bs, c);
> + }
> +
> + c->writethrough = enable;
> + return old;
> +}
Writethrough mode should not be needed anymore in 1.2 if the new
implementation of writethrough is added.
And anyway...
> + if (changed) {
> + ret = add_cow_cache_flush(bs, s->bitmap_cache);
> + }
... here you're treating the cache essentially as writethrough. Is this
flush necessary?
> + qemu_co_mutex_unlock(&s->lock);
> + qemu_iovec_destroy(&hd_qiov);
> + return ret;
> +}
> +
> +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
> +{
> + BDRVAddCowState *s = bs->opaque;
> + int sector_per_byte = SECTORS_PER_CLUSTER * 8;
> + int ret;
> + int64_t bitmap_size =
> + (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
> +
> + ret = bdrv_truncate(bs->file,
> + ADD_COW_BITMAP_POS + bitmap_size);
> + if (ret < 0) {
> + return ret;
> + }
> + bdrv_truncate(s->image_hd, size);
> + return 0;
> +}
> +
> +static coroutine_fn int add_cow_co_flush(BlockDriverState *bs)
> +{
> + BDRVAddCowState *s = bs->opaque;
> + int ret;
> +
> + qemu_co_mutex_lock(&s->lock);
> + ret = add_cow_cache_flush(bs, s->bitmap_cache);
> + qemu_co_mutex_unlock(&s->lock);
> + if (ret < 0) {
> + return ret;
> + }
> +
> + return bdrv_co_flush(bs->file);
Flushing bs->file here is not needed anymore...
> +}
> +
> +static QEMUOptionParameter add_cow_create_options[] = {
> + {
> + .name = BLOCK_OPT_SIZE,
> + .type = OPT_SIZE,
> + .help = "Virtual disk size"
> + },
> + {
> + .name = BLOCK_OPT_BACKING_FILE,
> + .type = OPT_STRING,
> + .help = "File name of a base image"
> + },
> + {
> + .name = BLOCK_OPT_IMAGE_FILE,
> + .type = OPT_STRING,
> + .help = "File name of a image file"
> + },
> + {
> + .name = BLOCK_OPT_BACKING_FMT,
> + .type = OPT_STRING,
> + .help = "Image format of the base image"
> + },
> + { NULL }
> +};
> +
> +static BlockDriver bdrv_add_cow = {
> + .format_name = "add-cow",
> + .instance_size = sizeof(BDRVAddCowState),
> + .bdrv_probe = add_cow_probe,
> + .bdrv_open = add_cow_open,
> + .bdrv_close = add_cow_close,
> + .bdrv_create = add_cow_create,
> + .bdrv_co_readv = add_cow_co_readv,
> + .bdrv_co_writev = add_cow_co_writev,
> + .bdrv_truncate = bdrv_add_cow_truncate,
> + .bdrv_co_is_allocated = add_cow_is_allocated,
> +
> + .create_options = add_cow_create_options,
> + .bdrv_co_flush_to_disk = add_cow_co_flush,
... and this should be bdrv_co_flush_to_os.
Paolo
