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-dpkg-bugs-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Reply via email to