On 12/12/22 13:03, Ales Musil wrote: > 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. >
Cool, thanks! Applied to the main branch. Regards, Dumitru > 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 >> >> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
