diff options
author | Doug Ledford <dledford@redhat.com> | 2010-07-20 17:42:26 -0400 |
---|---|---|
committer | Doug Ledford <dledford@redhat.com> | 2010-07-22 10:16:31 -0400 |
commit | 56850593049e792c1775047481f814dd444042c5 (patch) | |
tree | b849902282eabc1c8bd3e3284a365452a4a0290d | |
parent | 0155af90d8352d3ca031347e75854b3a5a4052ac (diff) | |
download | mdadm-56850593049e792c1775047481f814dd444042c5.tar.gz mdadm-56850593049e792c1775047481f814dd444042c5.tar.xz mdadm-56850593049e792c1775047481f814dd444042c5.zip |
Bugfix: mapfile locking is broken/racyfor-neil
While we attempt to use a lockfile to grant exclusive access to the
mapfile, our implementation is buggy. Specifically, we create a lockfile,
then lock it, at which point new instances can open the lockfile and
attempt to lock it, which will cause them to block. However, when we are
ready to unlock it, we unlink the file. This causes existing lock waiters
to get a lock on an unlinked inode while a different instance may now
create a new lockfile and get an exclusive lock on it.
There are two possible fixes: don't unlink the lock file or require that
the open of the lockfile include the O_CREAT|O_EXCL flags and on EEXIST
retry the open until we either fail for a different error or actually
create the lock file. I implemented the later on the assumption that the
md device map file and therefore the lockfile are on local, early read
write storage (probably a tmpfs of some sort) and therefore not subject
to the problem that it doesn't work over nfs2 or earlier.
In addition add warnings if we ever fail to get a lock when we request it.
Signed-off-by: Doug Ledford <dledford@redhat.com>
-rw-r--r-- | Incremental.c | 8 | ||||
-rw-r--r-- | mapfile.c | 16 |
2 files changed, 11 insertions, 13 deletions
diff --git a/Incremental.c b/Incremental.c index 35490e7..a8a072d 100644 --- a/Incremental.c +++ b/Incremental.c @@ -294,7 +294,9 @@ int Incremental(char *devname, int verbose, int runstop, /* 4/ Check if array exists. */ - map_lock(&map); + if (map_lock(&map)) + fprintf(stderr, Name ": failed to get exclusive lock on " + "mapfile\n"); mp = map_by_uuid(&map, info.uuid); if (mp) mdfd = open_dev(mp->devnum); @@ -793,7 +795,9 @@ int Incremental_container(struct supertype *st, char *devname, int verbose, struct mdinfo *ra; struct map_ent *map = NULL; - map_lock(&map); + if (map_lock(&map)) + fprintf(stderr, Name ": failed to get exclusive lock on " + "mapfile\n"); for (ra = list ; ra ; ra = ra->next) { int mdfd; @@ -58,7 +58,7 @@ char *mapname[2][3] = { }; char *mapdir[2] = { MAP_DIR, NULL }; -int mapmode[3] = { O_RDONLY, O_RDWR|O_CREAT, O_RDWR|O_CREAT|O_TRUNC }; +int mapmode[3] = { O_RDONLY, O_RDWR|O_CREAT, O_RDWR|O_CREAT|O_EXCL }; char *mapsmode[3] = { "r", "w", "w"}; FILE *open_map(int modenum, int *choice) @@ -120,14 +120,11 @@ static int lwhich = 0; int map_lock(struct map_ent **melp) { if (lf == NULL) { - lf = open_map(2, &lwhich); + do { + lf = open_map(2, &lwhich); + } while (lf == NULL && errno == EEXIST); if (lf == NULL) return -1; - if (flock(fileno(lf), LOCK_EX) != 0) { - fclose(lf); - lf = NULL; - return -1; - } } if (*melp) map_free(*melp); @@ -137,10 +134,7 @@ int map_lock(struct map_ent **melp) void map_unlock(struct map_ent **melp) { - if (lf) { - flock(fileno(lf), LOCK_UN); - fclose(lf); - } + fclose(lf); unlink(mapname[lwhich][2]); lf = NULL; } |