| Message ID | 20181206123123.31081-1-zajec5@gmail.com |
|---|---|
| State | Rejected |
| Delegated to: | Rafał Miłecki |
| Headers | show |
| Series | [OpenWrt-Devel,libubox] hotplug: add hotplug_call() helper | expand |
On Thu, 6 Dec 2018 at 20:42, Rafał Miłecki <zajec5@gmail.com> wrote: > > From: Rafał Miłecki <rafal@milecki.pl> > > This new function imlements common code needed to run /etc/hotplug.d/ > listeners. It allows specifying subsystem and environment strings as > commonly used in the hotplug.d. hotplug-call is currently a OpenWrt-specific shell script from base-files. A library function's dependence on exact name and path of non-standard executable does not seem fit. Hotplug events from the kernel is already being handled by hotplug.json script. Events from non-standard subsystem like "iface" as defined by netifd are generated and handled fine just by itself [1]. As such, I cannot find enough other places needing this function to make it into libubox. [1] https://git.openwrt.org/?p=project/netifd.git;a=blob;f=interface-event.c;h=a40f6dc883d3a3654d73d0592cd7eb65c2a8de85;hb=HEAD#l45 yousong
On 2018-12-07 04:41, Yousong Zhou wrote: > On Thu, 6 Dec 2018 at 20:42, Rafał Miłecki <zajec5@gmail.com> wrote: >> >> From: Rafał Miłecki <rafal@milecki.pl> >> >> This new function imlements common code needed to run /etc/hotplug.d/ >> listeners. It allows specifying subsystem and environment strings as >> commonly used in the hotplug.d. > > hotplug-call is currently a OpenWrt-specific shell script from > base-files. A library function's dependence on exact name and path of > non-standard executable does not seem fit. You're probably right, I guess the best solution would be to implement hotplug-call script behavior directly in the libubox library. > Hotplug events from the kernel is already being handled by > hotplug.json script. Events from non-standard subsystem like "iface" > as defined by netifd are generated and handled fine just by itself > [1]. As such, I cannot find enough other places needing this function > to make it into libubox. We could make netifd use that hotplug_call() helper, but now I noticed that netifd simply uses setenv() after the fork(). That may be simpler than building all that envp array. I'm not sure now if it's better to work on: 1) Improved (native) hotplug_call() implementation in the libubox 2) Original idea of doing fork() in the fstools / block + setenv() Implementing hotplug_call() was originally suggested by John, let's see what's his opinion too. > [1] > https://git.openwrt.org/?p=project/netifd.git;a=blob;f=interface-event.c;h=a40f6dc883d3a3654d73d0592cd7eb65c2a8de85;hb=HEAD#l45
On 07/12/2018 09:39, Rafał Miłecki wrote: > On 2018-12-07 04:41, Yousong Zhou wrote: >> On Thu, 6 Dec 2018 at 20:42, Rafał Miłecki <zajec5@gmail.com> wrote: >>> >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> This new function imlements common code needed to run /etc/hotplug.d/ >>> listeners. It allows specifying subsystem and environment strings as >>> commonly used in the hotplug.d. >> >> hotplug-call is currently a OpenWrt-specific shell script from >> base-files. A library function's dependence on exact name and path of >> non-standard executable does not seem fit. > > You're probably right, I guess the best solution would be to implement > hotplug-call script behavior directly in the libubox library. > > >> Hotplug events from the kernel is already being handled by >> hotplug.json script. Events from non-standard subsystem like "iface" >> as defined by netifd are generated and handled fine just by itself >> [1]. As such, I cannot find enough other places needing this function >> to make it into libubox. > > We could make netifd use that hotplug_call() helper, but now I noticed > that netifd simply uses setenv() after the fork(). That may be simpler > than building all that envp array. > > > I'm not sure now if it's better to work on: > 1) Improved (native) hotplug_call() implementation in the libubox > 2) Original idea of doing fork() in the fstools / block + setenv() > > Implementing hotplug_call() was originally suggested by John, let's see > what's his opinion too. I was simply thinking of making the code reusable. i do understand the concens. I dont really hold a really strong position on this. if you feel that it is better to implment this inside blockd directly or any other way, feel free to do so. John > >> [1] >> https://git.openwrt.org/?p=project/netifd.git;a=blob;f=interface-event.c;h=a40f6dc883d3a3654d73d0592cd7eb65c2a8de85;hb=HEAD#l45 >> > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
diff --git a/CMakeLists.txt b/CMakeLists.txt index 57804cf..aa54499 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -15,7 +15,7 @@ IF(JSONC_FOUND) INCLUDE_DIRECTORIES(${JSONC_INCLUDE_DIRS}) ENDIF() -SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c base64.c) +SET(SOURCES avl.c avl-cmp.c blob.c blobmsg.c uloop.c usock.c ustream.c ustream-fd.c vlist.c utils.c safe_list.c runqueue.c md5.c kvlist.c ulog.c base64.c hotplug.c) ADD_LIBRARY(ubox SHARED ${SOURCES}) ADD_LIBRARY(ubox-static STATIC ${SOURCES}) diff --git a/hotplug.c b/hotplug.c new file mode 100644 index 0000000..f3298fa --- /dev/null +++ b/hotplug.c @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: ISC +/* + * hotplug - helpers for hotplug.d events + * + * Copyright (C) 2018 Rafał Miłecki <rafal@milecki.pl> + */ + +#include "hotplug.h" + +#include <errno.h> +#include <stdlib.h> +#include <sys/wait.h> +#include <unistd.h> + +int hotplug_call(char *subsystem, char * const *envp) +{ + pid_t pid; + + pid = fork(); + if (!pid) { + char * const argv[] = { "hotplug-call", subsystem, NULL }; + + execve("/sbin/hotplug-call", argv, envp); + exit(-1); + } else if (pid > 0) { + int status; + + waitpid(pid, &status, 0); + if (WEXITSTATUS(status)) + return WEXITSTATUS(status); + return 0; + } else { + return -errno; + } +} diff --git a/hotplug.h b/hotplug.h new file mode 100644 index 0000000..224a764 --- /dev/null +++ b/hotplug.h @@ -0,0 +1,8 @@ +// SPDX-License-Identifier: ISC + +#ifndef __LIBUBOX_HOTPLUG_H +#define __LIBUBOX_HOTPLUG_H + +int hotplug_call(char *subsystem, char * const *envp); + +#endif