On Wed, Oct 11 2017, Zhilong Liu wrote:
quoted hunk ↗ jump to hunk
On 10/11/2017 04:16 AM, NeilBrown wrote:
quoted
On Tue, Oct 10 2017, Zhilong Liu wrote:
quoted
On 09/25/2017 01:52 PM, NeilBrown wrote:
quoted
Creating an array by opening a block-device with major number of 9
will transparently load the md module if needed.
Creating an array by opening
/sys/module/md_mod/parameters/new_array
and writing to it won't, it will just fail if md_mod isn't loaded.
So when opening that file fails with ENOENT, run "modprobe md_mod" and
try again.
This fixes a bug whereby if you have "CREATE names=yes" in mdadm.conf,
and the md modules isn't loaded, then creating or assembling an
array will not honor the "names=yes" configuration.
Signed-off-by: NeilBrown <redacted>
---
mdopen.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mdopen.c b/mdopen.c
index 3c0052f2db23..dcdc6f23e6c1 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -312,6 +312,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy,
if (block_udev)
udev_block(devnm);
fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
+ if (fd < 0 && errno == ENOENT) {
+ system("modprobe md_mod");
[nip]
quoted
Hmmm.. that's annoying. I wonder why "system" is marked
"warn_unused_result".
in /usr/include/stdlib.h:
...
712 /* Execute the given line as a shell command.
713
714 This function is a cancellation point and therefore not marked with
715 __THROW. */
716 extern int system (const char *__command) __wur;
...
the "warn_unused_result" is from the __wur parameter, re-compile mdadm
after delete the '__wur',
it works.
quoted
In this case I really don't care - I'm not convinced an extra error
message will really help.
Maybe
if (system("modprobe md_mod") == 0)
fd = open("/sys/......", O_WRONLY);
Agree.
quoted
We do what a better error message, then it should be based on 'fd < 0'.
e.g.
if (fd < 0 || n != strlen(devnm))
pr_err("Fail create array using /sys/module/md_mod/parameters/new_array\n");
you mean something like this?
diff --git a/mdopen.c b/mdopen.c
index dcdc6f2..9de347e 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -313,14 +313,17 @@ int create_mddev(char *dev, char *name, int autof,
int trustworthy,
udev_block(devnm);
fd = open("/sys/module/md_mod/parameters/new_array",
O_WRONLY);
if (fd < 0 && errno == ENOENT) {
- system("modprobe md_mod");
- fd =
open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
+ if (system("modprobe md_mod") == 0)
+ fd =
open("/sys/module/md_mod/parameters/new_array",
+ O_WRONLY);
}
if (fd >= 0) {
n = write(fd, devnm, strlen(devnm));
close(fd);
}
- if (n < 0) {
+ if (fd < 0 || n != strlen(devnm)) {
+ pr_err("Fail create array using "
+ "/sys/module/md_mod/parameters/new_array\n");
devnm[0] = 0;
udev_unblock();
}
Yes - exactly like that except that I wouldn't wrap the long string.
Lines longer than 80 chars are good to avoid, but breaking string
literals is worse than having long lines. e.g. it makes searching for
the string hard.
Thanks,
NeilBrown