Re: [OpenWrt-Devel] [PATCH] base-files: procd initscriptsrestartonreload
> You forgot to call reload after changing /tmp/test :) > FYI, I just tested your script, and it works as expected - after > changing the file and calling reload, the pid is different. I see, I took the wording "reload on config file change" to mean that prcod used inotify and reloaded at the time of the actual change. (which isn't a feature I'm very interested in.) Thanks. Everything makes much more sense now. Time to go change some patches. Tristan -- All original matter is hereby placed immediately under the public domain. ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] base-files: procd initscriptsrestartonreload
On 2014-09-16 14:18, tris...@trstn.net wrote:
> Felix Fietkau wrote:
>> ...
>> By the way, you mentioned in an earlier email that procd_set_param file
>> did not work for you to restart your service when a config file changes.
>> Can you produce a test case for that? I just tested it myself on a
>> simple service and it worked just fine.
>
> So I just made a test service:
>
> root@hg:~# cat /etc/openwrt_version
> r42538
> root@hg:~# cat /etc/init.d/test
> #!/bin/sh /etc/rc.common
>
> START=70
> USE_PROCD=1
>
> start_service() {
> procd_open_instance
> procd_set_param command /bin/sleep 1
> procd_set_param file /tmp/test
> procd_set_param respawn
> procd_close_instance
> }
>
> root@hg:~# cat /tmp/test
> hello
> root@hg:~# /etc/init.d/test start
> root@hg:~# pidof sleep
> 6958
> root@hg:~# /etc/init.d/test reload
> root@hg:~# pidof sleep
> 6958
> root@hg:~# echo goodbye > /tmp/test
> root@hg:~# pidof sleep
> 6958
> root@hg:~# rm /tmp/test
> root@hg:~# pidof sleep
> 6958
> root@hg:~# echo hello > /tmp/test
> root@hg:~# pidof sleep
> 6958
> root@hg:~# /etc/init.d/test restart
> root@hg:~# pidof sleep
> 6985
You forgot to call reload after changing /tmp/test :)
FYI, I just tested your script, and it works as expected - after
changing the file and calling reload, the pid is different.
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] base-files: procd initscriptsrestartonreload
Felix Fietkau wrote:
> ...
> By the way, you mentioned in an earlier email that procd_set_param file
> did not work for you to restart your service when a config file changes.
> Can you produce a test case for that? I just tested it myself on a
> simple service and it worked just fine.
So I just made a test service:
root@hg:~# cat /etc/openwrt_version
r42538
root@hg:~# cat /etc/init.d/test
#!/bin/sh /etc/rc.common
START=70
USE_PROCD=1
start_service() {
procd_open_instance
procd_set_param command /bin/sleep 1
procd_set_param file /tmp/test
procd_set_param respawn
procd_close_instance
}
root@hg:~# cat /tmp/test
hello
root@hg:~# /etc/init.d/test start
root@hg:~# pidof sleep
6958
root@hg:~# /etc/init.d/test reload
root@hg:~# pidof sleep
6958
root@hg:~# echo goodbye > /tmp/test
root@hg:~# pidof sleep
6958
root@hg:~# rm /tmp/test
root@hg:~# pidof sleep
6958
root@hg:~# echo hello > /tmp/test
root@hg:~# pidof sleep
6958
root@hg:~# /etc/init.d/test restart
root@hg:~# pidof sleep
6985
Which more or less bears out what I was seeing with dnsmasq.
Note, that adding:
reload_service() {
restart
}
didn't change anything, except that reload did restart the service.
> For procd based services, the assumption should be that all information
> is there to decide whether a service should be restarted on reload.
> If a service does not get enough information to handle reload properly,
> that's a bug which should not be worked around by using an unconditional
> restart on reload.
I suppose it is reasonable to make any of the reload methods the default,
and assume that the service will override the default if needed. I just
found the current default to be surprising, as it doesn't match other
handlers' defaults.
> > Knowing that the service manages this itself nothing need happen.
> >
> > And there are mixtures of the four, some services manage data one way and
> > connections another. It might be useful to have one name for data and
> > another for connections, but that's not what we've inherited, merely
> > reload means to reload the configuration.
> What do you mean by "connections"?
"Connections" is probably too narrow a word, control is maybe better?
The properties of how the service interacts with the outside world. As
in, dnsmasq will reload hosts and ethers on SIGHUP, but will quickly
notice any change in it's upstream resolv.conf, yet won't change what
ports it listens to or the how it allocates addresses without a restart.
The general idea though is that reloading different parts of a services
configuration may use different means. I think there is some history of
using reload for one purpose and restart for another, but that's not born
out by procd's default either.
Tristan
--
All original matter is hereby placed immediately under the public domain.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] base-files: procd initscriptsrestartonreload
On 2014-09-15 11:42, Tristan Plumb wrote: > >> On 15/09/2014 10:36, Tristan Plumb wrote: >> which specific package is causing issue ? >> >>> In my setup, I've noticed this with dnsmasq and babeld. That is, >> >>> that I needed to restart and instead of reload to get things to >> >>> take effect. Everything else I run is configured by command line >> >>> arguments. >> >> what do you want to do ? track a file and if changed trigger a reload ? >> > Nope, I want /etc/init.d/foo reload to work as expected. > >> vagueness == no fix > >> if you wont this fixed you need to be more elaborate than "as expected" > > The current behavior with a procd service is that unless the service has > declared its own routine /etc/init.d/foo reload will restart the service > if the command line changes, but otherwise take no action to alert the > service of new configuration files. The expected behavior is that reload > will reload changed configuration by whatever means necessary, including > restarting the service. > > Which is what /etc/init.d/foo help claims it does. > > > Lacking information about how the service is configured, it must > restart. For procd based services, the assumption should be that all information is there to decide whether a service should be restarted on reload. If a service does not get enough information to handle reload properly, that's a bug which should not be worked around by using an unconditional restart on reload. > Knowing the configuration files, it must restart if they, the command > line arguments, or the enviroment have changed, but not otherwise. > > Knowing that the service takes SIGHUP to reread configuration files, it > must restart if the command line arguments, or environment, have changed, > but only be sent a signal otherwise. That is currently not implemented within procd, but should be easy to add. > Knowing that the service manages this itself nothing need happen. > > And there are mixtures of the four, some services manage data one way and > connections another. It might be useful to have one name for data and > another for connections, but that's not what we've inherited, merely > reload means to reload the configuration. What do you mean by "connections"? By the way, you mentioned in an earlier email that procd_set_param file did not work for you to restart your service when a config file changes. Can you produce a test case for that? I just tested it myself on a simple service and it worked just fine. - Felix ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
Re: [OpenWrt-Devel] [PATCH] base-files: procd initscriptsrestartonreload
> On 15/09/2014 10:36, Tristan Plumb wrote: > which specific package is causing issue ? > >>> In my setup, I've noticed this with dnsmasq and babeld. That is, > >>> that I needed to restart and instead of reload to get things to > >>> take effect. Everything else I run is configured by command line > >>> arguments. > >> what do you want to do ? track a file and if changed trigger a reload ? > > Nope, I want /etc/init.d/foo reload to work as expected. > vagueness == no fix > if you wont this fixed you need to be more elaborate than "as expected" The current behavior with a procd service is that unless the service has declared its own routine /etc/init.d/foo reload will restart the service if the command line changes, but otherwise take no action to alert the service of new configuration files. The expected behavior is that reload will reload changed configuration by whatever means necessary, including restarting the service. Which is what /etc/init.d/foo help claims it does. Lacking information about how the service is configured, it must restart. Knowing the configuration files, it must restart if they, the command line arguments, or the enviroment have changed, but not otherwise. Knowing that the service takes SIGHUP to reread configuration files, it must restart if the command line arguments, or environment, have changed, but only be sent a signal otherwise. Knowing that the service manages this itself nothing need happen. And there are mixtures of the four, some services manage data one way and connections another. It might be useful to have one name for data and another for connections, but that's not what we've inherited, merely reload means to reload the configuration. Does that sound about right? Tristan -- All original matter is hereby placed immediately under the public domain. ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
