[RESEND PATCH v5 0/4] cmd: env: add option for quiet output on env info

Hi,
It is a V5 for [1] serie.
RESEND without "stm32mp1: use the command env info -q in env_check" sent in separate serie [4].
I add the -q option for 'env info' command and I also add pytest for this command.
Test for ENV_IS_IN_DEVICE is included in separate serie [2] (I will activate ENV_IS_IN_EXT4 support in sandbox)
To avoid compilation warning, I add prototype for env_get_location for the patch 3/7 "cmd: env: check real location for env info command" as it is done in [3].
[1] http://patchwork.ozlabs.org/project/uboot/list/?series=183438 [2] http://patchwork.ozlabs.org/project/uboot/list/?series=158160 [3] http://patchwork.ozlabs.org/patch/1230200/ [4] http://patchwork.ozlabs.org/project/uboot/list/?series=183450
Changes in v5: - allow to execute cmd_nvedit_info on real board - rename test_env_info_quiet to test_env_info_sandbox
Changes in v4: - rebase on master branch - move 5/7 stm32mp1: configs: activate CMD_ERASEENV in a new serie 183380 - move 2/7 and 4/7 in a new serie 183387
Changes in v3: - update commit message (sub-commandi) - rename test_env_info_test to test_env_info_quiet
Changes in v2: - update prototype in env_internal.h as done in "env: add prototypes for weak function" - remove comment change in env.c (implementation information) - move env_location declaration - activate env info command in sandbox (new) - add pytest test_env_info and test_env_info_test (new)
Patrick Delaunay (4): cmd: env: add option for quiet output on env info cmd: env: check real location for env info command configs: sandbox: Enable sub command 'env info' test: env: add test for env info sub-command
cmd/Kconfig | 1 + cmd/nvedit.c | 37 ++++++++++++++---- configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + include/env_internal.h | 11 ++++++ test/py/tests/test_env.py | 63 ++++++++++++++++++++++++++++++ 8 files changed, 108 insertions(+), 8 deletions(-)

