From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=47937 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Q0E5k-0002E3-2k
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:22:35 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <hch@lst.de>) id 1Q0E4r-0000O1-79
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:21:26 -0400
Received: from verein.lst.de ([213.95.11.211]:50743 helo=newverein.lst.de)
	by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <hch@lst.de>)
	id 1Q0E4r-0000Nb-0p
	for qemu-devel@nongnu.org; Thu, 17 Mar 2011 10:21:25 -0400
Date: Thu, 17 Mar 2011 15:21:22 +0100
From: Christoph Hellwig <hch@lst.de>
Subject: Re: [Qemu-devel] Re: [PATCH, RFC] virtio_blk: add cache control
	support
Message-ID: <20110317142122.GA27717@lst.de>
References: <20110315141049.GA30627@lst.de> <20110315141644.GA30803@lst.de>
	<87y64fhfjw.fsf@rustcorp.com.au> <20110316140958.GB21877@lst.de>
	<877hbygwu7.fsf@rustcorp.com.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <877hbygwu7.fsf@rustcorp.com.au>
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Rusty Russell <rusty@rustcorp.com.au>
Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, linux-kernel@vger.kernel.org, Christian Borntraeger <borntraeger@de.ibm.com>, prerna@linux.vnet.ibm.com, Christoph Hellwig <hch@lst.de>

On Thu, Mar 17, 2011 at 03:36:08PM +1030, Rusty Russell wrote:
> > I'm happ to switch strcmp.
> 
> Of course, that's assuming buf is nul terminated.

It's the string the user writes into it, which normally should be
nul-terminated.

> > No, it's intentional.  config space writes can't return errors, so we need
> > to check that the value has really changed.  I'll add a comment explaining it.
> 
> OK, under what circumstances could it fail?
> 
> If you're using this mechanism to indicate that the host doesn't support
> the feature, that's making an assumption about the nature of config
> space writes which isn't true for non-PCI virtio.
> 
> ie. lguest and S/390 don't trap writes to config space.
> 
> Or perhaps they should?  But we should be explicit about needing it...

We have the features flag to indicate if updating the caching mode is
supported, but we we could still fail it for other reasons - e.g. we could fail
to reopen the file with/without O_SYNC.  But if lguest or S/390 don't support
trapping config space write this feature won't work for them at all.  As do
other features that make use of config space write, e.g. updating the MAC
address for virtio-net.