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

[Michael Heimpold]

Hi,

I really appreciate working on this, thanks!
I just tried to apply this on top of current master but it fails to compile.
Additionally, a  few comments inline below.

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?

> +		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.

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

> +	} 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?

> +
>  	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?

Thanks,
Michael




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