On 10/12/2017 08:06 AM, NeilBrown wrote:
On Wed, Oct 11 2017, Zhilong Liu wrote:
On 10/11/2017 04:16 AM, NeilBrown wrote:
On Tue, Oct 10 2017, Zhilong Liu wrote:
On 09/25/2017 01:52 PM, NeilBrown wrote:
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 <neilb@xxxxxxxx>
---
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]
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.
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.
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 for your detail explanation. I draft it like this, and
already checked it via to ./linux/scripts/checkpatch.pl.
Is this changing ok to you?
diff --git a/mdopen.c b/mdopen.c
index dcdc6f2..da8d9d1 100644
--- a/mdopen.c
+++ b/mdopen.c
@@ -26,6 +26,8 @@
#include "md_p.h"
#include <ctype.h>
+#define NEW_ARRAY_FILE "/sys/module/md_mod/parameters/new_array"
+
void make_parts(char *dev, int cnt)
{
/* make 'cnt' partition devices for 'dev'
@@ -311,16 +313,17 @@ int create_mddev(char *dev, char *name, int autof,
int trustworthy,
sprintf(devnm, "md_%s", cname);
if (block_udev)
udev_block(devnm);
- fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
+ fd = open(NEW_ARRAY_FILE, 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(NEW_ARRAY_FILE, 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 %s\n", NEW_ARRAY_FILE);
devnm[0] = 0;
udev_unblock();
}
@@ -331,12 +334,13 @@ int create_mddev(char *dev, char *name, int autof,
int trustworthy,
sprintf(devnm, "md%d", num);
if (block_udev)
udev_block(devnm);
- fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY);
+ fd = open(NEW_ARRAY_FILE, 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 %s\n", NEW_ARRAY_FILE);
devnm[0] = 0;
udev_unblock();
}
--
2.6.6
Thanks,
-Zhilong
Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
