On 21 Jan 2025, at 9:01, Roi Dayan wrote:
> On 20/01/2025 17:38, Eelco Chaudron wrote: >> >> >> On 20 Jan 2025, at 16:00, Roi Dayan wrote: >> >>> ovs_get_program_version() already returns the formatted program name and >>> version instead of doing it again. >>> >>> Signed-off-by: Roi Dayan <[email protected]> >> >> One small issue below, but we should be able to apply those at commit time. >> Did some tests with these change, and with the comment below folded in; > > ok so not sending v3. thanks. > >> >> Acked-by: Eelco Chaudron <[email protected]> >> >>> --- >>> >>> Notes: >>> v2 >>> - Do the same for ovsdb-server.c and ovsdb-error.c. >>> >>> lib/ovsdb-error.c | 2 +- >>> ovsdb/ovsdb-server.c | 3 +-- >>> vswitchd/bridge.c | 3 +-- >>> 3 files changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/lib/ovsdb-error.c b/lib/ovsdb-error.c >>> index 56512fc28dda..036897db0cd7 100644 >>> --- a/lib/ovsdb-error.c >>> +++ b/lib/ovsdb-error.c >>> @@ -146,7 +146,7 @@ ovsdb_internal_error(struct ovsdb_error *inner_error, >>> ds_put_char(&ds, ')'); >>> } >>> >>> - ds_put_format(&ds, " (%s %s)", program_name, VERSION VERSION_SUFFIX); >>> + ds_put_format(&ds, " %s", ovs_get_program_version()); >> >> We should keep the () here. > > > I just thought it looks good without the brackets as the last string showing > the program name and version without the brackets. > looks like this: > > ovsdb-tool: internal error: ovsdb/log.c:610: fake error > (backtrace:./ovsdb/ovsdb-tool(backtrace_capture+0x20) [0x4eee6b], > ./ovsdb/ovsdb-tool(ovsdb_internal_error+0x118) [0x4cc38e], > ./ovsdb/ovsdb-tool(ovsdb_log_write+0x33) [0x481ad5], > ./ovsdb/ovsdb-tool(ovsdb_log_write_and_free+0x22) [0x481afa], > ./ovsdb/ovsdb-tool() [0x478cf9], ./ovsdb/ovsdb-tool() [0x478e9f], > ./ovsdb/ovsdb-tool() [0x478f99], ./ovsdb/ovsdb-tool() [0x4b157a], > ./ovsdb/ovsdb-tool(ovs_cmdl_run_command+0x27) [0x4b162a], > ./ovsdb/ovsdb-tool(main+0x97) [0x478541], > /lib64/libc.so.6(__libc_start_main+0xf2) [0x7ff7ccc4a6a2], > ./ovsdb/ovsdb-tool(_start+0x2d) [0x477edd]) ovsdb-tool (Open vSwitch) 3.4.90 Yes, I noticed the same while testing, however, people might have scripts to grep for this (hoping the addition of ‘(Open vSwitch)’ is not messing it up already). I’ve copied in Mike and Ilya who work more on the ovsdb. Any option on this? Cheers, Eelco >>> if (inner_error) { >>> char *s = ovsdb_error_to_string_free(inner_error); >>> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c >>> index fbc7ad5efe32..7fde858724b8 100644 >>> --- a/ovsdb/ovsdb-server.c >>> +++ b/ovsdb/ovsdb-server.c >>> @@ -817,8 +817,7 @@ main(int argc, char *argv[]) >>> /* ovsdb-server is usually a long-running process, in which case it >>> * makes plenty of sense to log the version, but --run makes >>> * ovsdb-server more like a command-line tool, so skip it. */ >>> - VLOG_INFO("%s (Open vSwitch) %s", program_name, >>> - VERSION VERSION_SUFFIX); >>> + VLOG_INFO("%s", ovs_get_program_version()); >>> } >>> >>> unixctl_command_register("exit", "", 0, 0, ovsdb_server_exit, >>> &exiting); >>> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c >>> index 509ea19ecc99..456b784c01d0 100644 >>> --- a/vswitchd/bridge.c >>> +++ b/vswitchd/bridge.c >>> @@ -3471,8 +3471,7 @@ bridge_run(void) >>> >>> vlog_enable_async(); >>> >>> - VLOG_INFO_ONCE("%s (Open vSwitch) %s", program_name, >>> - VERSION VERSION_SUFFIX); >>> + VLOG_INFO_ONCE("%s", ovs_get_program_version()); >>> } >>> } >>> >>> -- >>> 2.21.0 >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
