From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:47584) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIFUL-0003Yp-FK for qemu-devel@nongnu.org; Mon, 24 Oct 2011 04:02:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RIFUH-0001yA-LY for qemu-devel@nongnu.org; Mon, 24 Oct 2011 04:02:29 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50715) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RIFUH-0001xx-By for qemu-devel@nongnu.org; Mon, 24 Oct 2011 04:02:25 -0400 Message-ID: <4EA51C45.7030800@redhat.com> Date: Mon, 24 Oct 2011 10:05:25 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1319216912-26964-1-git-send-email-kwolf@redhat.com> <4EA1BD95.8030205@redhat.com> <4EA425B7.2030708@redhat.com> In-Reply-To: <4EA425B7.2030708@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/2] block: Write out internal caches even with cache=unsafe List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Alexander Graf , qemu-devel@nongnu.org, avi@redhat.com Am 23.10.2011 16:33, schrieb Paolo Bonzini: > On 10/22/2011 05:07 PM, Alexander Graf wrote: >> >> On 21.10.2011, at 11:44, Paolo Bonzini wrote: >> >>> On 10/21/2011 07:08 PM, Kevin Wolf wrote: >>>> Avi complained that not even writing out qcow2's cache on >>>> bdrv_flush() made cache=unsafe too unsafe to be useful. He's got >>>> a point. >>> >>> Why? cache=unsafe is explicitly allowing to s/data/manure/ on >>> crash. >> >> Exactly, but not on kill. By not flushing internal caches you're >> almost guaranteed to get an inconsistent qcow2 image. > > This should be covered already by termsig_handler. bdrv_close_all > closes all block devices, and qcow2_close does flush the caches. > > SIGKILL doesn't give any guarantee of course but it does not in general, > even without cache=unsafe; you might get a SIGKILL "a moment before" a > bdrv_flush even without cache=unsafe, and get unclean qcow2 metadata. Unclean yes, in the sense that you may get cluster leaks. If getting SIGKILL "a moment before" the flush led to real corruption however, cache=none would be broken as well. >> By not flushing internal caches you're almost guaranteed to get an >> inconsistent qcow2 image. > > Of course the inconsistencies with cache=unsafe will be massive if you > don't have a clean exit, but that's expected. If in some cases you want > a clean exit, but right now you don't, the place to fix those cases > doesn't seem to be the block layer, but the main loop. I don't think there's much the main loop can do against SIGKILL, segfaults or abort(). > Also, > > 1) why should cache=unsafe differentiate an OS that sends a flush from > one that doesn't (e.g. MS-DOS), from the point of view of image metadata? > > 2) why should the guest actually send a flush if cache=unsafe? Currently > > if (flags & BDRV_O_CACHE_WB) > bs->enable_write_cache = 1; > > covers cache=unsafe. However, in the end write cache enable means "do I > need to flush data", and the answer is "no" when cache=unsafe, because > the flushes are useless and guests are free to reorder requests. > > Perhaps what you want is to make qcow2 caches > writethrough in cache=unsafe mode, so that at least a try is made to > write the metadata (even though the underlying raw > protocol won't flush it)? I'm not sure that is particularly useful, but > maybe it can help me understanding the benefit of this change. Yes, this is the intention. It's about flushing metadata, not guest data. The semantics that I think cache=unsafe should have is that after a bdrv_flush() we have flushed all caches in qemu (so that the image survives a qemu crash), but we don't care about flushing the host page cache. Kevin