[PATCH v2] inotifyd: -x: new option
Add -x option which allows to specify the exit status of PROG for which
inotifyd should exit.
An example use case for this change is writing parallel system startup
scripts with busybox' runit: inotifyd can be used to wait for a specific
pid-file to appear in /var/run and then exit, allowing the blocked script
to proceed.
function old new delta
inotifyd_main653 742 +89
.rodata 157197 157261 +64
packed_usage 30460 30516 +56
--
(add/remove: 0/0 grow/shrink: 3/0 up/down: 209/0) Total: 209 bytes
Signed-off-by: Bartosz Golaszewski
---
v2:
- don't call xstrtol_range() if -x is not specified
v1:
http://lists.busybox.net/pipermail/busybox/2015-August/083219.html
miscutils/inotifyd.c | 26 --
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/miscutils/inotifyd.c b/miscutils/inotifyd.c
index 7a1a6a2..e142668 100644
--- a/miscutils/inotifyd.c
+++ b/miscutils/inotifyd.c
@@ -28,7 +28,7 @@
*/
//usage:#define inotifyd_trivial_usage
-//usage: "PROG FILE1[:MASK]..."
+//usage: "[OPTS] PROG FILE1[:MASK]..."
//usage:#define inotifyd_full_usage "\n\n"
//usage: "Run PROG on filesystem changes."
//usage: "\nWhen a filesystem event matching MASK occurs on FILEn,"
@@ -52,12 +52,17 @@
//usage: "\n n Subfile is created"
//usage: "\n d Subfile is deleted"
//usage: "\n"
+//usage: "\nOptions:"
+//usage: "\n -x STATUS Exit if PROG returns STATUS"
+//usage: "\n"
//usage: "\ninotifyd waits for PROG to exit."
//usage: "\nWhen x event happens for all FILEs, inotifyd exits."
#include "libbb.h"
#include
+#define OPT_x (1 << 0)
+
static const char mask_names[] ALIGN1 =
"a" // 0x0001 File was accessed
"c" // 0x0002 File was modified
@@ -84,17 +89,21 @@ enum {
int inotifyd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
int inotifyd_main(int argc, char **argv)
{
- int n;
- unsigned mask;
+ int n, exp_st, st;
+ unsigned mask, opts;
struct pollfd pfd;
char **watches; // names of files being watched
const char *args[5];
+ char *opt_x_str;
+
+ opts = getopt32(argv, "x:", &opt_x_str);
+ argc -= optind;
+ argv += optind;
// sanity check: agent and at least one watch must be given
- if (!argv[1] || !argv[2])
+ if (!argv[0] || !argv[1])
bb_show_usage();
- argv++;
// inotify_add_watch will number watched files
// starting from 1, thus watches[0] is unimportant,
// and 1st file name is watches[1].
@@ -190,7 +199,12 @@ int inotifyd_main(int argc, char **argv)
args[1] = events;
args[2] = watches[ie->wd];
args[3] = ie->len ? ie->name : NULL;
- spawn_and_wait((char **)args);
+ st = spawn_and_wait((char **)args);
+ if (opts & OPT_x) {
+ exp_st =
xstrtol_range(opt_x_str, 10, 0, 255);
+ if (st == exp_st)
+ goto done;
+ }
}
// we are done if all files got final x event
if (ie->mask & 0x8000) {
--
2.1.4
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
2015-08-25 15:39 GMT+02:00 Bartosz Golaszewski :
> Add -x option which allows to specify the exit status of PROG for which
> inotifyd should exit.
>
> An example use case for this change is writing parallel system startup
> scripts with busybox' runit: inotifyd can be used to wait for a specific
> pid-file to appear in /var/run and then exit, allowing the blocked script
> to proceed.
Is there any chance of merging this? I suppose inotifyd in busybox is
loosely based on inotifywait from inotify-tools which has an option to
exit upon receiving an event. It's also much cleaner then killing
inotifyd from spawned processes.
--
Best regards,
Bartosz Golaszewski
> function old new delta
> inotifyd_main653 742 +89
> .rodata 157197 157261 +64
> packed_usage 30460 30516 +56
> --
> (add/remove: 0/0 grow/shrink: 3/0 up/down: 209/0) Total: 209 bytes
>
> Signed-off-by: Bartosz Golaszewski
> ---
> v2:
> - don't call xstrtol_range() if -x is not specified
>
> v1:
> http://lists.busybox.net/pipermail/busybox/2015-August/083219.html
>
> miscutils/inotifyd.c | 26 --
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/miscutils/inotifyd.c b/miscutils/inotifyd.c
> index 7a1a6a2..e142668 100644
> --- a/miscutils/inotifyd.c
> +++ b/miscutils/inotifyd.c
> @@ -28,7 +28,7 @@
> */
>
> //usage:#define inotifyd_trivial_usage
> -//usage: "PROG FILE1[:MASK]..."
> +//usage: "[OPTS] PROG FILE1[:MASK]..."
> //usage:#define inotifyd_full_usage "\n\n"
> //usage: "Run PROG on filesystem changes."
> //usage: "\nWhen a filesystem event matching MASK occurs on FILEn,"
> @@ -52,12 +52,17 @@
> //usage: "\n n Subfile is created"
> //usage: "\n d Subfile is deleted"
> //usage: "\n"
> +//usage: "\nOptions:"
> +//usage: "\n -x STATUS Exit if PROG returns STATUS"
> +//usage: "\n"
> //usage: "\ninotifyd waits for PROG to exit."
> //usage: "\nWhen x event happens for all FILEs, inotifyd exits."
>
> #include "libbb.h"
> #include
>
> +#define OPT_x (1 << 0)
> +
> static const char mask_names[] ALIGN1 =
> "a" // 0x0001 File was accessed
> "c" // 0x0002 File was modified
> @@ -84,17 +89,21 @@ enum {
> int inotifyd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> int inotifyd_main(int argc, char **argv)
> {
> - int n;
> - unsigned mask;
> + int n, exp_st, st;
> + unsigned mask, opts;
> struct pollfd pfd;
> char **watches; // names of files being watched
> const char *args[5];
> + char *opt_x_str;
> +
> + opts = getopt32(argv, "x:", &opt_x_str);
> + argc -= optind;
> + argv += optind;
>
> // sanity check: agent and at least one watch must be given
> - if (!argv[1] || !argv[2])
> + if (!argv[0] || !argv[1])
> bb_show_usage();
>
> - argv++;
> // inotify_add_watch will number watched files
> // starting from 1, thus watches[0] is unimportant,
> // and 1st file name is watches[1].
> @@ -190,7 +199,12 @@ int inotifyd_main(int argc, char **argv)
> args[1] = events;
> args[2] = watches[ie->wd];
> args[3] = ie->len ? ie->name : NULL;
> - spawn_and_wait((char **)args);
> + st = spawn_and_wait((char **)args);
> + if (opts & OPT_x) {
> + exp_st =
> xstrtol_range(opt_x_str, 10, 0, 255);
> + if (st == exp_st)
> + goto done;
> + }
> }
> // we are done if all files got final x event
> if (ie->mask & 0x8000) {
> --
> 2.1.4
>
___
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On Tue, Sep 15, 2015 at 12:21 PM, Bartosz Gołaszewski wrote: > 2015-08-25 15:39 GMT+02:00 Bartosz Golaszewski : >> Add -x option which allows to specify the exit status of PROG for which >> inotifyd should exit. >> >> An example use case for this change is writing parallel system startup >> scripts with busybox' runit: inotifyd can be used to wait for a specific >> pid-file to appear in /var/run and then exit, allowing the blocked script >> to proceed. > > Is there any chance of merging this? I suppose inotifyd in busybox is > loosely based on inotifywait from inotify-tools which has an option to > exit upon receiving an event. No, I was not aware inotifywait exists. > It's also much cleaner then killing inotifyd from spawned processes. I think killing $PPID from children is a completely legitimate approach. Are you sure you aren't just feeling attachment to the work you did? This happens to me too. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On 16/10/2015 20:29, Denys Vlasenko wrote: I think killing $PPID from children is a completely legitimate approach. FWIW, I dislike parricide. The flow of information is backwards; it forces the child to know in what context it is run. I agree with Bartosz in that it is much cleaner to encode information in the child's return code, and have the parent explicitly support a code that makes itexit. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On Fri, Oct 16, 2015 at 10:49 PM, Laurent Bercot wrote: > On 16/10/2015 20:29, Denys Vlasenko wrote: >> >> I think killing $PPID from children is a completely legitimate approach. > > > FWIW, I dislike parricide. The flow of information is backwards; it > forces the child to know in what context it is run. If you have a program which does not know that it is being run by inotifyd, you can create a shim which does know that: #!/bin/sh PROG "$@" test "$?" = 111 && kill $PPID > I agree with Bartosz in that it is much cleaner to encode information > in the child's return code, and have the parent explicitly support a > code that makes itexit. This does not sound like technical argument, it's an aesthetic one. You propose to add complexity to inotifyd because it's looks "cleaner". I don't see it as "cleaner", I see both ways as legitimate. But your way requires additional code. ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On 17/10/2015 09:47, Denys Vlasenko wrote: If you have a program which does not know that it is being run by inotifyd, you can create a shim which does know that: #!/bin/sh PROG "$@" test "$?" = 111 && kill $PPID Yes. You still have to know which program needs it and which one does not, and you have to run that shim for every program that does. This is code - a longer code path than Bartosz's patch, and that has to be maintained separately. This does not sound like technical argument, it's an aesthetic one. Indeed, but aesthetics usually translate to very tangible, technical benefits: either shorter code paths, or better maintainability. Here, it translates to both. You propose to add complexity to inotifyd because it's looks "cleaner". I don't see it as "cleaner", I see both ways as legitimate. But your way requires additional code. It requires additional code in inotifyd, yes. But it factorizes code that you would have had to run anyway, in scripting the child. It also allows you to make the natural assumption that a child cannot impact its parent (except by returning a certain exit code), which I find makes scripts easier to understand, so more maintainable. It is not intuitive, when reading the script, to think "ah, inotifyd can be killed by its child" - the mechanism works, and is legitimate as you put it, but the information flow is more convoluted and requires more effort to understand than just following the exit codes trail. -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On Sat, Oct 17, 2015 at 12:38 PM, Laurent Bercot wrote: > On 17/10/2015 09:47, Denys Vlasenko wrote: >> >> If you have a program which does not know that it is being run by >> inotifyd, >> you can create a shim which does know that: >> >> #!/bin/sh >> PROG "$@" >> test "$?" = 111 && kill $PPID > > > Yes. You still have to know which program needs it and which one does > not, and you have to run that shim for every program that does. This > is code - a longer code path than Bartosz's patch, and that has to be > maintained separately. inotifyd syntax is "inotifyd PROG FILE1[:MASK]...", and PROG is run with PROG ACTUAL_EVENTS FILEn [SUBFILE] This severely limits the number of unmodified PROGs you can run to achieve some useful result. For example, even cat'ting newly created files to stdout can't be done by PROG=cat, you need to massage argv's IOW: in practice, you always need a shim. >> You propose to add complexity to inotifyd because it's looks "cleaner". >> I don't see it as "cleaner", I see both ways as legitimate. >> But your way requires additional code. > > It requires additional code in inotifyd, yes. But it factorizes code > that you would have had to run anyway, in scripting the child. It also > allows you to make the natural assumption that a child cannot impact > its parent (except by returning a certain exit code), which I find > makes scripts easier to understand, so more maintainable. Sorry, but I don't agree to this view even after you repeat it. Here is an example where this idiom of "killing the parent" is used already: busybox/examples/var_service/ntpd/ntp.script ... if test x"$1" = x"unsync"; then # No replies for our NTP requests were seen for some time. # # Among more mundate cases like network outages, this happens # if we ran for a LONG time (days) and ntp server's IP has changed. # ntpd has no code to re-resolve peers' addresses to IPs, # we need to help it: # echo "$dt: $1"\ "syncronization lost, restarting ntpd"\ >>"$0.log.$$" mv -- "$0.log.$$" "$0.log" kill $PPID exit fi ... Can you point me to the software you are working on so that I can look on a wider picture of the problem you are trying to solve? ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On 20/10/2015 13:17, Denys Vlasenko wrote: inotifyd syntax is "inotifyd PROG FILE1[:MASK]...", and PROG is run with PROG ACTUAL_EVENTS FILEn [SUBFILE] This severely limits the number of unmodified PROGs you can run to achieve some useful result. (...) IOW: in practice, you always need a shim. Ah, indeed. It's a shame PROG is a single argument, though: I very much prefer executing whole command lines. If you have to tailor PROG to inotifyd's needs anyway, the "it's less code" argument falls. Sorry, but I don't agree to this view even after you repeat it. Here is an example where this idiom of "killing the parent" is used already: Well, giving me another example of this idiom doesn't convince me that it's a good one either. :P ntpd, just like inotifyd, can be killed by one of its children? I still don't like it. I'm a Unix traditionalist and reactionary: parents should have control over their children, not the other way round. Can you point me to the software you are working on so that I can look on a wider picture of the problem you are trying to solve? That would be Bartosz. I'm not working on any software using inotifyd, I was just adding my support to his proposal from a pure "Unix software design" point of view. But for me it's theoretical, and borderline bikeshedding, and I can work with your design even if it's not my preferred one - so if you won't be convinced, that's ok too, and I'll just drop it. :) -- Laurent ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
2015-10-20 13:17 GMT+02:00 Denys Vlasenko : > > inotifyd syntax is "inotifyd PROG FILE1[:MASK]...", > and PROG is run with > > PROG ACTUAL_EVENTS FILEn [SUBFILE] > > This severely limits the number of unmodified PROGs you can run > to achieve some useful result. For example, even cat'ting newly > created files to stdout can't be done by PROG=cat, you need > to massage argv's > > IOW: in practice, you always need a shim. I agree that the functionality of inotifyd is limited. I tried working with what's available. Would you be willing to accept patches that expand this applet? Like for example being able to redirect the events to stdout instead of only being able to pass them as arguments to PROG? This is how inotifywait works BTW. Other than that: I'm late to the party - I don't have anything more to add than what Laurent already said. I don't like the idea of a child killing its parents - I prefer strict hierarchy. Best regards, Bartosz Golaszewski ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On Wed, Oct 21, 2015 at 11:56 AM, Bartosz Gołaszewski wrote: > 2015-10-20 13:17 GMT+02:00 Denys Vlasenko : >> >> inotifyd syntax is "inotifyd PROG FILE1[:MASK]...", >> and PROG is run with >> >> PROG ACTUAL_EVENTS FILEn [SUBFILE] >> >> This severely limits the number of unmodified PROGs you can run >> to achieve some useful result. For example, even cat'ting newly >> created files to stdout can't be done by PROG=cat, you need >> to massage argv's >> >> IOW: in practice, you always need a shim. > > I agree that the functionality of inotifyd is limited. I tried working > with what's available. > > Would you be willing to accept patches that expand this applet? Yes. > Like > for example being able to redirect the events to stdout instead of > only being able to pass them as arguments to PROG? This is how > inotifywait works BTW. It already does that: BusyBox v1.25.0.git (2015-10-19 04:25:25 CEST) multi-call binary. Usage: inotifyd PROG FILE1[:MASK]... Run PROG on filesystem changes. When a filesystem event matching MASK occurs on FILEn, PROG ACTUAL_EVENTS FILEn [SUBFILE] is run. If PROG is -, events are sent to stdout. ^ # inotifyd - /etc:r r/etc r/etcld.so.cache r/etcld.so.cache r/etcld.so.cache r/etcld.so.cache r/etcld.so.cache r/etcmagic r/etcbashrc r/etcbashrc ... ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
2015-10-22 16:46 GMT+02:00 Denys Vlasenko : > On Wed, Oct 21, 2015 at 11:56 AM, Bartosz Gołaszewski > wrote: >> 2015-10-20 13:17 GMT+02:00 Denys Vlasenko : >>> >>> inotifyd syntax is "inotifyd PROG FILE1[:MASK]...", >>> and PROG is run with >>> >>> PROG ACTUAL_EVENTS FILEn [SUBFILE] >>> >>> This severely limits the number of unmodified PROGs you can run >>> to achieve some useful result. For example, even cat'ting newly >>> created files to stdout can't be done by PROG=cat, you need >>> to massage argv's >>> >>> IOW: in practice, you always need a shim. >> >> I agree that the functionality of inotifyd is limited. I tried working >> with what's available. >> >> Would you be willing to accept patches that expand this applet? > > Yes. > >> Like >> for example being able to redirect the events to stdout instead of >> only being able to pass them as arguments to PROG? This is how >> inotifywait works BTW. > > It already does that: > > BusyBox v1.25.0.git (2015-10-19 04:25:25 CEST) multi-call binary. > > Usage: inotifyd PROG FILE1[:MASK]... > > Run PROG on filesystem changes. > When a filesystem event matching MASK occurs on FILEn, > PROG ACTUAL_EVENTS FILEn [SUBFILE] is run. > If PROG is -, events are sent to stdout. > ^ > > # inotifyd - /etc:r > r/etc > r/etcld.so.cache > r/etcld.so.cache > r/etcld.so.cache > r/etcld.so.cache > r/etcld.so.cache > r/etcmagic > r/etcbashrc > r/etcbashrc > ... Indeed, I forgot. Just to clarify - is it a definite no to the patch? -- Best regards, Bartosz Golaszewski ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
On Fri, Oct 23, 2015 at 3:14 PM, Bartosz Gołaszewski wrote: >>> Like >>> for example being able to redirect the events to stdout instead of >>> only being able to pass them as arguments to PROG? This is how >>> inotifywait works BTW. >> >> It already does that: >> >> BusyBox v1.25.0.git (2015-10-19 04:25:25 CEST) multi-call binary. >> >> Usage: inotifyd PROG FILE1[:MASK]... >> >> Run PROG on filesystem changes. >> When a filesystem event matching MASK occurs on FILEn, >> PROG ACTUAL_EVENTS FILEn [SUBFILE] is run. >> If PROG is -, events are sent to stdout. >> ^ >> >> # inotifyd - /etc:r >> r/etc >> r/etcld.so.cache >> r/etcld.so.cache >> r/etcld.so.cache >> r/etcld.so.cache >> r/etcld.so.cache >> r/etcmagic >> r/etcbashrc >> r/etcbashrc >> ... > > Indeed, I forgot. > > Just to clarify - is it a definite no to the patch? I want to help you by looking at actual setup you want to speed up. By seeing your problem, I might be convinced that your solution is indeed necessary ;) ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
Re: [PATCH v2] inotifyd: -x: new option
2015-10-24 4:26 GMT+02:00 Denys Vlasenko : > On Fri, Oct 23, 2015 at 3:14 PM, Bartosz Gołaszewski > wrote: Like for example being able to redirect the events to stdout instead of only being able to pass them as arguments to PROG? This is how inotifywait works BTW. >>> >>> It already does that: >>> >>> BusyBox v1.25.0.git (2015-10-19 04:25:25 CEST) multi-call binary. >>> >>> Usage: inotifyd PROG FILE1[:MASK]... >>> >>> Run PROG on filesystem changes. >>> When a filesystem event matching MASK occurs on FILEn, >>> PROG ACTUAL_EVENTS FILEn [SUBFILE] is run. >>> If PROG is -, events are sent to stdout. >>> ^ >>> >>> # inotifyd - /etc:r >>> r/etc >>> r/etcld.so.cache >>> r/etcld.so.cache >>> r/etcld.so.cache >>> r/etcld.so.cache >>> r/etcld.so.cache >>> r/etcmagic >>> r/etcbashrc >>> r/etcbashrc >>> ... >> >> Indeed, I forgot. >> >> Just to clarify - is it a definite no to the patch? > > I want to help you by looking at actual setup you want to speed up. This is not speeding anything up - it's rather to have an elegant process startup. I start several processes that need xserver to be running (fail if the xserver socket is not present). The start-up scripts start inotifyd to wait for the socket to appear and then continue the execution immediately. Otherwise I'd have to loop the process execution until they don't fail to start. > By seeing your problem, I might be convinced that your solution > is indeed necessary ;) I don't like the idea of a shim script killing inotifyd - its parent - that's all. It would be nice to have this functionality if it doesn't increase the default size (it's n by default in Kconfig). Additionally if you would have the shim script drop privileges for any reason (I know it's unlikely ;)) it would fail to signal inotifyd. -- Best regards, Bartosz Golaszewski ___ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox
