Hi Thanks for the report.
This isn't really an fdm bug per se, the point of maildirs is to allow delivery to happen safely without locking. On AFS you are probably best using mboxes which have flaws but can be locked atomically. Saying that, I don't see a reason not to make this work for people who are prepared to accept the risk or who know for sure they are delivering mail sequentially. With fdm, you will want to set queue-high/queue-low to 1, and perhaps parallel-accounts to 1 if you have multiple accounts delivering to the same maildir. This diff is reasonable, but if possible I'd rather it was implemented as a wrapper function (xlink?) which has the same semantics as link but falls back to stat/rename if it fails with EXDEV. It will also probably need a note in the manual documenting this behaviour. Offhand I don't think there are any other places where fdm does a cross-directory link. Regards Nicholas On Thu, Jul 23, 2009 at 11:50:37PM +0200, Frank Terbeck wrote: > [ Ccing the fdm-users mailing list. Please reply to all addresses in ] > [ To: and Cc: in order to keep all involved parties and the debian ] > [ BTS in the loop. ] > > A new bug report via the debian BTS, which includes a suggestion for a > fix. > > The referenced bug #352589 can be found here: > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=352589 > > Regards, Frank > > Louis Opter <opte...@epitech.net>: > > Package: fdm > > Version: 1.5-3 > > Severity: important > > Tags: patch > > > > Dear maintainer, > > > > link() fails with EXDEV when you try to link files accross file systems. > > > > On the AFS file system, this also occurs between directories : > > http://docs.openafs.org/AdminGuide/ch02.html#HDRWQ32 > > > > This special case breaks fdm when using maildirs on OpenAFS. > > > > I suggest a patch which workaround EXDEV by using stat() and rename(). > > > > I haven't run fdm's regressions tests on it. I have just checked it on > > AFS and ext3. > > > > A similar issue has been fixed in openssh (see #352589). > > > > The same bug certainly exists in other parts of fdm. > > > > Here is the code : > > > --- fdm-1.5.orig/deliver-maildir.c 2008-03-06 10:25:32.000000000 +0100 > +++ fdm-1.5-3.debian.louis/deliver-maildir.c 2009-07-23 21:28:34.418851829 > +0200 > @@ -146,6 +146,7 @@ > char src[MAXPATHLEN], dst[MAXPATHLEN]; > int fd; > ssize_t n; > + struct stat sb; > > name = NULL; > fd = -1; > @@ -195,28 +196,52 @@ > fd = -1; > > /* > - * Create the new path and attempt to link it. A failed link jumps > - * back to find another name in the tmp directory. > + * Create the new path and attempt to link it. A failed link on EEXIST > + * jumps back to find another name in the tmp directory. > */ > if (ppath(dst, sizeof dst, "%s/new/%s", path, name) != 0) > goto error_unlink; > log_debug2( > "%s: linking .../tmp/%s to .../new/%s", a->name, name, name); > if (link(src, dst) != 0) { > - if (errno == EEXIST) { > - log_debug2("%s: %s: link failed", a->name, src); > + log_debug2("%s: %s: link failed", a->name, src); > + /* > + * EXDEV must also be handled, especially for the AFS filesystem > + * where hardlinks can't traverse directories : > + * http://docs.openafs.org/AdminGuide/ch02.html#HDRWQ32 > + */ > + if (errno == EXDEV) { > + log_debug2("%s: renaming .../tmp/%s to .../new/%s", > + a->name, name, name); > + /* > + * But stat + rename can be racy. > + */ > + if (stat(dst, &sb) == -1) { > + if (errno != ENOENT || rename(src, dst) != 0) { > + log_debug2("%s: %s: rename failed", > + a->name, src); > + goto error_cleanup; > + } > + } else { /* EEXIST */ > + if (unlink(src) != 0) > + fatal("unlink failed"); > + cleanup_deregister(src); > + goto restart; > + } > + } else if (errno == EEXIST) { > if (unlink(src) != 0) > fatal("unlink failed"); > cleanup_deregister(src); > goto restart; > - } > - goto error_unlink; > + } else /* Dot it only if we are not on EXDEV */ > + goto error_unlink; > + } else { > + /* Unlink the original tmp file. */ > + log_debug2("%s: unlinking .../tmp/%s", a->name, name); > + if (unlink(src) != 0) > + goto error_unlink; > } > > - /* Unlink the original tmp file. */ > - log_debug2("%s: unlinking .../tmp/%s", a->name, name); > - if (unlink(src) != 0) > - goto error_unlink; > cleanup_deregister(src); > > /* Save the mail file as a tag. */ > @@ -229,6 +254,7 @@ > error_unlink: > if (unlink(src) != 0) > fatal("unlink failed"); > +error_cleanup: > cleanup_deregister(src); > > error_log: > > > ------------------------------------------------------------------------------ > _______________________________________________ > fdm-users mailing list > fdm-us...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/fdm-users -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
