Hi Dumitru,

just one small comment below.


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.


>    --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.

-- 

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

Reply via email to