You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
162 lines
5.1 KiB
162 lines
5.1 KiB
From 27ad4900501c615b7c6b266bf23948e5606dba53 Mon Sep 17 00:00:00 2001 |
|
From: Logan Gunthorpe <logang@deltatee.com> |
|
Date: Wed, 27 Jul 2022 15:52:46 -0600 |
|
Subject: [PATCH 50/52] mdadm: Don't open md device for CREATE and ASSEMBLE |
|
|
|
The mdadm command tries to open the md device for most modes, first |
|
thing, no matter what. When running to create or assemble an array, |
|
in most cases, the md device will not exist, the open call will fail |
|
and everything will proceed correctly. |
|
|
|
However, when running tests, a create or assembly command may be run |
|
shortly after stopping an array and the old md device file may still |
|
be around. Then, if create_on_open is set in the kernel, a new md |
|
device will be created when mdadm does its initial open. |
|
|
|
When mdadm gets around to creating the new device with the new_array |
|
parameter it issues this error: |
|
|
|
mdadm: Fail to create md0 when using |
|
/sys/module/md_mod/parameters/new_array, fallback to creation via node |
|
|
|
This is because an mddev was already created by the kernel with the |
|
earlier open() call and thus the new one being created will fail with |
|
EEXIST. The mdadm command will still successfully be created due to |
|
falling back to the node creation method. However, the error message |
|
itself will fail any test that's running it. |
|
|
|
This issue is a race condition that is very rare, but a recent change |
|
in the kernel caused this to happen more frequently: about 1 in 50 |
|
times. |
|
|
|
To fix this, don't bother trying to open the md device for CREATE, |
|
ASSEMBLE and BUILD commands, as the file descriptor will never be used |
|
anyway even if it is successfully openned. The mdfd has not been used |
|
for these commands since: |
|
|
|
7f91af49ad09 ("Delay creation of array devices for assemble/build/create") |
|
|
|
The checks that were done on the open device can be changed to being |
|
done with stat. |
|
|
|
Side note: it would be nice to disable create_on_open as well to help |
|
solve this, but it seems the work for this was never finished. By default, |
|
mdadm will create using the old node interface when a name is specified |
|
unless the user specifically puts names=yes in a config file, which |
|
doesn't seem to be common or desirable to require this.. |
|
|
|
Signed-off-by: Logan Gunthorpe <logang@deltatee.com> |
|
Signed-off-by: Jes Sorensen <jsorensen@fb.com> |
|
--- |
|
lib.c | 12 ++++++++++++ |
|
mdadm.c | 40 ++++++++++++++++++++-------------------- |
|
mdadm.h | 1 + |
|
3 files changed, 33 insertions(+), 20 deletions(-) |
|
|
|
diff --git a/lib.c b/lib.c |
|
index 7e3e3d47..e395b28d 100644 |
|
--- a/lib.c |
|
+++ b/lib.c |
|
@@ -164,6 +164,18 @@ char *stat2devnm(struct stat *st) |
|
return devid2devnm(st->st_rdev); |
|
} |
|
|
|
+bool stat_is_md_dev(struct stat *st) |
|
+{ |
|
+ if ((S_IFMT & st->st_mode) != S_IFBLK) |
|
+ return false; |
|
+ if (major(st->st_rdev) == MD_MAJOR) |
|
+ return true; |
|
+ if (major(st->st_rdev) == (unsigned)get_mdp_major()) |
|
+ return true; |
|
+ |
|
+ return false; |
|
+} |
|
+ |
|
char *fd2devnm(int fd) |
|
{ |
|
struct stat stb; |
|
diff --git a/mdadm.c b/mdadm.c |
|
index 845e4466..972adb52 100644 |
|
--- a/mdadm.c |
|
+++ b/mdadm.c |
|
@@ -1329,6 +1329,9 @@ int main(int argc, char *argv[]) |
|
|
|
if (mode == MANAGE || mode == BUILD || mode == CREATE || |
|
mode == GROW || (mode == ASSEMBLE && ! c.scan)) { |
|
+ struct stat stb; |
|
+ int ret; |
|
+ |
|
if (devs_found < 1) { |
|
pr_err("an md device must be given in this mode\n"); |
|
exit(2); |
|
@@ -1341,6 +1344,12 @@ int main(int argc, char *argv[]) |
|
mdfd = open_mddev(devlist->devname, 1); |
|
if (mdfd < 0) |
|
exit(1); |
|
+ |
|
+ ret = fstat(mdfd, &stb); |
|
+ if (ret) { |
|
+ pr_err("fstat failed on %s.\n", devlist->devname); |
|
+ exit(1); |
|
+ } |
|
} else { |
|
char *bname = basename(devlist->devname); |
|
|
|
@@ -1348,30 +1357,21 @@ int main(int argc, char *argv[]) |
|
pr_err("Name %s is too long.\n", devlist->devname); |
|
exit(1); |
|
} |
|
- /* non-existent device is OK */ |
|
- mdfd = open_mddev(devlist->devname, 0); |
|
- } |
|
- if (mdfd == -2) { |
|
- pr_err("device %s exists but is not an md array.\n", devlist->devname); |
|
- exit(1); |
|
- } |
|
- if ((int)ident.super_minor == -2) { |
|
- struct stat stb; |
|
- if (mdfd < 0) { |
|
+ |
|
+ ret = stat(devlist->devname, &stb); |
|
+ if (ident.super_minor == -2 && ret != 0) { |
|
pr_err("--super-minor=dev given, and listed device %s doesn't exist.\n", |
|
- devlist->devname); |
|
+ devlist->devname); |
|
+ exit(1); |
|
+ } |
|
+ |
|
+ if (!ret && !stat_is_md_dev(&stb)) { |
|
+ pr_err("device %s exists but is not an md array.\n", devlist->devname); |
|
exit(1); |
|
} |
|
- fstat(mdfd, &stb); |
|
- ident.super_minor = minor(stb.st_rdev); |
|
- } |
|
- if (mdfd >= 0 && mode != MANAGE && mode != GROW) { |
|
- /* We don't really want this open yet, we just might |
|
- * have wanted to check some things |
|
- */ |
|
- close(mdfd); |
|
- mdfd = -1; |
|
} |
|
+ if (ident.super_minor == -2) |
|
+ ident.super_minor = minor(stb.st_rdev); |
|
} |
|
|
|
if (s.raiddisks) { |
|
diff --git a/mdadm.h b/mdadm.h |
|
index adb7cdaa..8208b81e 100644 |
|
--- a/mdadm.h |
|
+++ b/mdadm.h |
|
@@ -1672,6 +1672,7 @@ void *super1_make_v0(struct supertype *st, struct mdinfo *info, mdp_super_t *sb0 |
|
extern char *stat2kname(struct stat *st); |
|
extern char *fd2kname(int fd); |
|
extern char *stat2devnm(struct stat *st); |
|
+bool stat_is_md_dev(struct stat *st); |
|
extern char *fd2devnm(int fd); |
|
extern void udev_block(char *devnm); |
|
extern void udev_unblock(void); |
|
-- |
|
2.31.1 |
|
|
|
|