On 12/12/22 13:47, Dumitru Ceara wrote: > 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. >
I also backported this to all stable branches down to branch 22.03. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
