Re: [Qemu-devel] [PATCH] sheepdog: implement direct write semantics

[MORITA Kazutaka]

At Thu, 20 Dec 2012 02:29:31 +0800,
Liu Yuan wrote:
> 
> From: Liu Yuan <address@hidden>
> 
> Sheepdog supports both writeback/writethrough write but has not yet supported
> DIRECTIO semantics which bypass the cache completely even if Sheepdog daemon 
> is
> set up with cache enabled.
> 
> Suppose cache is enabled on Sheepdog daemon size, the new cache control is
> 
> cache=writeback # enable the writeback semantics for write
> cache=writethrough # enable the writethrough semantics for write
> cache='directsync | none | off' # disable cache competely

I wonder if we should disable cache when cache=none.  Many management
frontend uses cache=none by default but, I think, users still expect
that data is cached (e.g. by disk write cache when a raw format is
used).  cache=none only means that the host page cache is not used for
VM disk IO.

In that sense,

> @@ -1118,12 +1118,19 @@ static int sd_open(BlockDriverState *bs, const char 
> *filename, int flags)
>          goto out;
>      }
>  
> -    s->cache_enabled = true;
> -    s->flush_fd = connect_to_sdog(s->addr, s->port);
> -    if (s->flush_fd < 0) {
> -        error_report("failed to connect");
> -        ret = s->flush_fd;
> -        goto out;
> +    if (flags & BDRV_O_NOCACHE) {
> +        s->cache_flags = SD_FLAG_CMD_DIRECT;
> +    } else if (flags & BDRV_O_CACHE_WB) {

'else' should be removed, and

> +        s->cache_flags = SD_FLAG_CMD_CACHE;
> +    }
> +
> +    if (s->cache_flags != SD_FLAG_CMD_DIRECT) {

should be 's->cache_flags == SD_FLAG_CMD_CACHE'?  Do we need to send a
flush request when cache=writethourgh?

Thanks,

Kazutaka