On Sun, Mar 26, 2017 at 6:45 PM, Alexandru Ardelean
<ardeleana...@gmail.com> wrote:
> On Sun, Mar 26, 2017 at 7:38 PM, Hans Dedecker <dedec...@gmail.com> wrote:
>> On Sun, Mar 26, 2017 at 6:21 PM, Alexandru Ardelean
>> <ardeleana...@gmail.com> wrote:
>>> On Sun, Mar 26, 2017 at 7:06 PM, Hans Dedecker <dedec...@gmail.com> wrote:
>>>> On Mon, Mar 20, 2017 at 3:08 PM, Alexandru Ardelean
>>>> <ardeleana...@gmail.com> wrote:
>>>>> From: Alexandru Ardelean <ardeleana...@gmail.com>
>>>>>
>>>>> The context is that we generate some of the UCI config
>>>>> for netifd via scripts/programs.
>>>>>
>>>>> Every once in a while, there's a goof when doing that
>>>>> UCI generation, and netifd prints out the error at
>>>>> stderr, but returns 0 (success) err-code.
>>>>>
>>>>> This change will fail the ubus call if UCI config
>>>>> is invalid or missing for /etc/config/network.
>>>>>
>>>>> Signed-off-by: Alexandru Ardelean <ardeleana...@gmail.com>
>>>>> ---
>>>>> config.c | 10 ++++++++--
>>>>> config.h | 9 ++++++++-
>>>>> main.c | 4 ++--
>>>>> netifd.h | 2 +-
>>>>> ubus.c | 8 ++++++--
>>>>> 5 files changed, 25 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/config.c b/config.c
>>>>> index 0d965d3..3b1af82 100644
>>>>> --- a/config.c
>>>>> +++ b/config.c
>>>>> @@ -393,16 +393,20 @@ config_init_wireless(void)
>>>>> vlist_flush(&wdev->interfaces);
>>>>> }
>>>>>
>>>>> -void
>>>>> +int
>>>>> config_init_all(void)
>>>>> {
>>>>> + int ret = CONFIG_INIT_OK;
>>>>> +
>>>>> uci_network = config_init_package("network");
>>>>> if (!uci_network) {
>>>>> fprintf(stderr, "Failed to load network config\n");
>>>>> - return;
>>>>> + return CONFIG_INIT_ERR_NO_NETWORK;
>>>>> }
>>>>>
>>>>> uci_wireless = config_init_package("wireless");
>>>>> + if (!uci_wireless)
>>>>> + ret = CONFIG_INIT_ERR_NO_WIRELESS;
>>>>>
>>>>> vlist_update(&interfaces);
>>>>> config_init = true;
>>>>> @@ -426,4 +430,6 @@ config_init_all(void)
>>>>> interface_refresh_assignments(false);
>>>>> interface_start_pending();
>>>>> wireless_start_pending();
>>>>> +
>>>>> + return ret;
>>>>> }
>>>>> diff --git a/config.h b/config.h
>>>>> index 5adaca6..df30b64 100644
>>>>> --- a/config.h
>>>>> +++ b/config.h
>>>>> @@ -19,6 +19,13 @@
>>>>>
>>>>> extern bool config_init;
>>>>>
>>>>> -void config_init_all(void);
>>>>> +enum {
>>>>> + CONFIG_INIT_OK,
>>>>> + CONFIG_INIT_ERR_NO_WIRELESS,
>>>>> + CONFIG_INIT_ERR_NO_NETWORK,
>>>>> + __CONFIG_INIT_LAST
>>>>> +};
>>>>> +
>>>>> +int config_init_all(void);
>>>>>
>>>>> #endif
>>>>> diff --git a/main.c b/main.c
>>>>> index 5717b81..c173cef 100644
>>>>> --- a/main.c
>>>>> +++ b/main.c
>>>>> @@ -208,9 +208,9 @@ static void netifd_do_restart(struct uloop_timeout
>>>>> *timeout)
>>>>> execvp(global_argv[0], global_argv);
>>>>> }
>>>>>
>>>>> -void netifd_reload(void)
>>>>> +int netifd_reload(void)
>>>>> {
>>>>> - config_init_all();
>>>>> + return config_init_all();
>>>>> }
>>>>>
>>>>> void netifd_restart(void)
>>>>> diff --git a/netifd.h b/netifd.h
>>>>> index 5a90858..e565423 100644
>>>>> --- a/netifd.h
>>>>> +++ b/netifd.h
>>>>> @@ -98,6 +98,6 @@ struct interface;
>>>>> extern const char *main_path;
>>>>> extern const char *config_path;
>>>>> void netifd_restart(void);
>>>>> -void netifd_reload(void);
>>>>> +int netifd_reload(void);
>>>>>
>>>>> #endif
>>>>> diff --git a/ubus.c b/ubus.c
>>>>> index 1b1a4cd..31e535c 100644
>>>>> --- a/ubus.c
>>>>> +++ b/ubus.c
>>>>> @@ -44,8 +44,12 @@ netifd_handle_reload(struct ubus_context *ctx, struct
>>>>> ubus_object *obj,
>>>>> struct ubus_request_data *req, const char *method,
>>>>> struct blob_attr *msg)
>>>>> {
>>>>> - netifd_reload();
>>>>> - return 0;
>>>>> + switch (netifd_reload()) {
>>>>> + case CONFIG_INIT_ERR_NO_NETWORK:
>>>>> + return UBUS_STATUS_UNKNOWN_ERROR;
>>>> Is there any reason why only an ubus error code is returned for a
>>>> network uci failure and not for a wireless uci failure ?
>>>>
>>>> Hans
>>>>> + default:
>>>>> + return UBUS_STATUS_OK;
>>>>> + }
>>>>> }
>>>>>
>>>>> enum {
>>>>> --
>>>>> 2.7.4
>>>>>
>>>
>>>
>>> That would make the "ubus call network reload" return non-zero.
>>> Hmm, I guess that would not be a problem.
>>> I interpreted Felix's comment a bit differently regarding not failing
>>> the wireless config.
>> I understood Felix's comment as still load the network config in case
>> a wireless uci error is raised; but I would still return an error
>> code.. This would make the CONFIG_INIT enum definition superfluous and
>> will simplify the code.
>
> ack ; will update
>
> the main thing i would like to get some input on atm [before
> re-spinning], is the base-files change in rc.common [ regarding reload
> & restart ]
> i'm not too confident about it ; and it's a subtle change that can
> have a big impact on processes
OK I will check with Felix
Hans
>
>
> Alex
>
>>
>> Hans
>>>
>>> Alex
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
