procd uses a vlist to store instances in. if oe changes all are
restarted. we need to change this as per my last mail to only restart
those that actually changed.
John
On 28/09/2016 15:44, Lebleu Pierre wrote:
> Hi,
>
> Thank you for your answer.
>
> First, I am going to clarify a little bit what I am trying to do.
>
> I would like to have more than one instance for a given daemon (here,
> dnsmasq).
> I would like to stop one instance when I need.
> I would like to start one instance without stopping the other one.
>
> root@LEDE:~# ps w | grep dnsmasq
> 11633 dnsmasq 1560 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.main -k
> -x /var/run/dnsmasq/dnsmasq.main.pid
> 11634 dnsmasq 1556 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.second
> -k -x /var/run/dnsmasq/dnsmasq.second.pid
> 11636 root 1556 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.main -k
> -x /var/run/dnsmasq/dnsmasq.main.pid
> 11660 root 1712 R grep dnsmasq
> root@LEDE:~# /etc/init.d/dnsmasq start main
> root@LEDE:~# ps w | grep dnsmasq
> 11633 dnsmasq 1560 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.main -k
> -x /var/run/dnsmasq/dnsmasq.main.pid
> 11636 root 1556 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.main -k
> -x /var/run/dnsmasq/dnsmasq.main.pid
> 11715 root 1712 R grep dnsmasq
> root@LEDE:~# /etc/init.d/dnsmasq start second
> root@LEDE:~# ps w | grep dnsmasq
> 11753 dnsmasq 1556 S /usr/sbin/dnsmasq -C /var/etc/dnsmasq.conf.second
> -k -x /var/run/dnsmasq/dnsmasq.second.pid
> 11757 root 1712 R grep dnsmasq
>
> As you can see, when I want to start the second instance : it starts it but
> it also kills the first instance...
>
> I had a look at the procd code and it seems it does not flush the other
> instance if we call the method "add". Am I right ?
>
> Regarding the procd.sh file, it seems there is a lack of something. Indeed,
> we can call "stop myinstance" but we can't call "start myinstance".
>
> Here, please find my modification :
>
> diff --git a/package/base-files/files/etc/rc.common
> b/package/base-files/files/etc/rc.common
> index e0de073..8cc6e69 100755
> --- a/package/base-files/files/etc/rc.common
> +++ b/package/base-files/files/etc/rc.common
> @@ -102,9 +102,11 @@ ${INIT_TRACE:+set -x}
> . $IPKG_INSTROOT/lib/functions/procd.sh
> basescript=$(readlink "$initscript")
> rc_procd() {
> + local method="set"
> + [ -n "$2" ] && method="add"
> procd_open_service "$(basename ${basescript:-$initscript})"
> "$initscript"
> "$@"
> - procd_close_service
> + procd_close_service $method
> }
>
> start() {
> diff --git a/package/system/procd/files/procd.sh
> b/package/system/procd/files/procd.sh
> index fa6f8a9..1af9c7b 100644
> --- a/package/system/procd/files/procd.sh
> +++ b/package/system/procd/files/procd.sh
> @@ -72,11 +72,15 @@ _procd_open_service() {
> }
>
> _procd_close_service() {
> + local method="set"
> + [ -n "$1" ] && method="$1"
> +
> json_close_object
> _procd_open_trigger
> service_triggers
> _procd_close_trigger
> - _procd_ubus_call set
> +
> + _procd_ubus_call $method
> }
>
> _procd_add_array_data() {
>
>
> With this patch, it seems to work.
>
> Pierre
> -----Original Message-----
> From: John Crispin [mailto:j...@phrozen.org]
> Sent: dinsdag 27 september 2016 17:48
> To: Lebleu Pierre <pierre.leb...@technicolor.com>;
> lede-dev@lists.infradead.org
> Subject: Re: [LEDE-DEV] The procd daemon does not handle correctly the
> instances
>
>
>
> On 27/09/2016 17:34, Lebleu Pierre wrote:
>> Hi all,
>>
>> I found a bug in the daemon process management. Indeed, when we have several
>> instances of one daemon such as "dnsmasq" and we want to restart only one
>> instance, the other instance is killed by procd. It seems procd updates the
>> second instance and then, kill it.
>>
>> Please find the patch :
>> diff --git a/service/service.c b/service/service.c index
>> 0796adb..9ed07da 100644
>> --- a/service/service.c
>> +++ b/service/service.c
>> @@ -132,8 +132,6 @@ service_update(struct service *s, struct blob_attr **tb,
>> bool add)
>> }
>>
>> if (tb[SERVICE_SET_INSTANCES]) {
>> - if (!add)
>> - vlist_update(&s->instances);
>> blobmsg_for_each_attr(cur, tb[SERVICE_SET_INSTANCES], rem) {
>> service_instance_add(s, cur);
>> }
>
> this bit is certainly not correct. it simply deactivates the feature for all
> services. the actual problem is that procd stores the instances inside a
> vlist. this has an update mechanism triggered inside
> service_instance_update().
>
> what you want to do is compare in_o and in_n and only trigger
> instance_update() if there really was a change. if there is no change then
> simply migrate the pid over to the new instance.
>
>
>> @@ -238,7 +236,7 @@ service_handle_set(struct ubus_context *ctx, struct
>> ubus_object *obj,
>> int ret;
>>
>> blobmsg_parse(service_set_attrs, __SERVICE_SET_MAX, tb,
>> blob_data(msg), blob_len(msg));
>> - cur = tb[SERVICE_ATTR_NAME];
>> + cur = tb[SERVICE_SET_NAME];
>> if (!cur)
>> return UBUS_STATUS_INVALID_ARGUMENT;
>
> this is a copy paste error but luckily does not cause an error as SET and
> ATTR both eval to 0. i have merged this part as a fix.
>
> John
>
>
>>
>>
>> Regards,
>>
>> Pierre
>>
>>
>> _______________________________________________
>> Lede-dev mailing list
>> Lede-dev@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/lede-dev
>>
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
