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

[Paolo Bonzini]

Il 03/07/2012 15:46, Kevin Wolf ha scritto:
>> > 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?

Both posted.  The spec patch that introduces VIRTIO_BLK_F_CONFIG_WCE
also renames it to VIRTIO_BLK_F_WCE, since that's really what it does.
See this old comment in the kernel (not in the latest git anymore):

   /*
* If the FLUSH feature is supported we do have support for
* flushing a volatile write cache on the host.  [...]
* otherwise, we must assume that the host does not
* perform any kind of volatile write caching.
*/

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Kevin Wolf]

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 
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html