fsck: use private lock file rather than whole-disk directly

It seems overkill to lock directly whole-disk device (for -l) when we use the
lock only to synchronize fsck instances.

It's fsck private business, so don't use system files, but let's use private
/run/fsck/<diskname>.lock file.

Addresses: https://bugs.freedesktop.org/show_bug.cgi?id=79576
Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/disk-utils/Makemodule.am b/disk-utils/Makemodule.am
index 302aa68496a36b67ab44c08df21f3c328e4391b7..fb3b564cd63c72a0f6b2a996a467ae32548e6167 100644 (file)
--- a/disk-utils/Makemodule.am
+++ b/disk-utils/Makemodule.am
@@ -81,7 +81,7 @@ if BUILD_FSCK
 sbin_PROGRAMS += fsck
 dist_man_MANS += disk-utils/fsck.8
 fsck_SOURCES = disk-utils/fsck.c
-fsck_LDADD = $(LDADD) libmount.la libblkid.la
+fsck_LDADD = $(LDADD) libmount.la libblkid.la libcommon.la
 fsck_CFLAGS = $(AM_CFLAGS) -I$(ul_libmount_incdir) -I$(ul_libblkid_incdir)
 endif
 

diff --git a/disk-utils/fsck.8 b/disk-utils/fsck.8
index a4a11d7f56474ac26cdae9e76f1aa61a0d44e191..51864d39a5d08152df4b98eee23763a665698929 100644 (file)
--- a/disk-utils/fsck.8
+++ b/disk-utils/fsck.8
@@ -93,8 +93,9 @@ further details.
 .SH OPTIONS
 .TP
 .B \-l
-Lock the whole-disk device by an exclusive
-.BR flock (2).
+Create an exclusive
+.BR flock (2)
+lock file (/run/fsck/<diskname>.lock) for whole-disk device.
 This option can be used with one device only (this means that \fB\-A\fR and
 \fB\-l\fR are mutually exclusive).  This option is recommended when more
 .BR fsck (8)

diff --git a/disk-utils/fsck.c b/disk-utils/fsck.c
index 7c7043ac16c8807b8f36e246f1a8edded6cdd549..de5fb5b7c7d09fc65aebfc0326e8707ce154706f 100644 (file)
--- a/disk-utils/fsck.c
+++ b/disk-utils/fsck.c
@@ -53,6 +53,7 @@
 #include "exitcodes.h"
 #include "c.h"
 #include "closestream.h"
+#include "fileutils.h"
 
 #define XALLOC_EXIT_CODE       FSCK_EX_ERROR
 #include "xalloc.h"
@@ -64,6 +65,8 @@
 #define MAX_DEVICES 32
 #define MAX_ARGS 32
 
+#define FSCK_RUNTIME_DIRNAME   "/run/fsck"
+
 static const char *ignored_types[] = {
        "ignore",
        "iso9660",
@@ -100,7 +103,10 @@ struct fsck_fs_data
 struct fsck_instance {
        int     pid;
        int     flags;          /* FLAG_{DONE|PROGRESS} */
-       int     lock;           /* flock()ed whole disk file descriptor or -1 */
+
+       int     lock;           /* flock()ed lockpath file descriptor or -1 */
+       char    *lockpath;      /* /run/fsck/<diskname>.lock or NULL */
+
        int     exit_status;
        struct timeval start_time;
        struct timeval end_time;
@@ -327,19 +333,40 @@ static int is_irrotational_disk(dev_t disk)
 static void lock_disk(struct fsck_instance *inst)
 {
        dev_t disk = fs_get_disk(inst->fs, 1);
-       char *diskname;
+       char *diskpath = NULL, *diskname;
+
+       inst->lock = -1;
 
        if (!disk || is_irrotational_disk(disk))
-               return;
+               goto done;
+
+       diskpath = blkid_devno_to_devname(disk);
+       if (!diskpath)
+               goto done;
+
+       if (access(FSCK_RUNTIME_DIRNAME, F_OK) != 0) {
+               int rc = mkdir(FSCK_RUNTIME_DIRNAME,
+                                   S_IWUSR|
+                                   S_IRUSR|S_IRGRP|S_IROTH|
+                                   S_IXUSR|S_IXGRP|S_IXOTH);
+               if (rc && errno != EEXIST) {
+                       warn(_("cannot create directory %s"),
+                                       FSCK_RUNTIME_DIRNAME);
+                       goto done;
+               }
+       }
 
-       diskname = blkid_devno_to_devname(disk);
+       diskname = stripoff_last_component(diskpath);
        if (!diskname)
-               return;
+               diskname = diskpath;
+
+       xasprintf(&inst->lockpath, FSCK_RUNTIME_DIRNAME "/%s.lock", diskname);
 
        if (verbose)
-               printf(_("Locking disk %s ... "), diskname);
+               printf(_("Locking disk by %s ... "), inst->lockpath);
 
-       inst->lock = open(diskname, O_CLOEXEC | O_RDONLY);
+       inst->lock = open(inst->lockpath, O_RDONLY|O_CREAT|O_CLOEXEC,
+                                   S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH);
        if (inst->lock >= 0) {
                int rc = -1;
 
@@ -359,21 +386,31 @@ static void lock_disk(struct fsck_instance *inst)
                /* TRANSLATORS: These are followups to "Locking disk...". */
                printf("%s.\n", inst->lock >= 0 ? _("succeeded") : _("failed"));
 
-       free(diskname);
+
+done:
+       if (inst->lock < 0) {
+               free(inst->lockpath);
+               inst->lockpath = NULL;
+       }
+       free(diskpath);
        return;
 }
 
 static void unlock_disk(struct fsck_instance *inst)
 {
-       if (inst->lock >= 0) {
-               /* explicitly unlock, don't rely on close(), maybe some library
-                * (e.g. liblkid) has still open the device.
-                */
-               flock(inst->lock, LOCK_UN);
-               close(inst->lock);
+       if (inst->lock < 0)
+               return;
 
-               inst->lock = -1;
-       }
+       if (verbose)
+               printf(_("Unlocking %s.\n"), inst->lockpath);
+
+       close(inst->lock);                      /* unlock */
+       unlink(inst->lockpath);
+
+       free(inst->lockpath);
+
+       inst->lock = -1;
+       inst->lockpath = NULL;
 }
 
 static void free_instance(struct fsck_instance *i)
@@ -381,6 +418,7 @@ static void free_instance(struct fsck_instance *i)
        if (lockdisk)
                unlock_disk(i);
        free(i->prog);
+       free(i->lockpath);
        mnt_unref_fs(i->fs);
        free(i);
        return;