On Fri, May 22, 2020 at 7:21 PM Ciara Power <ciara.po...@intel.com> wrote:
>
> Initially, printf was used to indicate and error/warning resulting from
> telemetry initialisation. This is now fixed to use EAL logs for
> warnings, and the unnecessary printf for an error is removed.
>
> Fixes: eeb486f3ba65 ("eal: add telemetry as dependency")
> Fixes: dd6275a424ac ("telemetry: fix error log output")
> Cc: david.march...@redhat.com
>
> Signed-off-by: Ciara Power <ciara.po...@intel.com>
> ---
> lib/librte_eal/freebsd/eal.c | 4 +++-
> lib/librte_eal/linux/eal.c | 4 +++-
> lib/librte_telemetry/rte_telemetry.h | 9 +++++++++
> lib/librte_telemetry/telemetry.c | 2 --
> 4 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_eal/freebsd/eal.c b/lib/librte_eal/freebsd/eal.c
> index 14b52168e..07456059e 100644
> --- a/lib/librte_eal/freebsd/eal.c
> +++ b/lib/librte_eal/freebsd/eal.c
> @@ -956,13 +956,15 @@ rte_eal_init(int argc, char **argv)
> return -1;
> }
> if (!internal_config.no_telemetry) {
> - const char *error_str;
> + const char *error_str = NULL;
> if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> &internal_config.ctrl_cpuset, &error_str)
> != 0) {
> rte_eal_init_alert(error_str);
> return -1;
> }
> + if (error_str != NULL)
> + RTE_LOG(WARNING, EAL, "%s\n", error_str);
> }
>
> eal_mcfg_complete();
> diff --git a/lib/librte_eal/linux/eal.c b/lib/librte_eal/linux/eal.c
> index 9620d2544..10317a660 100644
> --- a/lib/librte_eal/linux/eal.c
> +++ b/lib/librte_eal/linux/eal.c
> @@ -1294,13 +1294,15 @@ rte_eal_init(int argc, char **argv)
> return -1;
> }
> if (!internal_config.no_telemetry) {
> - const char *error_str;
> + const char *error_str = NULL;
> if (rte_telemetry_init(rte_eal_get_runtime_dir(),
> &internal_config.ctrl_cpuset, &error_str)
> != 0) {
> rte_eal_init_alert(error_str);
> return -1;
> }
> + if (error_str != NULL)
> + RTE_LOG(WARNING, EAL, "%s\n", error_str);
"EAL: No legacy callbacks, legacy socket not created" comes all the times now.
Should we treat this as a warning? as it comes for with for the
built-in applications?
If it is important why not have a default handler in the EAL library?
> }
>
> eal_mcfg_complete();
> diff --git a/lib/librte_telemetry/rte_telemetry.h
> b/lib/librte_telemetry/rte_telemetry.h
> index 2c3c96cf7..eb7f2c917 100644
> --- a/lib/librte_telemetry/rte_telemetry.h
> +++ b/lib/librte_telemetry/rte_telemetry.h
> @@ -241,8 +241,17 @@ int
> rte_telemetry_register_cmd(const char *cmd, telemetry_cb fn, const char
> *help);
>
> /**
> + * @internal
> * Initialize Telemetry.
> *
> + * @param runtime_dir
> + * The runtime directory of DPDK.
> + * @param cpuset
> + * The CPU set to be used for setting the thread affinity.
> + * @param err_str
> + * This err_str pointer should point to NULL on entry. In the case of an
> error
> + * or warning, it will be non-NULL on exit.
> + *
> * @return
> * 0 on success.
> * @return
> diff --git a/lib/librte_telemetry/telemetry.c
> b/lib/librte_telemetry/telemetry.c
> index 7b6f8a79e..e7e3d861d 100644
> --- a/lib/librte_telemetry/telemetry.c
> +++ b/lib/librte_telemetry/telemetry.c
> @@ -403,12 +403,10 @@ rte_telemetry_init(const char *runtime_dir,
> rte_cpuset_t *cpuset,
> {
> if (telemetry_v2_init(runtime_dir, cpuset) != 0) {
> *err_str = telemetry_log_error;
> - printf("Error initialising telemetry - %s\n", *err_str);
> return -1;
> }
> if (telemetry_legacy_init(runtime_dir, cpuset) != 0) {
> *err_str = telemetry_log_error;
> - printf("No telemetry legacy support - %s\n", *err_str);
> }
> return 0;
> }
> --
> 2.17.1
>
