[OpenWrt-Devel] [PATCH fstools RFC] block: generate hotplug.d mount even "add" after mounting

[Rafał Miłecki]

On 30.11.2018 21:01, Michael Heimpold wrote:
> Am Freitag, 30. November 2018, 16:07:40 CET schrieb Rafał Miłecki:
>> From: Rafał Miłecki <rafal at milecki.pl>
>>
>> This is what was implemented in mountd and what some scripts used to
>> use. It's a pretty generic solution for managing software that may use
>> e.g. USB storage.
>>
>> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
>> ---
>> It's just a RFC for now. It's mainly missing a "remove" event support.
>> ---
>>   block.c | 27 +++++++++++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/block.c b/block.c
>> index a356315..20c14fe 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -880,6 +880,31 @@ static int exec_mount(const char *source, const char
>> *target, return err;
>>   }
>>
>> +static void hotplug_call_mount(const char *action, const char *device)
>> +{
>> +	pid_t pid;
>> +
>> +	pid = fork();
>> +	if (!pid) {
>> +		char * const argv[] = { "hotplug-call", "mount", (char *)0 };
> Any special reason not to use NULL here?

Copy & paste. I'll switch to the NULL.


>> +		char actionenv[14];
> 
> "ACTION=remove" would just fit, personally I always align to 4 or 8 byte...
> 
>> +		char deviceenv[32];
>> +		char *envp[] = { actionenv, deviceenv, (char *)0 };
> 
> as above, not NULL?
> 
>> +		snprintf(actionenv, sizeof(actionenv), "ACTION=%s", action);
>> +		snprintf(deviceenv, sizeof(deviceenv), "DEVICE=%s", device);
> 
> maybe we should add the mountpoint, too. so a script could easily look for
> files (e.g. firmware updates) below this entry point without the need to
> find out the mountpoint itself.

I like this idea!


>> +
>> +		execve("/sbin/hotplug-call", argv, envp);
>> +		exit(-1);
> EXIT_FAILURE instead of -1?

Maybe. execve should never really return. I think we use exit(-1) everywhere.


>> +	} else if (pid > 0) {
>> +		int status;
>> +
>> +		waitpid(pid, &status, 0);
>> +		if (WEXITSTATUS(status))
>> +			ULOG_ERR("hotplug-call call failed: %d\n", WEXITSTATUS(status));
>> +	}
>> +}
>> +
>>   static int handle_mount(const char *source, const char *target,
>>                           const char *fstype, struct mount *m)
>>   {
>> @@ -1079,6 +1104,8 @@ static int mount_device(struct probe_info *pr, int
>> type)
>>
>>   	handle_swapfiles(true);
>>
>> +	hotplug_call_mount("add", device);
> "devices" not  available at this point -> compile error?
> 
> Maybe we also should check that the mount actually was successful before
> firing the hotplug event?

Few lines above there is error handling.


>> +
>>   	return 0;
>>   }
> 
> I'm not so familiar with ubus - still on my todo list to investigate deeper:-)
> But I also though about firing an event via ubus?

Not sure about possible use case / gain. Maybe send a patch once basic
hotplug.d events are implemented?


Thanks for a review!

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel