Commit-ID: 2fc6f0c50e251f4b23b50430e4f95ea3cf87a9dc
Gitweb: http://git.kernel.org/tip/2fc6f0c50e251f4b23b50430e4f95ea3cf87a9dc
Author: Pekka Enberg <penberg@xxxxxxxxxx>
AuthorDate: Thu, 16 Jun 2011 17:18:58 +0300
Committer: Pekka Enberg <penberg@xxxxxxxxxx>
CommitDate: Thu, 16 Jun 2011 17:18:58 +0300
kvm tools, qcow: Use fdatasync() instead of sync_file_range()
As explained by Christoph Hellwig, sync_file_range() is not sufficient to
guarantee that Qcow image metadata is never corrupted:
On Thu, Jun 16, 2011 at 12:34:04PM +0300, Pekka Enberg wrote:
> Hi Christoph,
>
> On Thu, Jun 16, 2011 at 09:21:03AM +0300, Pekka Enberg wrote:
> >> And btw, we use sync_file_range()
>
> On Thu, Jun 16, 2011 at 12:24 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
> > Which doesn't help you at all. ?sync_file_range is just a hint for VM
> > writeback, but never commits filesystem metadata nor the physical
> > disk's write cache. ?In short it's a completely dangerous interface, and
> > that is pretty well documented in the man page.
>
> Doh - I didn't read it carefully enough and got hung up with:
>
> Therefore, unless the application is strictly performing overwrites of
> already-instantiated disk blocks, there are no guarantees that the data will
> be available after a crash.
>
> without noticing that it obviously doesn't work with filesystems like
> btrfs that do copy-on-write.
You also missed:
" This system call does not flush disk write caches and thus does not
provide any data integrity on systems with volatile disk write
caches."
so it's not safe if you either have a cache, or are using btrfs, or
are using a sparse image, or are using an image preallocated using
fallocate/posix_fallocate.
> What's the right thing to do here? Is fdatasync() sufficient?
Yes.
Cc: Ingo Molnar <mingo@xxxxxxx>
Cc: Prasad Joshi <prasadjoshi124@xxxxxxxxx>
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Signed-off-by: Pekka Enberg <penberg@xxxxxxxxxx>
---
tools/kvm/disk/qcow.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index 7b1563b..3f8c52d 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -322,14 +322,12 @@ static inline u64 file_size(int fd)
return st.st_size;
}
-#define SYNC_FLAGS (SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE)
-
static inline int qcow_pwrite_sync(int fd, void *buf, size_t count, off_t offset)
{
if (pwrite_in_full(fd, buf, count, offset) < 0)
return -1;
- return sync_file_range(fd, offset, count, SYNC_FLAGS);
+ return fdatasync(fd);
}
/* Writes a level 2 table at the end of the file. */
--
To unsubscribe from this list: send the line "unsubscribe linux-tip-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
|