Jonathan Nieder wrote: > To fix that, we should fsync() each file with multiple hard links only > once and then rename all links, so the logic to skip fsync would be no > longer needed.
... and here's a patch on top implementing that. --- debian/changelog | 13 ++++++------- src/archives.c | 38 ++++++++------------------------------ 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/debian/changelog b/debian/changelog index e5de69d4..24d5df9f 100644 --- a/debian/changelog +++ b/debian/changelog @@ -189,13 +189,12 @@ dpkg (1.16.1~jrn) local; urgency=low [ Jonathan Nieder ] * Defer installing hard links with their final name until the file has been fsynced. - * Open extracted files for reading, not writing, in order to fsync them. - Otherwise the open can error out when preparing to rename a binary into - place that has a hard link already in use. Regression introduced in - 1.15.6.1. Closes: #635683 - * If fsync fails with EBADF, as it is documented to on older platforms - when fd is not opened for writing, reopen the file for writing and try - again. If that fails with ETXTBSY, just skip the fsync. + * Suppress the deferred fsync before renaming hard links to files that + have already been fsynced and linked before. This prevents errors from + opening binaries for writing when another link is already in use. + Regression introduced in 1.15.6.1. Closes: #635683 + * When possible, open extracted files for reading, not writing, in order + to fsync them. [ Updated dpkg translations ] * German (Sven Joachim). Closes: #620312 diff --git a/src/archives.c b/src/archives.c index c221f388..61eee5be 100644 --- a/src/archives.c +++ b/src/archives.c @@ -851,6 +851,13 @@ tarobject(void *ctx, struct tar_entry *ti) * in .dpkg-new. */ + /* + * Files with multiple links only need one fsync: before the first link + * is renamed into place. + */ + if (ti->type == tar_filetype_hardlink) + nifd->namenode->flags &= ~fnnf_deferred_fsync; + if (ti->type == tar_filetype_file || ti->type == tar_filetype_symlink || ti->type == tar_filetype_hardlink) { nifd->namenode->flags |= fnnf_deferred_rename; @@ -920,32 +927,6 @@ deferred_extract_fsync(const char *path) * The documentation for some Unixen (e.g., AIX) says fsync * requires a file descriptor open for writing and will * return EBADF otherwise. - * - * Unfortunately, opening a file for writing fails if that - * file happens to be a binary whose text is mapped. That - * can happen in two cases: - * - * A) We are installing a hard link to a binary that was - * already installed into place and run. In that case, - * we've already done the fsync, so it's safe to skip. - * - * B) Some obnoxious user decided to run the .dpkg-new - * file directly. - * - * For now let's assume we are in case A and skip the fsync - * when errno == ETXTBSY. XXX: It would be better to prevent (A) - * higher in the call stack and fall back to sync() here. - * - * On Linux, we just open the file for reading, fsync does not - * return EBADF, and the behavior is as simple as - * - * fd = open(...); - * if (fd < 0) - * ohshite(...); - * if (fsync(...)) - * ohshite(...); - * if (close(...)) - * ohshite(...); */ int fd; @@ -961,11 +942,8 @@ deferred_extract_fsync(const char *path) /* Try again with an fd open for writing. */ close(fd); fd = open(path, O_WRONLY); - if (fd < 0) { - if (errno == ETXTBSY) - goto done; + if (fd < 0) ohshite(_("unable to open '%.255s'"), path); - } if (fsync(fd)) ohshite(_("unable to sync file '%.255s'"), path); -- 1.7.6 -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org