Send connman mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.01.org/mailman/listinfo/connman
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."


Today's Topics:

   1. Re: [PATCH] stats: add "StatsDir" configuration to define the
      directory of stats file. (Daniel Wagner)
   2. Re: Details about the Patch (Patrik Flykt)
   3. Re: [PATCH] stats: add "StatsDir" configuration to define the
      directory of stats file. (Patrik Flykt)
   4. Re: [PATCH] stats: add "StatsDir" configuration to define the
      directory of stats file. (wangfe-nestlabs)


----------------------------------------------------------------------

Message: 1
Date: Thu, 2 Jun 2016 07:56:43 +0200
From: Daniel Wagner <[email protected]>
To: Feng Wang <[email protected]>, [email protected]
Subject: Re: [PATCH] stats: add "StatsDir" configuration to define the
        directory of stats file.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252

On 06/02/2016 01:51 AM, Feng Wang wrote:
> user can put stats files in volatile memory(RAM) and STORAGEDIR in
> non-volatile memory(FLASH) to prevent the flash wear-out because
> of too many writes. If "StatsDir" is not defined, the default value
> is STORAGEDIR.

Another option would be to disable the stats files completely (maybe at
compile time).

I think it doesn't really make sense to store those files in a RAM disk,
because they are thought to store persistent network statistic usage.
With your change you would only be able to retrieve the numbers between
restarts of ConnMan. If you restart the whole system, the numbers are
gone, which might be what you want.

Furthermore, the mmap code in stats.c doesn't really work well with some
filesystems (JJF2). Maybe it is just time to be able to disable the
persistent statistic numbers at compile.

cheers,
daniel


------------------------------

Message: 2
Date: Thu, 02 Jun 2016 10:43:57 +0300
From: Patrik Flykt <[email protected]>
To: "Natarajan, Ponniah (P.)" <[email protected]>,
        "'[email protected]'" <[email protected]>
Subject: Re: Details about the Patch
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"

On Wed, 2016-06-01 at 14:49 +0000, Natarajan, Ponniah (P.) wrote:
> Hi Patrik,
> ?
> connman 1.31 is used

Please use 1.32, it contains a lot of bug fixes. None related to
technology.c, so it's not affecting this issue, though.

>  in our platform, Sometimes there is no powered status reported on
> powerup. But the connman settings file for WIFI Enable flag shows
> "true", whereas when run the connmanctl "technologies" which returns
> the powered status as "False"..
> It looks like there is no sync between the connman settings and the
> technologies powered status notification.
> ?
> /var/lib/connman# cat settings
> [global]
> OfflineMode=false
> ?
> [WiFi]
> Enable=true
> Tethering=false
> ?
> [Gadget]
> Enable=false
> Tethering=false

As this file is under /var/lib/, it is the property of ConnMan. ConnMan
won't read it again after starting and will only write to the file when
ConnMan shuts down or something changes. The latter I cannot remember
if it happens or not, please read the source code.

> connmanctl>technologies
> /net/connman/technology/wifi
> ? Name = WiFi
> ? Type = wifi
> ? Powered = False
> ? Connected = False
> ? Tethering = False

This is the correct run-time information. The contents of the file
/var/lib/connman/settings corresponds to the initial setup how things
was supposed to be at startup.

> Please correct me whether my understandings are wrong..
> ?
> Note: This issues occurs very sporadic.

Logs of at least -d src/technology.c. Impossible to say what is going
on otherwise.


Cheers,

        Patrik


------------------------------

Message: 3
Date: Thu, 02 Jun 2016 10:57:50 +0300
From: Patrik Flykt <[email protected]>
To: Feng Wang <[email protected]>, [email protected]
Subject: Re: [PATCH] stats: add "StatsDir" configuration to define the
        directory of stats file.
Message-ID: <[email protected]>
Content-Type: text/plain; charset="UTF-8"

On Wed, 2016-06-01 at 16:51 -0700, Feng Wang wrote:
> user can put stats files in volatile memory(RAM) and STORAGEDIR in
> non-volatile memory(FLASH) to prevent the flash wear-out because
> of too many writes. If "StatsDir" is not defined, the default value
> is STORAGEDIR.
> ---
> ?include/setting.h |??1 +
> ?src/main.c????????| 22 ++++++++++++++++++++++
> ?src/stats.c???????|??9 +++++----
> ?3 files changed, 28 insertions(+), 4 deletions(-)

