Colin Watson wrote:
> On Fri, Aug 08, 2008 at 04:00:20PM +0200, Jim Meyering wrote:
>> Olaf Hering <o...@suse.de> wrote:
>> > On Wed, Aug 06, Jim Meyering wrote:
>> >> Can I expect you to adjust your patch to detect
>> >> fsync and close failures?
>> >
>> > How should it handle the failures?
>> > parted cant do anything about the error in practice.
>>
>> Parted should report the write error, and propagate the failure "up" the
>> call tree. Any time Parted ignores a write failure, that is a potential
>> for serious data loss. An obvious bug.
>>
>> The hard part (given the current implementation) is making a write
>> failure translate to a parted exit status that is nonzero. For now,
>> if you would at least make it diagnose any failure, that'd be enough.
>>
>> For example, _do_fsync detects fsync failure and reports it.
>> Speaking of _do_fsync, the added fsync calls in your patch end up
>> being redundant with the fsync call performed by _do_fsync in some
>> code paths, but that's probably not worth worrying about.
>
> It's not clear that anyone ever made the changes Jim requested. How
> about the attached patch, which incorporates Olaf's previous patch and
> turns fsync/close failures into a retry/ignore exception? I tested this
> with the following LD_PRELOADable wrapper; compile with 'gcc -Wall -c -o
> break-fsync.o break-fsync.c && gcc -shared -o break-fsync.so
> break-fsync.o -ldl', and run with 'LD_PRELOAD=/path/to/break-fsync.so'.
>
> #define _GNU_SOURCE
> #include <errno.h>
> #include <dlfcn.h>
> #include <unistd.h>
>
> static int (*_fsync)(int fd) = NULL;
>
> int fsync(int fd)
> {
> if (!_fsync)
> _fsync = dlsym(RTLD_NEXT, "fsync");
> _fsync(fd);
> errno = EIO;
> return -1;
> }
>
> Thanks,
>
> --
> Colin Watson [cjwat...@ubuntu.com]
>
>>From 0f8f1c9dbaae6f3ccd4840a3b0d3815bffeb3f46 Mon Sep 17 00:00:00 2001
> From: Colin Watson <cjwat...@ubuntu.com>
> Date: Fri, 24 Jul 2009 18:25:22 +0100
> Subject: [PATCH] Use fsync rather than O_DIRECT
>
> O_DIRECT doesn't work on all filesystems (e.g. tmpfs), and fsync does
> just as good a job to ensure that buffers are flushed.
>
> Based on
> http://lists.alioth.debian.org/pipermail/parted-devel/2008-August/002392.html
> by Olaf Hering, but with added fsync/close error checking.
> ---
> libparted/arch/linux.c | 27 +++++++++++++++++++--------
> 1 files changed, 19 insertions(+), 8 deletions(-)
Hi Colin,
Thanks a lot for finishing that change.
It looks fine and passes the tests, so I pushed it to "next".
_______________________________________________
bug-parted mailing list
bug-parted@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-parted
