[ovs-dev] [PATCH 2/4] db-ctl-base: make cmd_show_table private

Alex Wang alexw at nicira.com
Thu Jul 16 17:11:12 UTC 2015


On Mon, Jul 13, 2015 at 11:48 PM, Andy Zhou <azhou at nicira.com> wrote:

> Instead of require user to declare a global variable, pass the value
> via ctl_init().
>
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> ---
>  lib/db-ctl-base.c     | 34 +++++++++++++++++++++++++---------
>  lib/db-ctl-base.h     | 17 +++--------------
>  utilities/ovs-vsctl.c |  4 ++--
>  vtep/vtep-ctl.c       |  4 ++--
>  4 files changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index d38bf03..b0f6fd5 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -46,6 +46,19 @@ VLOG_DEFINE_THIS_MODULE(db_ctl_base);
>  struct ovsdb_idl *the_idl;
>  struct ovsdb_idl_txn *the_idl_txn;
>
> +/* This array defines the 'show' command output format.  User can check
> the
> + * definition in utilities/ovs-vsctl.c as reference.
> + *
> + * Particularly, if an element in 'columns[]' represents a reference to
> + * another table, the referred table must also be defined as an entry in
> + * in 'cmd_show_tables[]'.
>


want to change 'columns[]' to 'columns[] in struct cmd_show_table'



> + *
> + * The definition must end with an all-NULL entry.  It is initalized once
> + * when ctl_init() is called.
> + *
> + * */
> +static const struct cmd_show_table *cmd_show_tables;
> +
>