This patch is too complicated for the task. Either define a statsdir at
compile time (automake --localstatedir= comes to mind), change
storagedir in Makefile.am or make /var/lib/connman a symlink to a ram
disk and remember to save its contents on shutdown.

For ConnMan run time the solution between a compile time change or a
system symlink are all equivalent solutions as the system below is not
expected to change between starting and stopping of ConnMan. And of
these both compile time options already exists, and the system can be
tweaked to suit ones needs.

The above also takes care of writing the settings file too many times.


        Patrik


> diff --git a/include/setting.h b/include/setting.h
> index a882021..f492373 100644
> --- a/include/setting.h
> +++ b/include/setting.h
> @@ -31,6 +31,7 @@ extern "C" {
> ?bool connman_setting_get_bool(const char *key);
> ?char **connman_setting_get_string_list(const char *key);
> ?unsigned int *connman_setting_get_uint_list(const char *key);
> +char *connman_setting_get_string(const char *key);
> ?
> ?unsigned int connman_timeout_input_request(void);
> ?unsigned int connman_timeout_browser_launch(void);
> diff --git a/src/main.c b/src/main.c
> index f44a2ed..62e3837 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -76,6 +76,7 @@ static struct {
> ?     char **tethering_technologies;
> ?     bool persistent_tethering_mode;
> ?     bool enable_6to4;
> +     char *statsdir;
> ?} connman_settings??= {
> ?     .bg_scan = true,
> ?     .pref_timeservers = NULL,
> @@ -90,6 +91,7 @@ static struct {
> ?     .tethering_technologies = NULL,
> ?     .persistent_tethering_mode = false,
> ?     .enable_6to4 = false,
> +     .statsdir = NULL,
> ?};
> ?
> ?#define CONF_BG_SCAN????????????????????"BackgroundScanning"
> @@ -105,6 +107,7 @@ static struct {
> ?#define CONF_TETHERING_TECHNOLOGIES??????"TetheringTechnologies"
> ?#define CONF_PERSISTENT_TETHERING_MODE??"PersistentTetheringMode"
> ?#define CONF_ENABLE_6TO4????????????????"Enable6to4"
> +#define CONF_STATS_DIR??????????????????"StatsDir"
> ?
> ?static const char *supported_options[] = {
> ?     CONF_BG_SCAN,
> @@ -120,6 +123,7 @@ static const char *supported_options[] = {
> ?     CONF_TETHERING_TECHNOLOGIES,
> ?     CONF_PERSISTENT_TETHERING_MODE,
> ?     CONF_ENABLE_6TO4,
> +     CONF_STATS_DIR,
> ?     NULL
> ?};
> ?
> @@ -244,6 +248,7 @@ static void parse_config(GKeyFile *config)
> ?     char **tethering;
> ?     gsize len;
> ?     int timeout;
> +     char *statsdir;
> ?
> ?     if (!config) {
> ?             connman_settings.auto_connect =
> @@ -367,6 +372,15 @@ static void parse_config(GKeyFile *config)
> ?             connman_settings.enable_6to4 = boolean;
> ?
> ?     g_clear_error(&error);
> +
> +     statsdir = __connman_config_get_string(config, "General",
> +                                     CONF_STATS_DIR, &error);
> +     if (!error)
> +             connman_settings.statsdir = statsdir;
> +     else
> +             connman_settings.statsdir = g_strdup(STORAGEDIR);
> +
> +     g_clear_error(&error);
> ?}
> ?
> ?static int config_init(const char *file)
> @@ -564,6 +578,14 @@ char **connman_setting_get_string_list(const
> char *key)
> ?     return NULL;
> ?}
> ?
> +char *connman_setting_get_string(const char *key)
> +{
> +     if (g_str_equal(key, CONF_STATS_DIR))
> +             return connman_settings.statsdir;
> +
> +     return NULL;
> +}
> +
> ?unsigned int *connman_setting_get_uint_list(const char *key)
> ?{
> ?     if (g_str_equal(key, CONF_AUTO_CONNECT))
> diff --git a/src/stats.c b/src/stats.c
> index 26343b1..3958e17 100644
> --- a/src/stats.c
> +++ b/src/stats.c
> @@ -350,7 +350,7 @@ static int stats_open(struct stats_file *file,
> ?static int stats_open_temp(struct stats_file *file)
> ?{
> ?     file->name = g_strdup_printf("%s/stats.XXXXXX.tmp",
> -                                     STORAGEDIR);
> +                                     connman_setting_get_string("
> StatsDir"));
> ?     file->fd = g_mkstemp_full(file->name, O_RDWR | O_CREAT,
> 0644);
> ?     if (file->fd < 0) {
> ?             connman_error("create tempory file error %s for %s",
> @@ -687,7 +687,7 @@ int __connman_stats_service_register(struct
> connman_service *service)
> ?             return -EALREADY;
> ?     }
> ?
> -     dir = g_strdup_printf("%s/%s", STORAGEDIR,
> +     dir = g_strdup_printf("%s/%s",
> connman_setting_get_string("StatsDir"),
> ?                             __connman_service_get_ident(service)
> );
> ?
> ?     /* If the dir doesn't exist, create it */
> @@ -704,9 +704,10 @@ int __connman_stats_service_register(struct
> connman_service *service)
> ?
> ?     g_free(dir);
> ?
> -     name = g_strdup_printf("%s/%s/data", STORAGEDIR,
> +     name = g_strdup_printf("%s/%s/data",
> connman_setting_get_string("StatsDir"),
> ?                             __connman_service_get_ident(service)
> );
> -     file->history_name = g_strdup_printf("%s/%s/history",
> STORAGEDIR,
> +     file->history_name = g_strdup_printf("%s/%s/history",
> +                             connman_setting_get_string("StatsDir
> "),
> ?                             __connman_service_get_ident(service)
> );
> ?
> ?     /* TODO: Use a global config file instead of hard coded
> value. */


------------------------------

Message: 4
Date: Thu, 2 Jun 2016 09:35:48 -0700
From: wangfe-nestlabs <[email protected]>
To: Patrik Flykt <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH] stats: add "StatsDir" configuration to define the
        directory of stats file.
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii

The StatsDir option allows stats file in the volatile memory but STORAGEDIR in 
the non-volatile memory.
If put everything in the RAM and save them in the non-volatile memory when 
conman shutdown, it is not very reliable.
Because the device can crash before saving the content.

Feng


> On Jun 2, 2016, at 12:57 AM, Patrik Flykt <[email protected]> 
> wrote:
> 
> On Wed, 2016-06-01 at 16:51 -0700, Feng Wang wrote:
>> user can put stats files in volatile memory(RAM) and STORAGEDIR in
>> non-volatile memory(FLASH) to prevent the flash wear-out because
>> of too many writes. If "StatsDir" is not defined, the default value
>> is STORAGEDIR.
>> ---
>>  include/setting.h |  1 +
>>  src/main.c        | 22 ++++++++++++++++++++++
>>  src/stats.c       |  9 +++++----
>>  3 files changed, 28 insertions(+), 4 deletions(-)
> 
> This patch is too complicated for the task. Either define a statsdir at
> compile time (automake --localstatedir= comes to mind), change
> storagedir in Makefile.am or make /var/lib/connman a symlink to a ram
> disk and remember to save its contents on shutdown.
> 
> For ConnMan run time the solution between a compile time change or a
> system symlink are all equivalent solutions as the system below is not
> expected to change between starting and stopping of ConnMan. And of
> these both compile time options already exists, and the system can be
> tweaked to suit ones needs.
> 
> The above also takes care of writing the settings file too many times.
> 
> 
>       Patrik
> 
> 
>> diff --git a/include/setting.h b/include/setting.h
>> index a882021..f492373 100644
>> --- a/include/setting.h
>> +++ b/include/setting.h
>> @@ -31,6 +31,7 @@ extern "C" {
>>  bool connman_setting_get_bool(const char *key);
>>  char **connman_setting_get_string_list(const char *key);
>>  unsigned int *connman_setting_get_uint_list(const char *key);
>> +char *connman_setting_get_string(const char *key);
>>  
>>  unsigned int connman_timeout_input_request(void);
>>  unsigned int connman_timeout_browser_launch(void);
>> diff --git a/src/main.c b/src/main.c
>> index f44a2ed..62e3837 100644
>> --- a/src/main.c
>> +++ b/src/main.c
>> @@ -76,6 +76,7 @@ static struct {
>>      char **tethering_technologies;
>>      bool persistent_tethering_mode;
>>      bool enable_6to4;
>> +    char *statsdir;
>>  } connman_settings  = {
>>      .bg_scan = true,
>>      .pref_timeservers = NULL,
>> @@ -90,6 +91,7 @@ static struct {
>>      .tethering_technologies = NULL,
>>      .persistent_tethering_mode = false,
>>      .enable_6to4 = false,
>> +    .statsdir = NULL,
>>  };
>>  
>>  #define CONF_BG_SCAN                    "BackgroundScanning"
>> @@ -105,6 +107,7 @@ static struct {
>>  #define CONF_TETHERING_TECHNOLOGIES      "TetheringTechnologies"
>>  #define CONF_PERSISTENT_TETHERING_MODE  "PersistentTetheringMode"
>>  #define CONF_ENABLE_6TO4                "Enable6to4"
>> +#define CONF_STATS_DIR                  "StatsDir"
>>  
>>  static const char *supported_options[] = {
>>      CONF_BG_SCAN,
>> @@ -120,6 +123,7 @@ static const char *supported_options[] = {
>>      CONF_TETHERING_TECHNOLOGIES,
>>      CONF_PERSISTENT_TETHERING_MODE,
>>      CONF_ENABLE_6TO4,
>> +    CONF_STATS_DIR,
>>      NULL
>>  };
>>  
>> @@ -244,6 +248,7 @@ static void parse_config(GKeyFile *config)
>>      char **tethering;
>>      gsize len;
>>      int timeout;
>> +    char *statsdir;
>>  
>>      if (!config) {
>>              connman_settings.auto_connect =
>> @@ -367,6 +372,15 @@ static void parse_config(GKeyFile *config)
>>              connman_settings.enable_6to4 = boolean;
>>  
>>      g_clear_error(&error);
>> +
>> +    statsdir = __connman_config_get_string(config, "General",
>> +                                    CONF_STATS_DIR, &error);
>> +    if (!error)
>> +            connman_settings.statsdir = statsdir;
>> +    else
>> +            connman_settings.statsdir = g_strdup(STORAGEDIR);
>> +
>> +    g_clear_error(&error);
>>  }
>>  
>>  static int config_init(const char *file)
>> @@ -564,6 +578,14 @@ char **connman_setting_get_string_list(const
>> char *key)
>>      return NULL;
>>  }
>>  
>> +char *connman_setting_get_string(const char *key)
>> +{
>> +    if (g_str_equal(key, CONF_STATS_DIR))
>> +            return connman_settings.statsdir;
>> +
>> +    return NULL;
>> +}
>> +
>>  unsigned int *connman_setting_get_uint_list(const char *key)
>>  {
>>      if (g_str_equal(key, CONF_AUTO_CONNECT))
>> diff --git a/src/stats.c b/src/stats.c
>> index 26343b1..3958e17 100644
>> --- a/src/stats.c
>> +++ b/src/stats.c
>> @@ -350,7 +350,7 @@ static int stats_open(struct stats_file *file,
>>  static int stats_open_temp(struct stats_file *file)
>>  {
>>      file->name = g_strdup_printf("%s/stats.XXXXXX.tmp",
>> -                                    STORAGEDIR);
>> +                                    connman_setting_get_string("
>> StatsDir"));
>>      file->fd = g_mkstemp_full(file->name, O_RDWR | O_CREAT,
>> 0644);
>>      if (file->fd < 0) {
>>              connman_error("create tempory file error %s for %s",
>> @@ -687,7 +687,7 @@ int __connman_stats_service_register(struct
>> connman_service *service)
>>              return -EALREADY;
>>      }
>>  
>> -    dir = g_strdup_printf("%s/%s", STORAGEDIR,
>> +    dir = g_strdup_printf("%s/%s",
>> connman_setting_get_string("StatsDir"),
>>                              __connman_service_get_ident(service)
>> );
>>  
>>      /* If the dir doesn't exist, create it */
>> @@ -704,9 +704,10 @@ int __connman_stats_service_register(struct
>> connman_service *service)
>>  
>>      g_free(dir);
>>  
>> -    name = g_strdup_printf("%s/%s/data", STORAGEDIR,
>> +    name = g_strdup_printf("%s/%s/data",
>> connman_setting_get_string("StatsDir"),
>>                              __connman_service_get_ident(service)
>> );
>> -    file->history_name = g_strdup_printf("%s/%s/history",
>> STORAGEDIR,
>> +    file->history_name = g_strdup_printf("%s/%s/history",
>> +                            connman_setting_get_string("StatsDir
>> "),
>>                              __connman_service_get_ident(service)
>> );
>>  
>>      /* TODO: Use a global config file instead of hard coded
>> value. */



------------------------------

Subject: Digest Footer

_______________________________________________
connman mailing list
[email protected]
https://lists.01.org/mailman/listinfo/connman


------------------------------

End of connman Digest, Vol 8, Issue 3
*************************************

Reply via email to