[OpenWrt-Devel] procd: service gets deleted when its last instance is freed

Submitted by Alin Năstac on Feb. 24, 2017, 9:26 a.m.

Details

Message ID 1487928365-10371-1-git-send-email-alin.nastac@gmail.com
State New
Headers show

Commit Message

Alin Năstac Feb. 24, 2017, 9:26 a.m.
Signed-off-by: Alin Nastac <alin.nastac@gmail.com>
---
 service/service.c | 5 ++++-
 service/service.h | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

John Crispin Feb. 24, 2017, 9:53 a.m.
On 24/02/2017 10:26, Alin Nastac wrote:
> Signed-off-by: Alin Nastac <alin.nastac@gmail.com>

Hi,

can you write a little more info as to why this is needed and what
scenario this fixes/changes ?

	John

> ---
>  service/service.c | 5 ++++-
>  service/service.h | 1 +
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/service/service.c b/service/service.c
> index 0584ee0..9675ba2 100644
> --- a/service/service.c
> +++ b/service/service.c
> @@ -140,6 +140,8 @@ service_update(struct service *s, struct blob_attr **tb, bool add)
>  			vlist_flush(&s->instances);
>  	}
>  
> +	s->deleted = false;
> +
>  	rc(s->name, "running");
>  
>  	return 0;
> @@ -149,6 +151,7 @@ static void
>  service_delete(struct service *s)
>  {
>  	vlist_flush_all(&s->instances);
> +	s->deleted = true;
>  	service_stopped(s);
>  }
>  
> @@ -602,7 +605,7 @@ service_start_early(char *name, char *cmdline)
>  
>  void service_stopped(struct service *s)
>  {
> -	if (avl_is_empty(&s->instances.avl)) {
> +	if (s->deleted && avl_is_empty(&s->instances.avl)) {
>  		service_event("service.stop", s->name, NULL);
>  		avl_delete(&services, &s->avl);
>  		trigger_del(s);
> diff --git a/service/service.h b/service/service.h
> index d4f0a83..cc629b1 100644
> --- a/service/service.h
> +++ b/service/service.h
> @@ -40,6 +40,7 @@ struct validate {
>  struct service {
>  	struct avl_node avl;
>  	const char *name;
> +	bool deleted;
>  
>  	struct blob_attr *trigger;
>  	struct vlist_tree instances;
>
Alin Năstac Feb. 24, 2017, 10:21 a.m.
Hi John,

On Fri, Feb 24, 2017 at 10:53 AM, John Crispin <john@phrozen.org> wrote:
> can you write a little more info as to why this is needed and what
> scenario this fixes/changes ?

1) root@OpenWrt:~# uci show system.ntp
system.ntp=timeserver
system.ntp.enable_server='0'
system.ntp.use_dhcp='1'
system.ntp.dhcp_interface='wan'
2) root@OpenWrt:~# uci show network.wan
network.wan=interface
network.wan.proto='dhcp'
network.wan.ifname='eth4'
network.wan.reqopts='1 3 6 15 33 42 51 121 249'
3) restart wan interface using ifup command
4) sysntpd service will be down afterwards although dhcp lease has an option 42

Because service is deleted when last instance gets freed, its triggers
will also released. Without these triggers in place, sysntpd will not
be reloaded when a new dhcp lease containing option 42 will be
received.
John Crispin Feb. 26, 2017, 9:43 a.m.
On 24/02/2017 11:21, Alin Năstac wrote:
> Hi John,
> 
> On Fri, Feb 24, 2017 at 10:53 AM, John Crispin <john@phrozen.org> wrote:
>> can you write a little more info as to why this is needed and what
>> scenario this fixes/changes ?
> 
> 1) root@OpenWrt:~# uci show system.ntp
> system.ntp=timeserver
> system.ntp.enable_server='0'
> system.ntp.use_dhcp='1'
> system.ntp.dhcp_interface='wan'
> 2) root@OpenWrt:~# uci show network.wan
> network.wan=interface
> network.wan.proto='dhcp'
> network.wan.ifname='eth4'
> network.wan.reqopts='1 3 6 15 33 42 51 121 249'
> 3) restart wan interface using ifup command
> 4) sysntpd service will be down afterwards although dhcp lease has an option 42
> 
> Because service is deleted when last instance gets freed, its triggers
> will also released. Without these triggers in place, sysntpd will not
> be reloaded when a new dhcp lease containing option 42 will be
> received.
> 


Hi Alin,

can resend this as a patch with that info added please ?

	John

Patch hide | download patch | download mbox

diff --git a/service/service.c b/service/service.c
index 0584ee0..9675ba2 100644
--- a/service/service.c
+++ b/service/service.c
@@ -140,6 +140,8 @@  service_update(struct service *s, struct blob_attr **tb, bool add)
 			vlist_flush(&s->instances);
 	}
 
+	s->deleted = false;
+
 	rc(s->name, "running");
 
 	return 0;
@@ -149,6 +151,7 @@  static void
 service_delete(struct service *s)
 {
 	vlist_flush_all(&s->instances);
+	s->deleted = true;
 	service_stopped(s);
 }
 
@@ -602,7 +605,7 @@  service_start_early(char *name, char *cmdline)
 
 void service_stopped(struct service *s)
 {
-	if (avl_is_empty(&s->instances.avl)) {
+	if (s->deleted && avl_is_empty(&s->instances.avl)) {
 		service_event("service.stop", s->name, NULL);
 		avl_delete(&services, &s->avl);
 		trigger_del(s);
diff --git a/service/service.h b/service/service.h
index d4f0a83..cc629b1 100644
--- a/service/service.h
+++ b/service/service.h
@@ -40,6 +40,7 @@  struct validate {
 struct service {
 	struct avl_node avl;
 	const char *name;
+	bool deleted;
 
 	struct blob_attr *trigger;
 	struct vlist_tree instances;