> 2025年1月22日 02:09，Mariusz Tkaczyk <mtkaczyk@xxxxxxxxxx> 写道：
>
> On Tue, 21 Jan 2025 23:16:03 +0800
> Coly Li <colyli@xxxxxxx> wrote:
>
>> During the boot process if mdadm is called in udev context, sbin paths
>> like /sbin, /usr/sbin, /usr/local/sbin normally not defined in PATH
>> env variable, calling system("modprobe md_mod") in
>> create_named_array() may fail with 'sh: modprobe: command not found'
>> error message.
>>
>> We don't want to move modprobe binary into udev private directory, so
>> setting the PATH env is a more proper method to avoid the above issue.
>>
>> This patch sets PATH env variable with
>> "/sbin:/usr/sbin:/usr/local/sbin" before calling system("modprobe
>> md_mod"). The change only takes effect within the udev worker
>> context, not seen by global udev environment.
>
> Hi Coly,
> Nice explanation, thanks!
>
>>
>> Signed-off-by: Coly Li <colyli@xxxxxxx>
>> ---
>> mdopen.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/mdopen.c b/mdopen.c
>> index 26f0c716..30cf781b 100644
>> --- a/mdopen.c
>> +++ b/mdopen.c
>> @@ -39,6 +39,18 @@ int create_named_array(char *devnm)
>>
>> fd = open(new_array_file, O_WRONLY);
>> if (fd < 0 && errno == ENOENT) {
>> + char buf[PATH_MAX];
>> +
>> + /*
>> + * When called by udev worker context, path of
>> modprobe
>> + * might not be in env PATH. Set sbin paths into PATH
>> + * env to avoid potential failure when run modprobe
>> here.
>> + */
>> + memset(buf, 0, PATH_MAX);
>
> just:
> char buf[PATH_MAX] = {0};
OK, let me change this.
>
>> + snprintf(buf, PATH_MAX - 1, "%s:%s", getenv("PATH"),
>> + "/sbin:/usr/sbin:/usr/local/sbin");
>> + setenv("PATH", buf, 1);
>
> Isn't it over-complicated? Why not simply:
> system("/sbin/modprobe md_mod");
>
> If modprobe is not always in /sbin (checked on my opensuse only)
Yes, it might be in /usr/local/sbin, /usr/sbin/ or somewhere else which I am not
able to take it for granted on other known or unknown distributions.
> we can make in configured during compilation, simple call `which
> modprobe` should do the job.
> What do you think?
Sure, it should work. But might be more complicated, I mean lines of code in total.
Thanks for the review.
Coly Li
