Package: eatmydata Version: 26-2 Severity: normal Tags: patch Dear Maintainer,
The eatmydata library does not properly handle the fsync (here and
always also the fdatasync) call when called with an invalid file
handle. The proper behaviour as documented in the manpage of fsync is
to return -1 and set errno to EBADF. But eatmydata still returns 0 and
resets errno.
This wrong behaviour exposes when trying to build libvirt while the
build system (cowbuilder in my case) is ran under eatmydata to speed
up things, since the libvirt test cases check for the proper behaviour
of fsync:
(...)
/tmp/buildd/libvirt-0.9.9/./gnulib/tests/test-fsync.c:53: assertion failed
(...)
FAIL: test-fdatasync
(...)
/tmp/buildd/libvirt-0.9.9/./gnulib/tests/test-fdatasync.c:52: assertion
failed
(...)
FAIL: test-fsync
The according lines are
ASSERT (fsync (-1) == -1);
ASSERT (fdatasync (-1) == -1);
And later also (silently assuming fd 99 is closed)
ASSERT (fsync (99) == -1);
How to repeat:
Use the following program:
#include <fcntl.h>
#include <stdio.h>
int
main (void)
{
printf ("fsync: %d\n", fsync (-1));
printf ("fdatasync: %d\n", fdatasync (-1));
}
Running without eatmydata
$ ./a.out
fsync: -1
fdatasync: -1
Runinng under eatmydata should yield the same result but doesn't:
$ eatmydata ./a.out
fsync: 0
fdatasync: 0
How to fix:
The eatmydata code should be kept stateless. Therefore, instead of
keeping track of all filehandles opened, the patch attached checks
the file descriptor using fcntl (F_GETFL). File handles opened for
reading are not treated differently, appearently this does no harm;
the manpage says this should return -1 and set EBADF; the truth as
presented by the real fsync appears to be different.
This approach however breaks eatmydatatest.c tests.
Cheers,
Christoph
diff --git a/eatmydata.c b/eatmydata.c
index ab943c3..4c5692e 100644
--- a/eatmydata.c
+++ b/eatmydata.c
@@ -78,6 +78,10 @@ int eatmydata_is_hungry(void)
int fsync(int fd)
{
if (eatmydata_is_hungry()) {
+ if (fcntl (fd, F_GETFL) == -1) {
+ errno = EBADF;
+ return -1;
+ }
errno= 0;
return 0;
}
@@ -123,6 +127,10 @@ int open(const char* pathname, int flags, ...)
int fdatasync(int fd)
{
if (eatmydata_is_hungry()) {
+ if (fcntl (fd, F_GETFL) == -1) {
+ errno = EBADF;
+ return -1;
+ }
errno= 0;
return 0;
}
-- System Information:
Debian Release: wheezy/sid
APT prefers testing
APT policy: (500, 'testing')
Architecture: amd64 (x86_64)
Kernel: Linux 3.0.27 (SMP w/4 CPU cores)
Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages eatmydata depends on:
ii libc6 2.13-27
eatmydata recommends no packages.
eatmydata suggests no packages.
-- no debconf information
signature.asc
Description: Digital signature
