From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47427) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TCFLA-00054Z-OR for qemu-devel@nongnu.org; Thu, 13 Sep 2012 15:44:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TCFL9-0001Z5-Sc for qemu-devel@nongnu.org; Thu, 13 Sep 2012 15:44:44 -0400 Received: from mail-wg0-f53.google.com ([74.125.82.53]:54728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TCFL9-0001Z0-ME for qemu-devel@nongnu.org; Thu, 13 Sep 2012 15:44:43 -0400 Received: by wgbdr1 with SMTP id dr1so11582wgb.10 for ; Thu, 13 Sep 2012 12:44:42 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <505237A6.7090202@redhat.com> Date: Thu, 13 Sep 2012 21:44:38 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <01df3140216ac0398bfb3a295c553c42cdf31e5b.1347548248.git.jcody@redhat.com> <5052060A.5010304@redhat.com> <50521531.6010308@redhat.com> <50522C73.5030100@redhat.com> <50522E32.4030406@redhat.com> In-Reply-To: <50522E32.4030406@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 06/16] block: do not parse BDRV_O_CACHE_WB in raw block drivers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: jcody@redhat.com Cc: kwolf@redhat.com, supriyak@linux.vnet.ibm.com, eblake@redhat.com, qemu-devel@nongnu.org, stefanha@gmail.com Il 13/09/2012 21:04, Jeff Cody ha scritto: >> > Perhaps we _should_ preserve that in bs->open_flags, while still using >> > the initial value of BDRV_O_CACHE_WB to initialize bs->enable_write_cache. > That would work. Part of the problem is that BDRV_O_CACHE_WB seems > overloaded; maybe bdrv_parse_cache_flags() should set a new flag, called > BDRV_O_CACHE_WCE, which can be used in lieu of enable_write_cache > (similar to getting rid of keep_read_only in favor of > BDRV_O_ALLOW_RDWR). And then bdrv_parse_cache_flags() would just not > set BDRV_O_CACHE_WB, which can then be used exclusively by the lower > layers for their parsing (and bdrv_open_common would just set > bs->open_flags to always have BDRV_O_CACHE_WB). > > Then patch 2/16 would change to having bdrv_set_enable_write_cache() > toggle BDRV_O_CACHE_WCE. > Yeah, that would work. Alternatively, we can keep this patch and leave bdrv_open_common as is; but then I would also prefer if raw-{posix,win32} took care of setting BDRV_O_CACHE_WB in bs->open_flags, so that the flags are consistent. This setting of BDRV_O_CACHE_WB can be extended later to other formats. Considering this and my comments to patch 3/16 we would have: - after opening with cache=writethrough: bs bs->file enable_write_cache true true BDRV_O_CACHE_WB false true bdrv_enable_write_cache() false true - after opening with cache=writeback: bs bs->file enable_write_cache true true BDRV_O_CACHE_WB true true bdrv_enable_write_cache() true true Paolo