From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH] xen_disk:
	implement	BLKIF_OP_FLUSH_DISKCACHE, remove BLKIF_OP_WRITE_BARRIER
Date: Wed, 25 Apr 2012 10:45:24 +0200
Message-ID: <20120425084524.GA17537@lst.de>
References: <1334595957-12552-1-git-send-email-stefano.stabellini@eu.citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
Content-Disposition: inline
In-Reply-To: <1334595957-12552-1-git-send-email-stefano.stabellini@eu.citrix.com>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: kwolf@redhat.com, xen-devel@lists.xensource.com, qemu-devel@nongnu.org, konrad.wilk@oracle.com
List-Id: xen-devel@lists.xenproject.org

> -    case BLKIF_OP_WRITE_BARRIER:
> +    case BLKIF_OP_FLUSH_DISKCACHE:
>          if (!ioreq->req.nr_segments) {
>              ioreq->presync = 1;
>              return 0;
>          }
> -        ioreq->presync = ioreq->postsync = 1;
> +        ioreq->postsync = 1;
>          /* fall through */

It might be worth documenting the semantics of BLKIF_OP_FLUSH_DISKCACHE
in a comment here.  I haven't found any spec for the xen_disk protocol,
but from looking at the Linux frontend it seems like the semantics
of REQ_FLUSH and REQ_FUA in the Linux block driver are overloaded into
BLKIF_OP_FLUSH_DISKCACHE, which is fairly confusing given that REQ_FLUSH
already overload functionality.

Even worse REQ_FLUSH with a payload implies a preflush, while REQ_FUA
implies a post flush, and it seems like Xen has no way to distinguish
the two, making thing like log writes very inefficient.

Independent of that the implementation should really use a state machine
around bdrv_aio_flush instead of doing guest-sychronous bdrv_flush calls.