Re: [parted-devel] [PATCH] Fall back to not using O_DIRECT

[Colin Watson]

On Fri, Aug 08, 2008 at 04:00:20PM +0200, Jim Meyering wrote:
> Olaf Hering <address@hidden> 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                                       address@hidden

0001-Use-fsync-rather-than-O_DIRECT.patch
Description: Text Data