Re: [PATCH 1/2] virtio-blk: support VIRTIO_BLK_F_CONFIG_WCE

[Kevin Wolf <kwolf@redhat.com>]

Am 03.07.2012 15:20, schrieb Paolo Bonzini:
> Introduce a new feature bit and configuration field that provide
> support for toggling the cache mode between writethrough and writeback.
> 
> Also rename VIRTIO_BLK_F_WCACHE to VIRTIO_BLK_F_WCE for consistency with
> the spec.

My spec (and my kernel as well) call it VIRTIO_BLK_F_FLUSH.

What's the status of the kernel and spec side of the change?

> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/virtio-blk.c |   16 ++++++++++++++--
>  hw/virtio-blk.h |    4 +++-
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index fe07746..280f96d 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -510,9 +510,19 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
>      blkcfg.size_max = 0;
>      blkcfg.physical_block_exp = get_physical_block_exp(s->conf);
>      blkcfg.alignment_offset = 0;
> +    blkcfg.wce = bdrv_enable_write_cache(s->bs);
>      memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
>  }
>  
> +static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
> +{
> +    VirtIOBlock *s = to_virtio_blk(vdev);
> +    struct virtio_blk_config blkcfg;
> +
> +    memcpy(&blkcfg, config, sizeof(blkcfg));
> +    bdrv_set_enable_write_cache(s->bs, blkcfg.wce != 0);
> +}

We need to call bdrv_flush() here when turning WCE off. And it seems we
don't have a way to signal failure, or may we just leave the bit unchanged?

> @@ -49,6 +50,7 @@ struct virtio_blk_config
>      uint8_t alignment_offset;
>      uint16_t min_io_size;
>      uint32_t opt_io_size;
> +    uint8_t wce;
>  } QEMU_PACKED;

If the spec isn't set in stone yet, we could make it a flags field
instead of using a whole byte for a single flag.

Kevin