On 12/12/22 07:48, Ales Musil wrote: > Hi Dumitru, > Hi Ales,
> just one small comment below. > Thanks for your review! > > On Fri, Dec 9, 2022 at 3:13 PM Dumitru Ceara <[email protected]> wrote: > >> Similar to other OVN utilities, ovn-trace now supports --no-leader-only >> and --leader-only arguments. The latter is default. >> >> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2152151 >> Reported-by: Patryk Diak <[email protected]> >> Signed-off-by: Dumitru Ceara <[email protected]> >> --- >> utilities/ovn-trace.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c >> index 79ed5a9af1..07ebac5e54 100644 >> --- a/utilities/ovn-trace.c >> +++ b/utilities/ovn-trace.c >> @@ -60,6 +60,9 @@ static char *unixctl_path; >> /* The southbound database. */ >> static struct ovsdb_idl *ovnsb_idl; >> >> +/* --leader-only, --no-leader-only: Only accept the leader in a cluster. >> */ >> +static int leader_only = true; >> + >> /* --detailed: Show a detailed, table-by-table trace. */ >> static bool detailed; >> >> @@ -138,6 +141,7 @@ main(int argc, char *argv[]) >> 1, INT_MAX, ovntrace_trace, NULL); >> } >> ovnsb_idl = ovsdb_idl_create(db, &sbrec_idl_class, true, false); >> + ovsdb_idl_set_leader_only(ovnsb_idl, leader_only); >> >> bool already_read = false; >> for (;;) { >> @@ -243,6 +247,8 @@ parse_options(int argc, char *argv[]) >> { >> enum { >> OPT_DB = UCHAR_MAX + 1, >> + OPT_LEADER_ONLY, >> + OPT_NO_LEADER_ONLY, >> OPT_UNIXCTL, >> OPT_DETAILED, >> OPT_SUMMARY, >> @@ -260,6 +266,8 @@ parse_options(int argc, char *argv[]) >> }; >> static const struct option long_options[] = { >> {"db", required_argument, NULL, OPT_DB}, >> + {"leader-only", no_argument, NULL, OPT_LEADER_ONLY}, >> + {"no-leader-only", no_argument, NULL, OPT_NO_LEADER_ONLY}, >> {"unixctl", required_argument, NULL, OPT_UNIXCTL}, >> {"detailed", no_argument, NULL, OPT_DETAILED}, >> {"summary", no_argument, NULL, OPT_SUMMARY}, >> @@ -294,6 +302,14 @@ parse_options(int argc, char *argv[]) >> db = optarg; >> break; >> >> + case OPT_LEADER_ONLY: >> + leader_only = true; >> + break; >> + >> + case OPT_NO_LEADER_ONLY: >> + leader_only = false; >> + break; >> + >> case OPT_UNIXCTL: >> unixctl_path = optarg; >> break; >> @@ -390,6 +406,7 @@ Output style options:\n\ >> Other options:\n\ >> --db=DATABASE connect to DATABASE\n\ >> (default: %s)\n\ >> + --no-leader-only accept any cluster member, not just the >> leader\n\ >> > > We should also put the --leader-only into the usage message so it is > completely clear. > The reason why I didn't do that is because we don't do it for any of the other default behaviors. E.g., --shuffle-remotes, --no-friendly-names. Maybe we can make add it explicitly to the usage message but I don't really think that's required. If you really see benefit in this maybe we can do it as a treewide follow up change, what do you think? > >> --ovs[=REMOTE] obtain corresponding OpenFlow flows from >> REMOTE\n\ >> (default: %s)\n\ >> --unixctl=SOCKET set control socket name\n\ >> -- >> 2.31.1 >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > With that fixed > > Acked-by: Ales Musil <[email protected]> > > Thanks, > Ales. > Thanks, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
