* [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. @ 2017-09-25 5:52 NeilBrown 2017-09-25 15:26 ` John Stoffel ` (2 more replies) 0 siblings, 3 replies; 15+ messages in thread From: NeilBrown @ 2017-09-25 5:52 UTC (permalink / raw) To: jes.sorensen; +Cc: Linux Raid [-- Attachment #1: Type: text/plain, Size: 1201 bytes --] 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@suse.com> --- 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"); + fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY); + } if (fd >= 0) { n = write(fd, devnm, strlen(devnm)); close(fd); -- 2.14.0.rc0.dirty [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-25 5:52 [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed NeilBrown @ 2017-09-25 15:26 ` John Stoffel 2017-09-25 23:50 ` NeilBrown 2017-09-27 21:30 ` Jes Sorensen [not found] ` <cba5f77f-d6de-7a6b-35b0-70b7c56eb3f7@suse.com> 2 siblings, 1 reply; 15+ messages in thread From: John Stoffel @ 2017-09-25 15:26 UTC (permalink / raw) To: NeilBrown; +Cc: jes.sorensen, Linux Raid >>>>> "NeilBrown" == NeilBrown <neilb@suse.com> writes: NeilBrown> Creating an array by opening a block-device with major number of 9 NeilBrown> will transparently load the md module if needed. NeilBrown> Creating an array by opening NeilBrown> /sys/module/md_mod/parameters/new_array NeilBrown> and writing to it won't, it will just fail if md_mod isn't loaded. NeilBrown> So when opening that file fails with ENOENT, run "modprobe md_mod" and NeilBrown> try again. NeilBrown> This fixes a bug whereby if you have "CREATE names=yes" in mdadm.conf, NeilBrown> and the md modules isn't loaded, then creating or assembling an NeilBrown> array will not honor the "names=yes" configuration. NeilBrown> Signed-off-by: NeilBrown <neilb@suse.com> NeilBrown> --- NeilBrown> mdopen.c | 4 ++++ NeilBrown> 1 file changed, 4 insertions(+) NeilBrown> diff --git a/mdopen.c b/mdopen.c NeilBrown> index 3c0052f2db23..dcdc6f23e6c1 100644 NeilBrown> --- a/mdopen.c NeilBrown> +++ b/mdopen.c NeilBrown> @@ -312,6 +312,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy, NeilBrown> if (block_udev) NeilBrown> udev_block(devnm); NeilBrown> fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY); NeilBrown> + if (fd < 0 && errno == ENOENT) { NeilBrown> + system("modprobe md_mod"); NeilBrown> + fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY); NeilBrown> + } NeilBrown> if (fd >= 0) { NeilBrown> n = write(fd, devnm, strlen(devnm)); NeilBrown> close(fd); NeilBrown> -- NeilBrown> 2.14.0.rc0.dirty I haven't looked, but shouldn't the path for modprobe be hardcoded here to /sbin/modprobe? Or the PATH sanitized so that random people can't put something into the system PATH and cause problems? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-25 15:26 ` John Stoffel @ 2017-09-25 23:50 ` NeilBrown 2017-09-26 15:11 ` Jes Sorensen 0 siblings, 1 reply; 15+ messages in thread From: NeilBrown @ 2017-09-25 23:50 UTC (permalink / raw) To: John Stoffel; +Cc: jes.sorensen, Linux Raid [-- Attachment #1: Type: text/plain, Size: 2480 bytes --] On Mon, Sep 25 2017, John Stoffel wrote: >>>>>> "NeilBrown" == NeilBrown <neilb@suse.com> writes: > > NeilBrown> Creating an array by opening a block-device with major number of 9 > NeilBrown> will transparently load the md module if needed. > NeilBrown> Creating an array by opening > NeilBrown> /sys/module/md_mod/parameters/new_array > NeilBrown> and writing to it won't, it will just fail if md_mod isn't loaded. > > NeilBrown> So when opening that file fails with ENOENT, run "modprobe md_mod" and > NeilBrown> try again. > > NeilBrown> This fixes a bug whereby if you have "CREATE names=yes" in mdadm.conf, > NeilBrown> and the md modules isn't loaded, then creating or assembling an > NeilBrown> array will not honor the "names=yes" configuration. > > NeilBrown> Signed-off-by: NeilBrown <neilb@suse.com> > NeilBrown> --- > NeilBrown> mdopen.c | 4 ++++ > NeilBrown> 1 file changed, 4 insertions(+) > > NeilBrown> diff --git a/mdopen.c b/mdopen.c > NeilBrown> index 3c0052f2db23..dcdc6f23e6c1 100644 > NeilBrown> --- a/mdopen.c > NeilBrown> +++ b/mdopen.c > NeilBrown> @@ -312,6 +312,10 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy, > NeilBrown> if (block_udev) > NeilBrown> udev_block(devnm); > NeilBrown> fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY); > NeilBrown> + if (fd < 0 && errno == ENOENT) { > NeilBrown> + system("modprobe md_mod"); > NeilBrown> + fd = open("/sys/module/md_mod/parameters/new_array", O_WRONLY); > NeilBrown> + } > NeilBrown> if (fd >= 0) { > NeilBrown> n = write(fd, devnm, strlen(devnm)); > NeilBrown> close(fd); > NeilBrown> -- > NeilBrown> 2.14.0.rc0.dirty > > I haven't looked, but shouldn't the path for modprobe be hardcoded > here to /sbin/modprobe? Or the PATH sanitized so that random people > can't put something into the system PATH and cause problems? That issue briefly crossed my mind as I wrote the code (is it OK to use system()? should I use /sbin/modprobe or just modprobe?) but as mdadm is not set-uid and cannot be run in an environment created by a non-privileged user, there is no security risk. Certainly a careless sysadmin might set path wrongs, but the most likely wrong outcome is that modprobe won't be found, and there is very little cost to that. So thanks for asking, but I don't think there is any need for any extra care, in which case "simplest is best". Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-25 23:50 ` NeilBrown @ 2017-09-26 15:11 ` Jes Sorensen 2017-09-26 19:12 ` Wols Lists 0 siblings, 1 reply; 15+ messages in thread From: Jes Sorensen @ 2017-09-26 15:11 UTC (permalink / raw) To: NeilBrown, John Stoffel; +Cc: Linux Raid On 09/25/2017 07:50 PM, NeilBrown wrote: > On Mon, Sep 25 2017, John Stoffel wrote: > >> I haven't looked, but shouldn't the path for modprobe be hardcoded >> here to /sbin/modprobe? Or the PATH sanitized so that random people >> can't put something into the system PATH and cause problems? > > That issue briefly crossed my mind as I wrote the code (is it OK to use > system()? should I use /sbin/modprobe or just modprobe?) but as mdadm is > not set-uid and cannot be run in an environment created by a > non-privileged user, there is no security risk. > Certainly a careless sysadmin might set path wrongs, but the most likely > wrong outcome is that modprobe won't be found, and there is very little > cost to that. > > So thanks for asking, but I don't think there is any need for any extra > care, in which case "simplest is best". I completely agree, I also think we shouldn't hardcode imposed layouts unless there are very strong reasons for it. Someone may want to put this into an initramfs or somewhere else and want to put it in a different location. Cheers, Jes ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-26 15:11 ` Jes Sorensen @ 2017-09-26 19:12 ` Wols Lists 2017-09-26 20:55 ` John Stoffel 0 siblings, 1 reply; 15+ messages in thread From: Wols Lists @ 2017-09-26 19:12 UTC (permalink / raw) To: Jes Sorensen, NeilBrown, John Stoffel; +Cc: Linux Raid On 26/09/17 16:11, Jes Sorensen wrote: > I completely agree, I also think we shouldn't hardcode imposed layouts > unless there are very strong reasons for it. Someone may want to put > this into an initramfs or somewhere else and want to put it in a > different location. Plus isn't /sbin deprecated? Iirc (very debatable :-) the end goal of the FHS is to have a read-only / with all executables in /bin. Okay it's going to take a looonngg time to get there, but if I'm right hard-coding already-deprecated standards isn't the best move. (Again, iirc :-) sbin stood for *static* binaries needed for system recovery, so its original purpose has long been abused ... :-) Cheers, Wol ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-26 19:12 ` Wols Lists @ 2017-09-26 20:55 ` John Stoffel 0 siblings, 0 replies; 15+ messages in thread From: John Stoffel @ 2017-09-26 20:55 UTC (permalink / raw) To: Wols Lists; +Cc: Jes Sorensen, NeilBrown, John Stoffel, Linux Raid >>>>> "Wols" == Wols Lists <antlists@youngman.org.uk> writes: Wols> On 26/09/17 16:11, Jes Sorensen wrote: >> I completely agree, I also think we shouldn't hardcode imposed layouts >> unless there are very strong reasons for it. Someone may want to put >> this into an initramfs or somewhere else and want to put it in a >> different location. Wols> Plus isn't /sbin deprecated? LOL! Wols> Iirc (very debatable :-) the end goal of the FHS is to have a Wols> read-only / with all executables in /bin. Okay it's going to Wols> take a looonngg time to get there, but if I'm right hard-coding Wols> already-deprecated standards isn't the best move. Heh heh. Wols> (Again, iirc :-) sbin stood for *static* binaries needed for Wols> system recovery, so its original purpose has long been abused Wols> ... :-) I agree. I more think of it as 'system binaries' myself. Stuff that's needed by the barebones system to just even run. But others haven't asuaged my concerns, so I think we're ok. John ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-09-25 5:52 [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed NeilBrown 2017-09-25 15:26 ` John Stoffel @ 2017-09-27 21:30 ` Jes Sorensen [not found] ` <cba5f77f-d6de-7a6b-35b0-70b7c56eb3f7@suse.com> 2 siblings, 0 replies; 15+ messages in thread From: Jes Sorensen @ 2017-09-27 21:30 UTC (permalink / raw) To: NeilBrown; +Cc: Linux Raid On 09/25/2017 01:52 AM, 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@suse.com> > --- > mdopen.c | 4 ++++ > 1 file changed, 4 insertions(+) Applied! Thanks, Jes ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <cba5f77f-d6de-7a6b-35b0-70b7c56eb3f7@suse.com>]
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. [not found] ` <cba5f77f-d6de-7a6b-35b0-70b7c56eb3f7@suse.com> @ 2017-10-10 20:16 ` NeilBrown 2017-10-11 7:39 ` Zhilong Liu 0 siblings, 1 reply; 15+ messages in thread From: NeilBrown @ 2017-10-10 20:16 UTC (permalink / raw) To: Zhilong Liu, jes.sorensen; +Cc: Linux Raid [-- Attachment #1: Type: text/plain, Size: 3342 bytes --] 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@suse.com> >> --- >> 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"); > > Hi, Neil; > this system() line would be treated warning as error when issue > make everything-test. > It complains: > ... ... > mdopen.c: In function ‘create_mddev’: > mdopen.c:316:10: error: ignoring return value of ‘system’, declared with > attribute warn_unused_result [-Werror=unused-result] > system("modprobe md_mod"); > ^ > cc1: all warnings being treated as errors > Makefile:196: recipe for target 'mdadm.O2' failed > make: *** [mdadm.O2] Error 1 > ... ... > > It shows that mdadm cannot assume that "system" will always succeed. The > code becomes > unreliable in this way. > > It should meets three conditions at the same time to ensure system is > successful. > 1. -1 != status > 2. WIFEXITED(status) is true > 3. 0 == WEXITSTATUS(status) > > Maybe add a test like this? > > diff --git a/mdopen.c b/mdopen.c > index dcdc6f2..51bf2d3 100644 > --- a/mdopen.c > +++ b/mdopen.c > @@ -313,7 +313,10 @@ 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"); > + int ret = system("modprobe md_mod"); > + if (ret) { > + pr_err("modprobe md_mod got failed!\n"); > + } > fd = > open("/sys/module/md_mod/parameters/new_array", O_WRONLY); > } > if (fd >= 0) { > Hmmm.. that's annoying. I wonder why "system" is marked "warn_unused_result". 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); 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"); Thanks, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-10 20:16 ` NeilBrown @ 2017-10-11 7:39 ` Zhilong Liu 2017-10-12 0:06 ` NeilBrown 0 siblings, 1 reply; 15+ messages in thread From: Zhilong Liu @ 2017-10-11 7:39 UTC (permalink / raw) To: NeilBrown, jes.sorensen; +Cc: Linux Raid 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@suse.com> >>> --- >>> 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(); } Thanks, -Zhilong > Thanks, > NeilBrown ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-11 7:39 ` Zhilong Liu @ 2017-10-12 0:06 ` NeilBrown 2017-10-12 3:55 ` Zhilong Liu 2017-10-12 9:55 ` Wols Lists 0 siblings, 2 replies; 15+ messages in thread From: NeilBrown @ 2017-10-12 0:06 UTC (permalink / raw) To: Zhilong Liu, jes.sorensen; +Cc: Linux Raid [-- Attachment #1: Type: text/plain, Size: 3802 bytes --] 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@suse.com> >>>> --- >>>> 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, NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-12 0:06 ` NeilBrown @ 2017-10-12 3:55 ` Zhilong Liu 2017-10-12 8:48 ` NeilBrown 2017-10-12 9:55 ` Wols Lists 1 sibling, 1 reply; 15+ messages in thread From: Zhilong Liu @ 2017-10-12 3:55 UTC (permalink / raw) To: NeilBrown, jes.sorensen; +Cc: Linux Raid 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@suse.com> >>>>> --- >>>>> 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 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-12 3:55 ` Zhilong Liu @ 2017-10-12 8:48 ` NeilBrown 2017-10-13 9:16 ` Zhilong Liu 0 siblings, 1 reply; 15+ messages in thread From: NeilBrown @ 2017-10-12 8:48 UTC (permalink / raw) To: Zhilong Liu, jes.sorensen; +Cc: Linux Raid [-- Attachment #1: Type: text/plain, Size: 6829 bytes --] On Thu, Oct 12 2017, Zhilong Liu wrote: > 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@suse.com> >>>>>> --- >>>>>> 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. I wouldn't worry too much about checkpatch.pl. It is worth looking at what it reports, but if you don't agree or the maintainer doesn't agree, then feel free to ignore it. > 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" > + Splitting this out into a separate string does make sense. However I would use static const char new_array_file[] = ....; rather than #define. I hadn't noticed that there are two places where we write to new_array. Maybe that should be split out into a function. Both should use modprobe if the open fails. Thanks, NeilBrown > 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 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-12 8:48 ` NeilBrown @ 2017-10-13 9:16 ` Zhilong Liu 2017-10-15 22:41 ` NeilBrown 0 siblings, 1 reply; 15+ messages in thread From: Zhilong Liu @ 2017-10-13 9:16 UTC (permalink / raw) To: NeilBrown, jes.sorensen; +Cc: Linux Raid On 10/12/2017 04:48 PM, NeilBrown wrote: >> On 10/12/2017 08:06 AM, NeilBrown wrote: > I wouldn't worry too much about checkpatch.pl. > It is worth looking at what it reports, but if you don't agree or the > maintainer doesn't agree, then feel free to ignore it. > > >> 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" >> + > Splitting this out into a separate string does make sense. > However I would use > static const char new_array_file[] = ....; > rather than #define. Maybe "const char *" is enough for this long string? Because this file only used when creating named array, not global using. > I hadn't noticed that there are two places where we write to new_array. > Maybe that should be split out into a function. > Both should use modprobe if the open fails. how about this changing? Thanks for your patience to have a look at it. diff --git a/mdopen.c b/mdopen.c index dcdc6f2..6d8402d 100644 --- a/mdopen.c +++ b/mdopen.c @@ -100,6 +100,29 @@ void make_parts(char *dev, int cnt) free(name); } +int create_named_array(char *devnm) +{ + const char *file = "/sys/module/md_mod/parameters/new_array"; + int fd; + int n = -1; + + fd = open(file, O_WRONLY); + if (fd < 0 && errno == ENOENT) { + if (system("modprobe md_mod") == 0) + fd = open(file, O_WRONLY); + } + if (fd >= 0) { + n = write(fd, devnm, strlen(devnm)); + close(fd); + } + if (fd < 0 || n != (int)strlen(devnm)) { + pr_err("Fail create %s when using %s\n", devnm, file); + return 0; + } + + return 1; +} + /* * We need a new md device to assemble/build/create an array. * 'dev' is a name given us by the user (command line or mdadm.conf) @@ -306,37 +329,19 @@ int create_mddev(char *dev, char *name, int autof, int trustworthy, devnm[0] = 0; if (num < 0 && cname && ci->names) { - int fd; - int n = -1; sprintf(devnm, "md_%s", cname); 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"); - 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 (!create_named_array(devnm)) { devnm[0] = 0; udev_unblock(); } } if (num >= 0) { - int fd; - int n = -1; sprintf(devnm, "md%d", num); if (block_udev) udev_block(devnm); - 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 (!create_named_array(devnm)) { devnm[0] = 0; udev_unblock(); } Thanks, -Zhilong > Thanks, > NeilBrown ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-13 9:16 ` Zhilong Liu @ 2017-10-15 22:41 ` NeilBrown 0 siblings, 0 replies; 15+ messages in thread From: NeilBrown @ 2017-10-15 22:41 UTC (permalink / raw) To: Zhilong Liu, jes.sorensen; +Cc: Linux Raid [-- Attachment #1: Type: text/plain, Size: 4294 bytes --] On Fri, Oct 13 2017, Zhilong Liu wrote: > On 10/12/2017 04:48 PM, NeilBrown wrote: >>> On 10/12/2017 08:06 AM, NeilBrown wrote: >> I wouldn't worry too much about checkpatch.pl. >> It is worth looking at what it reports, but if you don't agree or the >> maintainer doesn't agree, then feel free to ignore it. >> >> >>> 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" >>> + >> Splitting this out into a separate string does make sense. >> However I would use >> static const char new_array_file[] = ....; >> rather than #define. > > Maybe "const char *" is enough for this long string? Because this file > only used when > creating named array, not global using. I would still suggest "const char new_array_file[] = " because you don't need a variable to hold a pointer to the string, you just need the string. But if you want to make it static inside the function, rather then static outside the function, I see no problem with that. NeilBrown > >> I hadn't noticed that there are two places where we write to new_array. >> Maybe that should be split out into a function. >> Both should use modprobe if the open fails. > > how about this changing? Thanks for your patience to have a look at it. > > > diff --git a/mdopen.c b/mdopen.c > index dcdc6f2..6d8402d 100644 > --- a/mdopen.c > +++ b/mdopen.c > @@ -100,6 +100,29 @@ void make_parts(char *dev, int cnt) > free(name); > } > > +int create_named_array(char *devnm) > +{ > + const char *file = "/sys/module/md_mod/parameters/new_array"; > + int fd; > + int n = -1; > + > + fd = open(file, O_WRONLY); > + if (fd < 0 && errno == ENOENT) { > + if (system("modprobe md_mod") == 0) > + fd = open(file, O_WRONLY); > + } > + if (fd >= 0) { > + n = write(fd, devnm, strlen(devnm)); > + close(fd); > + } > + if (fd < 0 || n != (int)strlen(devnm)) { > + pr_err("Fail create %s when using %s\n", devnm, file); > + return 0; > + } > + > + return 1; > +} > + > /* > * We need a new md device to assemble/build/create an array. > * 'dev' is a name given us by the user (command line or mdadm.conf) > @@ -306,37 +329,19 @@ int create_mddev(char *dev, char *name, int autof, > int trustworthy, > > devnm[0] = 0; > if (num < 0 && cname && ci->names) { > - int fd; > - int n = -1; > sprintf(devnm, "md_%s", cname); > 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"); > - 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 (!create_named_array(devnm)) { > devnm[0] = 0; > udev_unblock(); > } > } > if (num >= 0) { > - int fd; > - int n = -1; > sprintf(devnm, "md%d", num); > if (block_udev) > udev_block(devnm); > - 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 (!create_named_array(devnm)) { > devnm[0] = 0; > udev_unblock(); > } > > Thanks, > -Zhilong > >> Thanks, >> NeilBrown [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 832 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed. 2017-10-12 0:06 ` NeilBrown 2017-10-12 3:55 ` Zhilong Liu @ 2017-10-12 9:55 ` Wols Lists 1 sibling, 0 replies; 15+ messages in thread From: Wols Lists @ 2017-10-12 9:55 UTC (permalink / raw) To: NeilBrown, Zhilong Liu, jes.sorensen; +Cc: Linux Raid On 12/10/17 01:06, NeilBrown wrote: > 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. Isn't that the kernel standard anyway? "Wrap at 80 chars *unless* that would mean breaking a string literal". Iirc an lwn story or something about how all subsystems tended to follow this rule except one, so they were going to try and standardise across everything. Cheers, Wol ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-10-15 22:41 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-09-25 5:52 [mdadm PATCH] mdopen: call "modprobe md_mod" if it might be needed NeilBrown 2017-09-25 15:26 ` John Stoffel 2017-09-25 23:50 ` NeilBrown 2017-09-26 15:11 ` Jes Sorensen 2017-09-26 19:12 ` Wols Lists 2017-09-26 20:55 ` John Stoffel 2017-09-27 21:30 ` Jes Sorensen [not found] ` <cba5f77f-d6de-7a6b-35b0-70b7c56eb3f7@suse.com> 2017-10-10 20:16 ` NeilBrown 2017-10-11 7:39 ` Zhilong Liu 2017-10-12 0:06 ` NeilBrown 2017-10-12 3:55 ` Zhilong Liu 2017-10-12 8:48 ` NeilBrown 2017-10-13 9:16 ` Zhilong Liu 2017-10-15 22:41 ` NeilBrown 2017-10-12 9:55 ` Wols Lists
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.