On Mon, Dec 12, 2022 at 12:59 PM Dumitru Ceara <[email protected]> wrote:
> 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? > Ok, in that let's leave it as is. Thanks, Ales > > > > >> --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 > > -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] IM: amusil <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
