From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54174) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TCG9P-0001DQ-B6 for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:36:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TCG9O-0004Yg-Fu for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:36:39 -0400 Received: from mail-wg0-f53.google.com ([74.125.82.53]:33688) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TCG9O-0004Xb-9U for qemu-devel@nongnu.org; Thu, 13 Sep 2012 16:36:38 -0400 Received: by wgbdr1 with SMTP id dr1so41221wgb.10 for ; Thu, 13 Sep 2012 13:36:37 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <50524246.5080800@redhat.com> Date: Thu, 13 Sep 2012 22:29:58 +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> <505237A6.7090202@redhat.com> In-Reply-To: <505237A6.7090202@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: , Cc: kwolf@redhat.com, supriyak@linux.vnet.ibm.com, stefanha@gmail.com, jcody@redhat.com, qemu-devel@nongnu.org, eblake@redhat.com Il 13/09/2012 21:44, Paolo Bonzini ha scritto: > 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. Hmm, no, what was I thinking... But there is a very simple thing we can do: - patch 2 can be left as is - in patch 3 bdrv_reopen_queue, always add BDRV_O_CACHE_WB to bs_entry->state.flags - in patch 3 bdrv_reopen_commit, always leave BDRV_O_CACHE_WB aside: reopen_state->bs->open_flags = (bs->open_flags & BDRV_O_CACHE_WB) | (reopen_state->flags & ~BDRV_O_CACHE_WB); - this patch can be dropped completely. Paolo