The "env info" can be use for test with -d and -p parameter, in scripting case the output of the command is not needed.
This patch allows to deactivate this output with a new option "-q".
For example, we can save the environment if default environment is used and persistent storage is managed with: if env info -p -d -q; then env save; fi
Without the quiet option, I have the unnecessary traces First boot: Default environment is used Environment can be persisted Saving Environment to EXT4... File System is consistent
Next boot: Environment was loaded from persistent storage Environment can be persisted
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v1)
cmd/Kconfig | 1 + cmd/nvedit.c | 26 +++++++++++++++++++------- 2 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 192b3b262f..1de57988ae 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -607,6 +607,7 @@ config CMD_NVEDIT_INFO This command can be optionally used for evaluation in scripts: [-d] : evaluate whether default environment is used [-p] : evaluate whether environment can be persisted + [-q] : quiet output The result of multiple evaluations will be combined with AND.
endmenu diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 49338b4d36..68cb1a4a8f 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1224,12 +1224,15 @@ static int print_env_info(void) * env info - display environment information * env info [-d] - evaluate whether default environment is used * env info [-p] - evaluate whether environment can be persisted + * Add [-q] - quiet mode, use only for command result, for test by example: + * test env info -p -d -q */ static int do_env_info(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { int eval_flags = 0; int eval_results = 0; + bool quiet = false;
/* display environment information */ if (argc <= 1) @@ -1247,6 +1250,9 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, case 'p': eval_flags |= ENV_INFO_IS_PERSISTED; break; + case 'q': + quiet = true; + break; default: return CMD_RET_USAGE; } @@ -1256,20 +1262,24 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, /* evaluate whether default environment is used */ if (eval_flags & ENV_INFO_IS_DEFAULT) { if (gd->flags & GD_FLG_ENV_DEFAULT) { - printf("Default environment is used\n"); + if (!quiet) + printf("Default environment is used\n"); eval_results |= ENV_INFO_IS_DEFAULT; } else { - printf("Environment was loaded from persistent storage\n"); + if (!quiet) + printf("Environment was loaded from persistent storage\n"); } }
/* evaluate whether environment can be persisted */ if (eval_flags & ENV_INFO_IS_PERSISTED) { #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) - printf("Environment can be persisted\n"); + if (!quiet) + printf("Environment can be persisted\n"); eval_results |= ENV_INFO_IS_PERSISTED; #else - printf("Environment cannot be persisted\n"); + if (!quiet) + printf("Environment cannot be persisted\n"); #endif }
@@ -1326,7 +1336,7 @@ static struct cmd_tbl cmd_env_sub[] = { U_BOOT_CMD_MKENT(import, 5, 0, do_env_import, "", ""), #endif #if defined(CONFIG_CMD_NVEDIT_INFO) - U_BOOT_CMD_MKENT(info, 2, 0, do_env_info, "", ""), + U_BOOT_CMD_MKENT(info, 3, 0, do_env_info, "", ""), #endif U_BOOT_CMD_MKENT(print, CONFIG_SYS_MAXARGS, 1, do_env_print, "", ""), #if defined(CONFIG_CMD_RUN) @@ -1405,8 +1415,10 @@ static char env_help_text[] = #endif #if defined(CONFIG_CMD_NVEDIT_INFO) "env info - display environment information\n" - "env info [-d] - whether default environment is used\n" - "env info [-p] - whether environment can be persisted\n" + "env info [-d] [-p] [-q] - evaluate environment information\n" + " "-d": default environment is used\n" + " "-p": environment can be persisted\n" + " "-q": quiet output\n" #endif "env print [-a | name ...] - print environment\n" #if defined(CONFIG_CMD_NVEDIT_EFI)

On Fri, Jun 19, 2020 at 02:03:34PM +0200, Patrick Delaunay wrote:
The "env info" can be use for test with -d and -p parameter, in scripting case the output of the command is not needed.
This patch allows to deactivate this output with a new option "-q".
For example, we can save the environment if default environment is used and persistent storage is managed with: if env info -p -d -q; then env save; fi
Without the quiet option, I have the unnecessary traces First boot: Default environment is used Environment can be persisted Saving Environment to EXT4... File System is consistent
Next boot: Environment was loaded from persistent storage Environment can be persisted
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On Fri, Jun 19, 2020 at 02:03:34PM +0200, Patrick Delaunay wrote:
The "env info" can be use for test with -d and -p parameter, in scripting case the output of the command is not needed.
This patch allows to deactivate this output with a new option "-q".
For example, we can save the environment if default environment is used and persistent storage is managed with: if env info -p -d -q; then env save; fi
Without the quiet option, I have the unnecessary traces First boot: Default environment is used Environment can be persisted Saving Environment to EXT4... File System is consistent
Next boot: Environment was loaded from persistent storage Environment can be persisted
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

Check the current ENV location, dynamically provided by the weak function env_get_location to be sure that the environment can be persistent.
The compilation flag ENV_IS_IN_DEVICE is not enough when the board dynamically select the available storage location (according boot device for example).
This patch solves issue for stm32mp1 platform, when the boot device is USB.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - update prototype in env_internal.h as done in "env: add prototypes for weak function" - remove comment change in env.c (implementation information) - move env_location declaration
cmd/nvedit.c | 15 ++++++++++++--- include/env_internal.h | 11 +++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/cmd/nvedit.c b/cmd/nvedit.c index 68cb1a4a8f..0f9cea96f3 100644 --- a/cmd/nvedit.c +++ b/cmd/nvedit.c @@ -1233,6 +1233,9 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, int eval_flags = 0; int eval_results = 0; bool quiet = false; +#if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) + enum env_location loc; +#endif
/* display environment information */ if (argc <= 1) @@ -1274,9 +1277,15 @@ static int do_env_info(struct cmd_tbl *cmdtp, int flag, /* evaluate whether environment can be persisted */ if (eval_flags & ENV_INFO_IS_PERSISTED) { #if defined(CONFIG_CMD_SAVEENV) && defined(ENV_IS_IN_DEVICE) - if (!quiet) - printf("Environment can be persisted\n"); - eval_results |= ENV_INFO_IS_PERSISTED; + loc = env_get_location(ENVOP_SAVE, gd->env_load_prio); + if (ENVL_NOWHERE != loc && ENVL_UNKNOWN != loc) { + if (!quiet) + printf("Environment can be persisted\n"); + eval_results |= ENV_INFO_IS_PERSISTED; + } else { + if (!quiet) + printf("Environment cannot be persisted\n"); + } #else if (!quiet) printf("Environment cannot be persisted\n"); diff --git a/include/env_internal.h b/include/env_internal.h index e89fbdb1b7..66550434c3 100644 --- a/include/env_internal.h +++ b/include/env_internal.h @@ -211,6 +211,17 @@ struct env_driver {
extern struct hsearch_data env_htab;
+/** + * env_get_location()- Provide the best location for the U-Boot environment + * + * It is a weak function allowing board to overidde the environment location + * + * @op: operations performed on the environment + * @prio: priority between the multiple environments, 0 being the + * highest priority + * @return an enum env_location value on success, or -ve error code. + */ +enum env_location env_get_location(enum env_operation op, int prio); #endif /* DO_DEPS_ONLY */
#endif /* _ENV_INTERNAL_H_ */

On Fri, Jun 19, 2020 at 02:03:35PM +0200, Patrick Delaunay wrote:
Check the current ENV location, dynamically provided by the weak function env_get_location to be sure that the environment can be persistent.
The compilation flag ENV_IS_IN_DEVICE is not enough when the board dynamically select the available storage location (according boot device for example).
This patch solves issue for stm32mp1 platform, when the boot device is USB.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org
Reviewed-by: Tom Rini trini@konsulko.com

On Fri, Jun 19, 2020 at 02:03:35PM +0200, Patrick Delaunay wrote:
Check the current ENV location, dynamically provided by the weak function env_get_location to be sure that the environment can be persistent.
The compilation flag ENV_IS_IN_DEVICE is not enough when the board dynamically select the available storage location (according boot device for example).
This patch solves issue for stm32mp1 platform, when the boot device is USB.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!

Enable support for sub command 'env info' in sandbox with CONFIG_CMD_NVEDIT_INFO. This is aimed primarily at adding unit test.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - activate env info command in sandbox (new)
configs/sandbox64_defconfig | 1 + configs/sandbox_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 4 files changed, 4 insertions(+)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index a3f049e124..a2410b3368 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -30,6 +30,7 @@ CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 5b7569319b..0c9e0b9f1f 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 21f9047046..bc4819f998 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -24,6 +24,7 @@ CONFIG_CMD_BOOTEFI_HELLO=y # CONFIG_CMD_ELF is not set CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index fc8b26e88c..64f2ed5102 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_ASKENV=y CONFIG_CMD_GREPENV=y CONFIG_CMD_ENV_CALLBACK=y CONFIG_CMD_ENV_FLAGS=y +CONFIG_CMD_NVEDIT_INFO=y CONFIG_LOOPW=y CONFIG_CMD_MD5SUM=y CONFIG_CMD_MEMINFO=y

On Fri, Jun 19, 2020 at 02:03:36PM +0200, Patrick Delaunay wrote:
Enable support for sub command 'env info' in sandbox with CONFIG_CMD_NVEDIT_INFO. This is aimed primarily at adding unit test.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!

Add a pytest for testing the env info sub-command:
test_env_info: test command with several option that can be executed on real hardware device without assumption
test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent
The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
Changes in v5: - allow to execute cmd_nvedit_info on real board - rename test_env_info_quiet to test_env_info_sandbox
Changes in v4: - rebase on master branch - move 5/7 stm32mp1: configs: activate CMD_ERASEENV in a new serie 183380 - move 2/7 and 4/7 in a new serie 183387
Changes in v3: - update commit message (sub-commandi) - rename test_env_info_test to test_env_info_quiet
Changes in v2: - add pytest test_env_info and test_env_info_test (new)
test/py/tests/test_env.py | 63 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+)
diff --git a/test/py/tests/test_env.py b/test/py/tests/test_env.py index 6ff38f1020..a64aaa9bc5 100644 --- a/test/py/tests/test_env.py +++ b/test/py/tests/test_env.py @@ -336,3 +336,66 @@ def test_env_import_whitelist_delete(state_test_env): unset_var(state_test_env, 'foo2') unset_var(state_test_env, 'foo3') unset_var(state_test_env, 'foo4') + +@pytest.mark.buildconfigspec('cmd_nvedit_info') +def test_env_info(state_test_env): + + """Test 'env info' command with all possible options. + """ + c = state_test_env.u_boot_console + + response = c.run_command('env info') + nb_line = 0 + for l in response.split('\n'): + if 'env_valid = ' in l: + assert '= invalid' in l or '= valid' in l or '= redundant' in l + nb_line += 1 + elif 'env_ready =' in l or 'env_use_default =' in l: + assert '= true' in l or '= false' in l + nb_line += 1 + else: + assert true + assert nb_line == 3 + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response or "Environment was loaded from persistent storage" in response + assert 'Environment can be persisted' in response or "Environment cannot be persisted" in response + + response = c.run_command('env info -p -d -q') + assert response == "" + + response = c.run_command('env info -p -q') + assert response == "" + + response = c.run_command('env info -d -q') + assert response == "" + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_nvedit_info') +@pytest.mark.buildconfigspec('cmd_echo') +def test_env_info_sandbox(state_test_env): + + """Test 'env info' command result with several options on sandbox + with a known ENV configuration: ready & default & persistent + """ + c = state_test_env.u_boot_console + + response = c.run_command('env info') + assert 'env_ready = true' in response + assert 'env_use_default = true' in response + + response = c.run_command('env info -p -d') + assert 'Default environment is used' in response + assert 'Environment cannot be persisted' in response + + response = c.run_command('env info -d -q') + response = c.run_command('echo $?') + assert response == "0" + + response = c.run_command('env info -p -q') + response = c.run_command('echo $?') + assert response == "1" + + response = c.run_command('env info -d -p -q') + response = c.run_command('echo $?') + assert response == "1"

On 6/19/20 6:03 AM, Patrick Delaunay wrote:
Add a pytest for testing the env info sub-command:
test_env_info: test command with several option that can be executed on real hardware device without assumption
test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent
The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi
Acked-by: Stephen Warren swarren@nvidia.com
+def test_env_info(state_test_env):
...
- for l in response.split('\n'):
if 'env_valid = ' in l:
assert '= invalid' in l or '= valid' in l or '= redundant' in l
nb_line += 1
elif 'env_ready =' in l or 'env_use_default =' in l:
assert '= true' in l or '= false' in l
nb_line += 1
else:
assert true
Those last two lines don't seem terribly useful:-)

Hi Stephen,
From: Stephen Warren swarren@wwwdotorg.org Sent: lundi 22 juin 2020 20:51
On 6/19/20 6:03 AM, Patrick Delaunay wrote:
Add a pytest for testing the env info sub-command:
test_env_info: test command with several option that can be executed on real hardware device without assumption
test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent
The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi
Acked-by: Stephen Warren swarren@nvidia.com
Thanks
+def test_env_info(state_test_env):
...
- for l in response.split('\n'):
if 'env_valid = ' in l:
assert '= invalid' in l or '= valid' in l or '= redundant' in l
nb_line += 1
elif 'env_ready =' in l or 'env_use_default =' in l:
assert '= true' in l or '= false' in l
nb_line += 1
else:
assert true
Those last two lines don't seem terribly useful:-)
Not really, beacuse I add the nb_line check, this first check can be removed.
Do expect a V6 just for that?
Patrick

On 6/23/20 7:25 AM, Patrick DELAUNAY wrote:
Hi Stephen,
From: Stephen Warren swarren@wwwdotorg.org Sent: lundi 22 juin 2020 20:51
On 6/19/20 6:03 AM, Patrick Delaunay wrote:
Add a pytest for testing the env info sub-command:
test_env_info: test command with several option that can be executed on real hardware device without assumption
test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent
The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi
Acked-by: Stephen Warren swarren@nvidia.com
Thanks
+def test_env_info(state_test_env):
...
- for l in response.split('\n'):
if 'env_valid = ' in l:
assert '= invalid' in l or '= valid' in l or '= redundant' in l
nb_line += 1
elif 'env_ready =' in l or 'env_use_default =' in l:
assert '= true' in l or '= false' in l
nb_line += 1
else:
assert true
Those last two lines don't seem terribly useful:-)
Not really, beacuse I add the nb_line check, this first check can be removed.
Do expect a V6 just for that?
Probably not.

Hi Stephen,
From: Stephen Warren swarren@wwwdotorg.org Sent: mardi 23 juin 2020 22:25
On 6/23/20 7:25 AM, Patrick DELAUNAY wrote:
Hi Stephen,
From: Stephen Warren swarren@wwwdotorg.org Sent: lundi 22 juin 2020 20:51
On 6/19/20 6:03 AM, Patrick Delaunay wrote:
Add a pytest for testing the env info sub-command:
...
+def test_env_info(state_test_env):
...
- for l in response.split('\n'):
if 'env_valid = ' in l:
assert '= invalid' in l or '= valid' in l or '= redundant' in l
nb_line += 1
elif 'env_ready =' in l or 'env_use_default =' in l:
assert '= true' in l or '= false' in l
nb_line += 1
else:
assert true
Those last two lines don't seem terribly useful:-)
Not really, beacuse I add the nb_line check, this first check can be removed.
Do expect a V6 just for that?
Probably not.
OK I will keep this remark if I need to update the serie (or the next planned updates of this test)
Patrick

On Fri, Jun 19, 2020 at 02:03:37PM +0200, Patrick Delaunay wrote:
Add a pytest for testing the env info sub-command:
test_env_info: test command with several option that can be executed on real hardware device without assumption
test_env_info_sandbox: test the result on sandbox with a known ENV configuration: ready & default & persistent
The quiet option '-q' is used for support in shell test; for example: if env info -p -d -q; then env save; fi
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com Acked-by: Stephen Warren swarren@nvidia.com
Applied to u-boot/master, thanks!
participants (4)
-
Patrick DELAUNAY
-
Patrick Delaunay
-
Stephen Warren
-
Tom Rini