2017-07-18 15:49:55

by C Shapiro

[permalink] [raw]
Subject: [PATCH v3] Support to change set file via arg

ChromeOS is currently shifting to a more dynamic build system where a
single build will be created/managed/deployed to support multiple models
that were all created based on a single reference board.

This is being done for general cost/maintenance savings around the sheer
number of builds that are currently managed for ChromeOS.

To support this, we're changing our build images to have all of the
configuration available for all of the models supported by a given
build.

For the bluetooth stack, models have different product id values in the
DeviceID field for the main.conf config file.

E.g.
- https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-pyro/chromeos-base/chromeos-bsp-pyro/files/main.conf
- https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-reef/chromeos-base/chromeos-bsp-reef/files/main.conf
- https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-snappy/chromeos-base/chromeos-bsp-snappy/files/main.conf

This change allows the config file to be passed as a runtime argument to
bluetoothd, which allows the ChromeOS init script to determine the model
at runtime and then pass the appropriate model specific config file.

Signed-off-by: C Shapiro <[email protected]>
Reviewed-by: Simon Glass <[email protected]>
---
Changes in v3:
- Fix the commit message with correct links to the example usage.

src/bluetoothd.8.in | 4 ++++
src/main.c | 12 +++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/bluetoothd.8.in b/src/bluetoothd.8.in
index 97ef3ec94..d61dcc5b3 100644
--- a/src/bluetoothd.8.in
+++ b/src/bluetoothd.8.in
@@ -27,6 +27,10 @@ Print bluetoothd options and exit.
Enable logging in foreground. Directs log output to the controlling terminal \
in addition to syslog.
.TP
+.B -f, --configfile
+Specifies an explicit config file path instead of relying on the default path \
+(@CONFIGDIR@/main.conf) for the config file.
+.TP
.B -d, --debug=<file1>:<file2>:...
Sets how much information bluetoothd sends to the log destination (usually \
syslog's "daemon" facility). If the file options are omitted, then debugging \
diff --git a/src/main.c b/src/main.c
index bcc1e6fae..87bdb66cb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -490,6 +490,7 @@ static guint setup_signalfd(void)
static char *option_debug = NULL;
static char *option_plugin = NULL;
static char *option_noplugin = NULL;
+static char *option_configfile = NULL;
static gboolean option_compat = FALSE;
static gboolean option_detach = TRUE;
static gboolean option_version = FALSE;
@@ -505,6 +506,9 @@ static void free_options(void)

g_free(option_noplugin);
option_noplugin = NULL;
+
+ g_free(option_configfile);
+ option_configfile = NULL;
}

