Re: [LEDE-DEV] The procd daemon does not handle correctly the instances
In fact, the code already exists for the method "add".
When we call the method "set", it does a vlist_update(), instance_add() and
then vlist_flush().
For the method "add", it only update the PID as you said so.
I just purpose to call the method "add" rather than "set" when we specify the
instance.
Pierre
-Original Message-
From: John Crispin [mailto:j...@phrozen.org]
Sent: donderdag 29 september 2016 7:43
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
this might actually be solvable int he scripts, will need to take a closer look
today
John
On 29/09/2016 07:20, John Crispin wrote:
> 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 Rgrep 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 Rgrep 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 Rgrep 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---
Re: [LEDE-DEV] The procd daemon does not handle correctly the instances
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 Rgrep 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 Rgrep 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 Rgrep 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(>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,
>
[LEDE-DEV] The procd daemon does not handle correctly the instances
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(>instances);
blobmsg_for_each_attr(cur, tb[SERVICE_SET_INSTANCES], rem) {
service_instance_add(s, cur);
}
@@ -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;
Regards,
Pierre
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
Re: [LEDE-DEV] Procd and askconsole
Hi John,
Indeed, I already tried that solution with the default shadow :
--- a/package/base-files/files/etc/shadow
+++ b/package/base-files/files/etc/shadow
@@ -1,4 +1,4 @@
-root::0:0:9:7:::
+root:*:0:0:9:7:::
daemon:*:0:0:9:7:::
ftp:*:0:0:9:7:::
network:*:0:0:9:7:::
Indeed, it works.
Until my custom script is called, nobody is able to login.
But, I think it will be better to have an inittab entry when everything is set
like the old sysvinit.
Pierre
-Original Message-
From: John Crispin [mailto:j...@phrozen.org]
Sent: dinsdag 20 september 2016 9:43
To: Lebleu Pierre <pierre.leb...@technicolor.com>; lede-dev@lists.infradead.org
Subject: Re: [LEDE-DEV] Procd and askconsole
Hi Pierre,
the bahaviour you are observing is actually by design. would it be an option to
ship a default unknown password on your device. that way login simply wont work
until you have set you custom per-device password. you could then do so as the
last step of your customs scripts.
would that be an option ?
John
On 20/09/2016 09:40, Lebleu Pierre wrote:
> Hi John,
>
> Thank you for your answer.
>
> I tried your solution but it seems not to work or it does not do what I want.
> Indeed, the password is one thing but there is also the fact that the system
> is not ready (the scripts are still running).
> As we can see, the hostname is not even set. I would like to allow the
> user login only when everything is ready, because the filesystem is about to
> be modified.
>
> Cheers,
>
>
> Pierre
>
> -Original Message-
> From: John Crispin [mailto:j...@phrozen.org]
> Sent: vrijdag 16 september 2016 11:18
> To: Lebleu Pierre <pierre.leb...@technicolor.com>;
> lede-dev@lists.infradead.org
> Subject: Re: [LEDE-DEV] Procd and askconsole
>
>
>
> On 16/09/2016 10:48, Lebleu Pierre wrote:
>> Hi all,
>>
>> I am new to this mailing list and I would like to present me as Pierre.
>>
>> I recently play a bit with procd and I found an "issue". Indeed, if I
>> do a factory reset, I am able to login as root without login. I have
>> some scripts in /etc/uci-defaults and one of them set the password
>> for the root account. So, this behaviour looks like to me a bug.
>>
>> For my understanding, when procd reaches STATE_INIT, it runs the
>> inittab and one of them is "askconsole". The problem is the system is
>> not completely ready to receive the user : the hostname is not even
>> set.
>>
>> In the old sysvinit, the inittab contains an entry called "bootwait"
>> wich is executed after the termination of init (eg : "/etc/rc.d").
>> I purpose to move the "askconsole" entry to STATE_RUNNING or to
>> create a new entry called "askconsolewait" in order to keep backward
>> compatibility.
>>
>> diff --git a/inittab.c b/inittab.c
>> index ae2c431..2d590e4 100644
>> --- a/inittab.c
>> +++ b/inittab.c
>> @@ -228,6 +228,10 @@ static struct init_handler handlers[] = {
>> .name = "respawn",
>> .cb = rcrespawn,
>> .multi = 1,
>> + }, {
>> + .name = "askconsolewait",
>> + .cb = askconsole,
>> + .multi = 1,
>> }
>> };
>>
>> @@ -251,11 +255,9 @@ void procd_inittab_run(const char *handl
Re: [LEDE-DEV] Procd and askconsole
Hi John,
Thank you for your answer.
I tried your solution but it seems not to work or it does not do what I want.
Indeed, the password is one thing but there is also the fact that the system is
not ready (the scripts are still running).
As we can see, the hostname is not even set. I would like to allow the user
login only when everything is ready, because
the filesystem is about to be modified.
Cheers,
Pierre
-Original Message-
From: John Crispin [mailto:j...@phrozen.org]
Sent: vrijdag 16 september 2016 11:18
To: Lebleu Pierre <pierre.leb...@technicolor.com>; lede-dev@lists.infradead.org
Subject: Re: [LEDE-DEV] Procd and askconsole
On 16/09/2016 10:48, Lebleu Pierre wrote:
> Hi all,
>
> I am new to this mailing list and I would like to present me as Pierre.
>
> I recently play a bit with procd and I found an "issue". Indeed, if I
> do a factory reset, I am able to login as root without login. I have
> some scripts in /etc/uci-defaults and one of them set the password for
> the root account. So, this behaviour looks like to me a bug.
>
> For my understanding, when procd reaches STATE_INIT, it runs the
> inittab and one of them is "askconsole". The problem is the system is
> not completely ready to receive the user : the hostname is not even
> set.
>
> In the old sysvinit, the inittab contains an entry called "bootwait"
> wich is executed after the termination of init (eg : "/etc/rc.d").
> I purpose to move the "askconsole" entry to STATE_RUNNING or to create
> a new entry called "askconsolewait" in order to keep backward
> compatibility.
>
> diff --git a/inittab.c b/inittab.c
> index ae2c431..2d590e4 100644
> --- a/inittab.c
> +++ b/inittab.c
> @@ -228,6 +228,10 @@ static struct init_handler handlers[] = {
> .name = "respawn",
> .cb = rcrespawn,
> .multi = 1,
> + }, {
> + .name = "askconsolewait",
> + .cb = askconsole,
> + .multi = 1,
> }
> };
>
> @@ -251,11 +255,9 @@ void procd_inittab_run(const char *handler)
>
> list_for_each_entry(a, , list)
> if (!strcmp(a->handler->name, handler)) {
> - if (a->handler->multi) {
> - a->handler->cb(a);
> - continue;
> - }
> a->handler->cb(a);
> + if (a->handler->multi)
> + continue;
> break;
> }
> }
> diff --git a/state.c b/state.c
> index 4ad9e2d..fe37419 100644
> --- a/state.c
> +++ b/state.c
> @@ -128,6 +128,7 @@ static void state_enter(void)
>
> case STATE_RUNNING:
> LOG("- init complete -\n");
> + procd_inittab_run("askconsolewait");
> break;
>
> case STATE_SHUTDOWN:
>
> What is your view ? Thank you.
>
> Cheers,
>
> Pierre
>
> ___
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
>
Hi Pierre,
just to be clear, you mean that there is a short timeslot between
inittab/askconsole and uci-defaults during which a passwordless login is
possible and you would liek to prevent this ?
if i understood the problem corretly please simply set
ttylogin=1 here ->
https://git.lede-project.org/?p=source.git;a=blob;f=package/base-files/files/bin/config_generate;h=80ed61b9e2dabf6f2f99102345be3da60097af3e;hb=HEAD#l231
that should make the image boot with password login required even if no
password is set.
the normal use case is that one flashes, enables the flag and then upon second
bootup the unit will require a login. in your use case you already want the
password protection on the very first boot i think
John
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
