[ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable

Ilya Maximets i.maximets at ovn.org
Fri Oct 25 18:42:46 UTC 2019


> Thanks Ben: It works fine. Just a minor typo. It's much neat and trimmed now.
> 
> I tried with multiple options e.g.
> export OVN_NBCTL_OPTIONS="--db=unix:nb1.ovsdb --no-leader-only --leader-only --no-leader-only"
> 
> and it gets called as expected:
> Called as ovn-nbctl --db=unix:nb1.ovsdb --no-leader-only --leader-only --no-leader-only set-connection pssl:6641
> 
> ________________________________
> From: Ben Pfaff <blp at ovn.org>
> Sent: Friday, October 25, 2019 11:12 AM
> To: amginwal at gmail.com <amginwal at gmail.com>
> Cc: dev at openvswitch.org <dev at openvswitch.org>; Ginwala, Aliasgar <aginwala at ebay.com>
> Subject: Re: [ovs-dev] [PATCH v2] command-line.c: Support parsing ctl options via env variable
> 
> On Thu, Oct 24, 2019 at 10:43:14PM -0700, amginwal at gmail.com wrote:
>> From: Aliasgar Ginwala <aginwala at ebay.com>
>>
>> Signed-off-by: Aliasgar Ginwala <aginwala at ebay.com>
> 
> To me, this looks more complicated than necessary, and thus harder to
> reason about regarding correctness.  What about the following?  It does
> not make me think as hard about correctness.  However, I have not tested
> it.
> 
> -8<--------------------------cut here-------------------------->8--
> 
> From: Aliasgar Ginwala <aginwala at ebay.com>
> Date: Thu, 24 Oct 2019 22:43:14 -0700
> Subject: [PATCH] command-line.c: Support parsing ctl options via env variable
> 
> Signed-off-by: Aliasgar Ginwala <aginwala at ebay.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  lib/command-line.c | 34 ++++++++++++++++++++++++++++++++++
>  lib/command-line.h |  3 +++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/lib/command-line.c b/lib/command-line.c
> index 9e000bd28d1a..fa02b49152e5 100644
> --- a/lib/command-line.c
> +++ b/lib/command-line.c
> @@ -19,6 +19,7 @@
>  #include <getopt.h>
>  #include <limits.h>
>  #include <stdlib.h>
> +#include "svec.h"
>  #include "openvswitch/dynamic-string.h"
>  #include "ovs-thread.h"
>  #include "util.h"
> @@ -77,6 +78,39 @@ find_option_by_value(const struct option *options, int value)
>      return NULL;
>  }
> 
> +/* Parses options set using environment variable.  The caller specifies the
> + * supported options in environment variable.  On success, adds the parsed
> + * env variables in 'argv', the number of options in 'argc', and returns argv.
> + */
> +char **
> +ovs_cmdl_env_parse_all(int *argcp, char *argv[], const char *env_options)
> +{
> +    struct svec args = SVEC_EMPTY_INITIALIZER;
> +
> +    /* argv[0] stays in place. */
> +    ovs_assert(*argcp > 0);
> +    svec_add(&args, argv[0]);
> Should be argv_
> +
> +    /* Anything from the environment variable goes next. */
> +    if (env_options) {
> +        char *env_copy = xstrdup(env_options);
> +        char *save_ptr = NULL;
> +        for (char *option = strtok_r(env_copy, " ", &save_ptr);
> +             option; option = strtok_r(NULL, " ", &save_ptr)) {
> +            svec_add(&args, option);
> +        }
> +        free(env_copy);

Just curious, can we use svec_parse_words() instead of above loop?

Best regards, Ilya Maximets.


More information about the dev mailing list