static void disconnect_dbus(void)
@@ -577,6 +581,9 @@ static GOptionEntry options[] = {
"Specify plugins to load", "NAME,..," },
{ "noplugin", 'P', 0, G_OPTION_ARG_STRING, &option_noplugin,
"Specify plugins not to load", "NAME,..." },
+ { "configfile", 'f', 0, G_OPTION_ARG_STRING, &option_configfile,
+ "Specify an explicit path to the config file",
+ "FILE"},
{ "compat", 'C', 0, G_OPTION_ARG_NONE, &option_compat,
"Provide deprecated command line interfaces" },
{ "experimental", 'E', 0, G_OPTION_ARG_NONE, &option_experimental,
@@ -636,7 +643,10 @@ int main(int argc, char *argv[])

sd_notify(0, "STATUS=Starting up");

- main_conf = load_config(CONFIGDIR "/main.conf");
+ if (option_configfile)
+ main_conf = load_config(option_configfile);
+ else
+ main_conf = load_config(CONFIGDIR "/main.conf");

parse_config(main_conf);

--
2.13.2.932.g7449e964c-goog


2017-08-03 17:21:19

by C Shapiro

[permalink] [raw]
Subject: Re: [PATCH v3] Support to change set file via arg

Hi Eramoto,
I appreciate the feedback. I incorporated it into v4 that I just sent out.

Thanks,
Charles

On Tue, Aug 1, 2017 at 4:39 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Charles,
>
> On 07/19/2017 12:49 AM, C Shapiro wrote:
>> ChromeOS is currently shifting to a more dynamic build system where a
>> single build will be created/managed/deployed to support multiple models
>> that were all created based on a single reference board.
>>
>> This is being done for general cost/maintenance savings around the sheer
>> number of builds that are currently managed for ChromeOS.
>>
>> To support this, we're changing our build images to have all of the
>> configuration available for all of the models supported by a given
>> build.
>>
>> For the bluetooth stack, models have different product id values in the
>> DeviceID field for the main.conf config file.
>>
>> E.g.
>> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-pyro/chromeos-base/chromeos-bsp-pyro/files/main.conf
>> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-reef/chromeos-base/chromeos-bsp-reef/files/main.conf
>> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-snappy/chromeos-base/chromeos-bsp-snappy/files/main.conf
>>
>> This change allows the config file to be passed as a runtime argument to
>> bluetoothd, which allows the ChromeOS init script to determine the model
>> at runtime and then pass the appropriate model specific config file.
>>
>> Signed-off-by: C Shapiro <[email protected]>
>> Reviewed-by: Simon Glass <[email protected]>
>> ---
>> Changes in v3:
>> - Fix the commit message with correct links to the example usage.
>>
>> src/bluetoothd.8.in | 4 ++++
>> src/main.c | 12 +++++++++++-
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> Could you add the specified main.conf path to the following messages
> when --configfile command-line option was used.
>
> Cscope tag: main.conf
> # line filename / context / line
> 1 212 src/main.c <<<unknown>>>
> warn("Unknown key %s for group %s in main.conf",
> 2 241 src/main.c <<<unknown>>>
> warn("Unknown group %s in main.conf", keys[i]);
> 3 276 src/main.c <<<unknown>>>
> DBG("parsing main.conf");
>
> I applied this patch and tested my patches (git log 32867836..c15b986e)
> with using this command-line option. At that time, I passed various
> main.confs composed of different config option settings to it. Using it
> was more useful than editing the fixed main.conf (CONFIGDIR "/main.conf"),
> but it was hard to resolve which output messages is related to any
> main.conf when I tested with each main.conf at once.
> So, I suggest to add the specified main.conf path to these messages. It
> seems we can make resolving easier when we hit a problem with a specified
> main.conf.
>
>
> Regards,
> Eramoto
>

2017-08-01 10:39:16

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH v3] Support to change set file via arg

Hi Charles,

On 07/19/2017 12:49 AM, C Shapiro wrote:
> ChromeOS is currently shifting to a more dynamic build system where a
> single build will be created/managed/deployed to support multiple models
> that were all created based on a single reference board.
>
> This is being done for general cost/maintenance savings around the sheer
> number of builds that are currently managed for ChromeOS.
>
> To support this, we're changing our build images to have all of the
> configuration available for all of the models supported by a given
> build.
>
> For the bluetooth stack, models have different product id values in the
> DeviceID field for the main.conf config file.
>
> E.g.
> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-pyro/chromeos-base/chromeos-bsp-pyro/files/main.conf
> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-reef/chromeos-base/chromeos-bsp-reef/files/main.conf
> - https://chromium.googlesource.com/chromiumos/overlays/board-overlays/+/master/overlay-snappy/chromeos-base/chromeos-bsp-snappy/files/main.conf
>
> This change allows the config file to be passed as a runtime argument to
> bluetoothd, which allows the ChromeOS init script to determine the model
> at runtime and then pass the appropriate model specific config file.
>
> Signed-off-by: C Shapiro <[email protected]>
> Reviewed-by: Simon Glass <[email protected]>
> ---
> Changes in v3:
> - Fix the commit message with correct links to the example usage.
>
> src/bluetoothd.8.in | 4 ++++
> src/main.c | 12 +++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)

Could you add the specified main.conf path to the following messages
when --configfile command-line option was used.

Cscope tag: main.conf
# line filename / context / line
1 212 src/main.c <<<unknown>>>
warn("Unknown key %s for group %s in main.conf",
2 241 src/main.c <<<unknown>>>
warn("Unknown group %s in main.conf", keys[i]);
3 276 src/main.c <<<unknown>>>
DBG("parsing main.conf");

I applied this patch and tested my patches (git log 32867836..c15b986e)
with using this command-line option. At that time, I passed various
main.confs composed of different config option settings to it. Using it
was more useful than editing the fixed main.conf (CONFIGDIR "/main.conf"),
but it was hard to resolve which output messages is related to any
main.conf when I tested with each main.conf at once.
So, I suggest to add the specified main.conf path to these messages. It
seems we can make resolving easier when we hit a problem with a specified
main.conf.


Regards,
Eramoto