>  /* Represents all tables in the schema.  User must define 'tables'
>   * in implementation and supply via clt_init().  The definition must end
>   * with an all-NULL entry. */
> @@ -1590,7 +1603,7 @@ parse_command(int argc, char *argv[], struct shash
> *local_options,
>  static void
>  pre_cmd_show(struct ctl_context *ctx)
>  {
> -    struct cmd_show_table *show;
> +    const struct cmd_show_table *show;
>
>      for (show = cmd_show_tables; show->table; show++) {
>          size_t i;
> @@ -1608,10 +1621,10 @@ pre_cmd_show(struct ctl_context *ctx)
>      }
>  }
>
> -static struct cmd_show_table *
> +static const struct cmd_show_table *
>  cmd_show_find_table_by_row(const struct ovsdb_idl_row *row)
>  {
> -    struct cmd_show_table *show;
> +    const struct cmd_show_table *show;
>
>      for (show = cmd_show_tables; show->table; show++) {
>          if (show->table == row->table->class) {
> @@ -1621,10 +1634,10 @@ cmd_show_find_table_by_row(const struct
> ovsdb_idl_row *row)
>      return NULL;
>  }
>
> -static struct cmd_show_table *
> +static const struct cmd_show_table *
>  cmd_show_find_table_by_name(const char *name)
>  {
> -    struct cmd_show_table *show;
> +    const struct cmd_show_table *show;
>
>      for (show = cmd_show_tables; show->table; show++) {
>          if (!strcmp(show->table->name, name)) {
> @@ -1638,7 +1651,8 @@ static void
>  cmd_show_row(struct ctl_context *ctx, const struct ovsdb_idl_row *row,
>               int level)
>  {
> -    struct cmd_show_table *show = cmd_show_find_table_by_row(row);
> +    struct cmd_show_table show_tables = *cmd_show_find_table_by_row(row);
> +    struct cmd_show_table *show = &show_tables;
>      size_t i;
>
>

We have an issue here.  The 'show->recurse' is used to prevent dependency
loop in the users defined 'struct cmd_show_table' array.  So, we cannot copy
the element here.




>      ds_put_char_multiple(&ctx->output, ' ', level * 4);
> @@ -1669,7 +1683,7 @@ cmd_show_row(struct ctl_context *ctx, const struct
> ovsdb_idl_row *row,
>          datum = ovsdb_idl_read(row, column);
>          if (column->type.key.type == OVSDB_TYPE_UUID &&
>              column->type.key.u.uuid.refTableName) {
> -            struct cmd_show_table *ref_show;
> +            const struct cmd_show_table *ref_show;
>              size_t j;
>
>              ref_show = cmd_show_find_table_by_name(
> @@ -1690,7 +1704,7 @@ cmd_show_row(struct ctl_context *ctx, const struct
> ovsdb_idl_row *row,
>          } else if (ovsdb_type_is_map(&column->type) &&
>                     column->type.value.type == OVSDB_TYPE_UUID &&
>                     column->type.value.u.uuid.refTableName) {
> -            struct cmd_show_table *ref_show;
> +            const struct cmd_show_table *ref_show;
>              size_t j;
>
>              /* Prints the key to ref'ed table name map if the ref'ed table
> @@ -1991,9 +2005,11 @@ ctl_register_commands(const struct
> ctl_command_syntax *commands)
>
>  /* Registers the 'db_ctl_commands' to 'all_commands'. */
>  void
> -ctl_init(const struct ctl_table_class tables_[])
> +ctl_init(const struct ctl_table_class tables_[],
> +         const struct cmd_show_table cmd_show_tables_[])
>  {
>      tables = tables_;
> +    cmd_show_tables = cmd_show_tables_;
>      ctl_register_commands(db_ctl_commands);
>  }
>
> diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
> index 684de11..a361b50 100644
> --- a/lib/db-ctl-base.h
> +++ b/lib/db-ctl-base.h
> @@ -35,8 +35,6 @@ struct table;
>   *
>   * - the 'the_idl' and 'the_idl_txn'.
>   *
> - * - the 'cmd_show_tables'.  (See 'struct cmd_show_table' for more info).
> - *
>   * - the command syntaxes for each command.  (See 'struct
> ctl_command_syntax'
>   *   for more info)  and regiters them using ctl_register_commands().
>   *
> @@ -54,7 +52,9 @@ extern struct ovsdb_idl *the_idl;
>  extern struct ovsdb_idl_txn *the_idl_txn;
>
>  struct ctl_table_class;
> -void ctl_init(const struct ctl_table_class *tables);
> +struct cmd_show_table;
> +void ctl_init(const struct ctl_table_class *tables,
> +              const struct cmd_show_table *cmd_show_tables);
>  char *ctl_default_db(void);
>  OVS_NO_RETURN void ctl_exit(int status);
>  OVS_NO_RETURN void ctl_fatal(const char *, ...) OVS_PRINTF_FORMAT(1, 2);
> @@ -176,17 +176,6 @@ struct cmd_show_table {
>      bool recurse;
>  };
>
> -/* This array defines the 'show' command output format.  User can check
> the
> - * definition in utilities/ovs-vsctl.c as reference.
> - *
> - * Particularly, if an element in 'columns[]' represents a reference to
> - * another table, the referred table must also be defined as an entry in
> - * in 'cmd_show_tables[]'.
> - *
> - * The definition must end with an all-NULL entry.
> - * */
> -extern struct cmd_show_table cmd_show_tables[];
> -
>
>  /* The base context struct for conducting the common database
>   * operations (commands listed in 'db_ctl_commands').  User should
> diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
> index 8d62d54..7794106 100644
> --- a/utilities/ovs-vsctl.c
> +++ b/utilities/ovs-vsctl.c
> @@ -970,7 +970,7 @@ cmd_init(struct ctl_context *ctx OVS_UNUSED)
>  {
>  }
>
> -struct cmd_show_table cmd_show_tables[] = {
> +static struct cmd_show_table cmd_show_tables[] = {
>      {&ovsrec_table_open_vswitch,
>       NULL,
>       {&ovsrec_open_vswitch_col_manager_options,
> @@ -2749,6 +2749,6 @@ static const struct ctl_command_syntax
> vsctl_commands[] = {
>  static void
>  vsctl_cmd_init(void)
>  {
> -    ctl_init(tables);
> +    ctl_init(tables, cmd_show_tables);
>      ctl_register_commands(vsctl_commands);
>  }
> diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
> index f065fc9..d9c4b26 100644
> --- a/vtep/vtep-ctl.c
> +++ b/vtep/vtep-ctl.c
> @@ -340,7 +340,7 @@ Other options:\n\
>  }
>
>
> -struct cmd_show_table cmd_show_tables[] = {
> +static struct cmd_show_table cmd_show_tables[] = {
>      {&vteprec_table_global,
>       NULL,
>       {&vteprec_global_col_managers,
> @@ -2310,6 +2310,6 @@ static const struct ctl_command_syntax
> vtep_commands[] = {
>  static void
>  vtep_ctl_cmd_init(void)
>  {
> -    ctl_init(tables);
> +    ctl_init(tables, cmd_show_tables);
>      ctl_register_commands(vtep_commands);
>  }
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list