[ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages

Mark Michelson mmichels at redhat.com
Mon Feb 26 14:30:45 UTC 2018


On 02/23/2018 06:07 PM, Ben Pfaff wrote:
> On Thu, Feb 15, 2018 at 04:49:57PM -0600, Mark Michelson wrote:
>> The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
>> their "Database Commands" section when it comes to referring to what
>> database tables exist. This commit amends this by making each *ctl
>> manpage reference the corresponding database manpage instead.
>>
>> To aid in having a more handy list, the --help text of ovn-nbctl,
>> ovn-sbctl, and ovs-vsctl have been modified to list the available
>> tables. This is also referenced in the manpages for those applications.
>>
>> Signed-off-by: Mark Michelson <mmichels at redhat.com>
> 
> Thanks.  I was hoping for more explanation in the list of tables for how
> users can refer to the tables.  Here is a version that adds more
> information.  What do you think?

Thanks for the update Ben. I like listing methods of referring to 
records in each table. I found the formatting to be a bit overwhelming 
in some cases. For instance, from `ovn-nbctl -h` the DHCP_Options table 
has this text:

   DHCP_Options: by UUID, via "dhcpv4_options" of Logical_Switch_Port 
with matching "name", via "dhcpv4_options" of Logical_Switch_Port with 
matching "external_ids:neutron:port_name", via "dhcpv6_options" of 
Logical_Switch_Port with matching "name", or via "dhcpv6_options" of 
Logical_Switch_Port with matching "external_ids:neutron:port_name"

Keep in mind that there is no column-limiting on this, so it stretches 
across the entire terminal. It makes it hard to visually parse the 
options. I made a adjustment to the formatting so that it looks more 
like this:

   DHCP_Options:
     by UUID
     via "dhcpv4_options" of Logical_Switch_Port with matching "name"
     via "dhcpv4_options" of Logical_Switch_Port with matching 
"external_ids:neutron:port_name"
     via "dhcpv6_options" of Logical_Switch_Port with matching "name"
     via "dhcpv6_options" of Logical_Switch_Port with matching 
"external_ids:neutron:port_name"

Separating the methods by line makes it easier to read IMHO. Here's the 
change

  --8<--------------------------cut here-------------------------->8--

From: Mark Michelson <mmichels at redhat.com>
Date: Mon, 26 Feb 2018 08:28:49 -0600
Subject: [PATCH] Refer to database manpages in *ctl manpages

The ovn-nbctl, ovn-sbctl, and ovs-vsctl manpages are inconsistent in
their "Database Commands" section when it comes to referring to what
database tables exist. This commit amends this by making each *ctl
manpage reference the corresponding database manpage instead.

To aid in having a more handy list, the --help text of ovn-nbctl,
ovn-sbctl, and ovs-vsctl have been modified to list the available
tables. This is also referenced in the manpages for those applications.

Signed-off-by: Mark Michelson <mmichels at redhat.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Please enter the commit message for your changes. Lines starting
---
  lib/db-ctl-base.c             | 69 
++++++++++++++++++++++++++++++++++++++++---
  lib/db-ctl-base.h             | 14 ++++-----
  ovn/lib/ovn-util.h            |  5 ++++
  ovn/utilities/ovn-nbctl.8.xml | 61 +++-----------------------------------
  ovn/utilities/ovn-nbctl.c     |  5 ++--
  ovn/utilities/ovn-sbctl.8.in  |  5 ++--
  ovn/utilities/ovn-sbctl.c     |  6 ++--
  utilities/ovs-vsctl.8.in      | 51 ++------------------------------
  utilities/ovs-vsctl.c         |  7 +++--
  vtep/vtep-ctl.c               |  7 +++--
  10 files changed, 103 insertions(+), 127 deletions(-)

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 9fec6fa0d..ae7b6cde2 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -35,6 +35,7 @@
  #include "ovsdb-idl-provider.h"
  #include "openvswitch/shash.h"
  #include "sset.h"
+#include "svec.h"
  #include "string.h"
  #include "table.h"
  #include "util.h"
@@ -61,6 +62,9 @@ static const struct cmd_show_table *cmd_show_tables;
  static void (*ctl_exit_func)(int status) = NULL;
  OVS_NO_RETURN static void ctl_exit(int status);

+/* IDL class. */
+static const struct ovsdb_idl_class *idl_class;
+
  /* Two arrays with 'n_classes' elements, which represent the tables in 
this
   * database and how the user can refer to their rows. */
  static const struct ctl_table_class *ctl_classes;
@@ -2132,15 +2136,15 @@ ctl_register_commands(const struct 
ctl_command_syntax *commands)

  /* Registers the 'db_ctl_commands' to 'all_commands'. */
  void
-ctl_init__(const struct ovsdb_idl_table_class *idl_classes_,
+ctl_init__(const struct ovsdb_idl_class *idl_class_,
             const struct ctl_table_class *ctl_classes_,
-           size_t n_classes_,
             const struct cmd_show_table cmd_show_tables_[],
             void (*ctl_exit_func_)(int status))
  {
-    idl_classes = idl_classes_;
+    idl_class = idl_class_;
+    idl_classes = idl_class_->tables;
      ctl_classes = ctl_classes_;
-    n_classes = n_classes_;
+    n_classes = idl_class->n_tables;
      ctl_exit_func = ctl_exit_func_;
      ctl_register_commands(db_ctl_commands);

@@ -2170,6 +2174,63 @@ ctl_get_db_cmd_usage(void)
  Potentially unsafe database commands require --force option.\n";
  }

+const char *
+ctl_list_db_tables_usage(void)
+{
+    static struct ds s = DS_EMPTY_INITIALIZER;
+    if (s.length) {
+        return ds_cstr(&s);
+    }
+
+    ds_put_cstr(&s, "Database commands may reference a row in each 
table in the following ways:\n");
+    for (int i = 0; i < n_classes; i++) {
+        struct svec options = SVEC_EMPTY_INITIALIZER;
+
+        svec_add(&options, "by UUID");
+        if (idl_classes[i].is_singleton) {
+            svec_add(&options, "as \".\"");
+        }
+
+        for (int j = 0; j < ARRAY_SIZE(ctl_classes[i].row_ids); j++) {
+            const struct ctl_row_id *id = &ctl_classes[i].row_ids[j];
+            if (!id->name_column) {
+                continue;
+            }
+
+            struct ds o = DS_EMPTY_INITIALIZER;
+            if (id->uuid_column) {
+                ds_put_format(&o, "via \"%s\"", id->uuid_column->name);
+                const struct ovsdb_idl_table_class *referrer
+                    = ovsdb_idl_table_class_from_column(idl_class,
+                                                        id->uuid_column);
+                if (referrer != &idl_classes[i]) {
+                    ds_put_format(&o, " of %s", referrer->name);
+                }
+                if (id->key) {
+                    ds_put_format(&o, " with matching \"%s:%s\"",
+                                  id->name_column->name, id->key);
+                } else {
+                    ds_put_format(&o, " with matching \"%s\"", 
id->name_column->name);
+                }
+            } else if (id->key) {
+                ds_put_format(&o, "by \"%s:%s\"", 
id->name_column->name, id->key);
+            } else {
+                ds_put_format(&o, "by \"%s\"", id->name_column->name);
+            }
+            svec_add_nocopy(&options, ds_steal_cstr(&o));
+        }
+
+        ds_put_format(&s, "  %s:", idl_classes[i].name);
+        for (int j = 0; j < options.n; j++) {
+            ds_put_format(&s, "\n    %s", options.names[j]);
+        }
+        ds_put_char(&s, '\n');
+        svec_destroy(&options);
+    }
+
+    return ds_cstr(&s);
+}
+
  /* Initializes 'ctx' from 'command'. */
  void
  ctl_context_init_command(struct ctl_context *ctx,
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index 81f0d0b27..a2f91abfb 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -51,13 +51,11 @@ struct cmd_show_table;

  /* ctl_init() figures out the number of tables on its own and flags an 
error if
   * 'ctl_classes' was defined with the wrong number of elements. */
-#define ctl_init(idl_classes, ctl_classes, cmd_show_table, ctl_exit_func) \
-    (BUILD_ASSERT(ARRAY_SIZE(idl_classes) == ARRAY_SIZE(ctl_classes)),  \
-     ctl_init__(idl_classes, ctl_classes, ARRAY_SIZE(idl_classes),      \
-                cmd_show_table, ctl_exit_func))
-void ctl_init__(const struct ovsdb_idl_table_class *idl_classes,
-                const struct ctl_table_class *ctl_classes,
-                size_t n_classes,
+#define ctl_init(idl_class, table_classes, ctl_classes, cmd_show_table, \
+                 ctl_exit_func)                                         \
+    (BUILD_ASSERT(ARRAY_SIZE(table_classes) == ARRAY_SIZE(ctl_classes)),  \
+     ctl_init__(idl_class, ctl_classes, cmd_show_table, ctl_exit_func))
+void ctl_init__(const struct ovsdb_idl_class *, const struct 
ctl_table_class *,
                  const struct cmd_show_table *cmd_show_tables,
                  void (*ctl_exit_func)(int status));
  char *ctl_default_db(void);
@@ -159,6 +157,8 @@ struct ctl_command {

  bool ctl_might_write_to_db(char **argv);
  const char *ctl_get_db_cmd_usage(void);
+
+const char *ctl_list_db_tables_usage(void);
  void ctl_print_commands(void);
  void ctl_print_options(const struct option *);
  void ctl_add_cmd_options(struct option **, size_t *n_options_p,
diff --git a/ovn/lib/ovn-util.h b/ovn/lib/ovn-util.h
index 7ff9f9a1c..6d5e1dfb5 100644
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -68,6 +68,11 @@ char *alloc_nat_zone_key(const struct uuid *key, 
const char *type);
  const char *default_nb_db(void);
  const char *default_sb_db(void);

+struct ovsdb_idl_table_class;
+const char *db_table_usage(struct ds *tables,
+                           const struct ovsdb_idl_table_class *class,
+                           int n_tables);
+
  bool ovn_is_known_nb_lsp_type(const char *type);

  uint32_t sbrec_logical_flow_hash(const struct sbrec_logical_flow *);
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 4d0aea963..7f4b3aba8 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -820,64 +820,11 @@
      additional ways to identify records.  Some commands also take
      <var>column</var> parameters that identify a particular field 
within the
      records in a table.</p>
-    <p>The following tables are currently defined:</p>
-    <dl>
-      <dt><code>Logical_Switch</code></dt>
-      <dd>
-        An L2 logical switch.  Records may be identified by name.
-      </dd>
-
-      <dt><code>Logical_Switch_Port</code></dt>
-      <dd>
-        A port within an L2 logical switch.  Records may be identified 
by name.
-      </dd>
-
-      <dt><code>ACL</code></dt>
-      <dd>
-        An ACL rule for a logical switch that points to it through its 
<var>acls</var> column.
-      </dd>
-
-      <dt><code>Logical_Router</code></dt>
-      <dd>
-        An L3 logical router.  Records may be identified by name.
-      </dd>
-
-      <dt><code>Logical_Router_Port</code></dt>
-      <dd>
-        A port within an L3 logical router.  Records may be identified 
by name.
-      </dd>
-
-      <dt><code>Logical_Router_Static_Route</code></dt>
-      <dd>
-        A static route belonging to an L3 logical router.
-      </dd>

-      <dt><code>Address_Set</code></dt>
-      <dd>
-        An address set that can be used in ACLs.
-      </dd>
-
-      <dt><code>Load_Balancer</code></dt>
-      <dd>
-        A load balancer for a logical switch that points to it through 
its <var>load_balancer</var> column.
-      </dd>
-
-      <dt><code>NAT</code></dt>
-      <dd>
-        A NAT rule for a Gateway router.
-      </dd>
-
-      <dt><code>DHCP_Options</code></dt>
-      <dd>
-        DHCP options.
-      </dd>
-
-      <dt><code>NB_Global</code></dt>
-      <dd>
-        North bound global configurations.
-      </dd>
-
-    </dl>
+    <p>
+      For a list of tables and their columns, see <code>ovn-nb</code>(5) or
+      see the table listing from the <code>--help</code> option.
+    </p>

      <p>
        Record names must be specified in full and with correct 
capitalization,
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index b3f60bdb7..80fb97cd4 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -455,6 +455,7 @@ SSL commands:\n\
  set the SSL configuration\n\
  \n\
  %s\
+%s\
  \n\
  Synchronization command (use with --wait=sb|hv):\n\
    sync                     wait even for earlier changes to take effect\n\
@@ -469,7 +470,7 @@ Options:\n\
    --dry-run                   do not commit changes to database\n\
    --oneline                   print exactly one line of output per 
command\n",
             program_name, program_name, ctl_get_db_cmd_usage(),
-           default_nb_db());
+           ctl_list_db_tables_usage(), default_nb_db());
      table_usage();
      vlog_usage();
      printf("\
@@ -4081,6 +4082,6 @@ static const struct ctl_command_syntax 
nbctl_commands[] = {
  static void
  nbctl_cmd_init(void)
  {
-    ctl_init(nbrec_table_classes, tables, NULL, nbctl_exit);
+    ctl_init(&nbrec_idl_class, nbrec_table_classes, tables, NULL, 
nbctl_exit);
      ctl_register_commands(nbctl_commands);
  }
diff --git a/ovn/utilities/ovn-sbctl.8.in b/ovn/utilities/ovn-sbctl.8.in
index cd43cf3be..c59a9f542 100644
--- a/ovn/utilities/ovn-sbctl.8.in
+++ b/ovn/utilities/ovn-sbctl.8.in
@@ -278,8 +278,9 @@ parameter may be the UUID for a record, and many 
tables offer
  additional ways to identify records.  Some commands also take
  \fIcolumn\fR parameters that identify a particular field within the
  records in a table.
-.\" It would be kind to list all the tables and their supported identifiers
-.\" here.
+.PP
+For a list of tables and their columns, see \fBovn\-sb\fR(5) or
+see the table listing from the \fB--help\fR option.
  .PP
  Record names must be specified in full and with correct
  capitalization, except that UUIDs may be abbreviated to their first 4
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index 5764ee336..c2fd18338 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -326,6 +326,7 @@ SSL commands:\n\
  set the SSL configuration\n\
  \n\
  %s\
+%s\
  \n\
  Options:\n\
    --db=DATABASE               connect to DATABASE\n\
@@ -334,7 +335,7 @@ Options:\n\
    --dry-run                   do not commit changes to database\n\
    --oneline                   print exactly one line of output per 
command\n",
             program_name, program_name, ctl_get_db_cmd_usage(),
-           default_sb_db());
+           ctl_list_db_tables_usage(), default_sb_db());
      table_usage();
      vlog_usage();
      printf("\
@@ -1442,6 +1443,7 @@ static const struct ctl_command_syntax 
sbctl_commands[] = {
  static void
  sbctl_cmd_init(void)
  {
-    ctl_init(sbrec_table_classes, tables, cmd_show_tables, sbctl_exit);
+    ctl_init(&sbrec_idl_class, sbrec_table_classes, tables,
+             cmd_show_tables, sbctl_exit);
      ctl_register_commands(sbctl_commands);
  }
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 34f41e4e8..b18782c31 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -530,55 +530,8 @@ additional ways to identify records.  Some commands 
also take
  \fIcolumn\fR parameters that identify a particular field within the
  records in a table.
  .PP
-The following tables are currently defined:
-.IP "\fBOpen_vSwitch\fR"
-Global configuration for an \fBovs\-vswitchd\fR.  This table contains
-exactly one record, identified by specifying \fB.\fR as the record
-name.
-.IP "\fBBridge\fR"
-Configuration for a bridge within an Open vSwitch.  Records may be
-identified by bridge name.
-.IP "\fBPort\fR"
-A bridge port.  Records may be identified by port name.
-.IP "\fBInterface\fR"
-A network device attached to a port.  Records may be identified by
-name.
-.IP "\fBFlow_Table\fR"
-Configuration for a particular OpenFlow flow table.  Records may be
-identified by name.
-.IP "\fBQoS\fR"
-Quality-of-service configuration for a \fBPort\fR.  Records may be
-identified by port name.
-.IP "\fBQueue\fR"
-Configuration for one queue within a \fBQoS\fR configuration.  Records
-may only be identified by UUID.
-.IP "\fBMirror\fR"
-A port mirroring configuration attached to a bridge.  Records may be
-identified by mirror name.
-.IP "\fBController\fR"
-Configuration for an OpenFlow controller.  A controller attached to a
-particular bridge may be identified by the bridge's name.
-.IP "\fBManager\fR"
-Configuration for an OVSDB connection.  Records may be identified
-by target (e.g. \fBtcp:1.2.3.4\fR).
-.IP "\fBNetFlow\fR"
-A NetFlow configuration attached to a bridge.  Records may be
-identified by bridge name.
-.IP "\fBSSL\fR"
-The global SSL configuration for \fBovs\-vswitchd\fR.  The record
-attached to the \fBOpen_vSwitch\fR table may be identified by
-specifying \fB.\fR as the record name.
-.IP "\fBsFlow\fR"
-An sFlow exporter configuration attached to a bridge.  Records may be
-identified by bridge name.
-.IP "\fBIPFIX\fR"
-An IPFIX exporter configuration attached to a bridge.  Records may be
-identified by bridge name.
-.IP "\fBFlow_Sample_Collector_Set\fR"
-An IPFIX exporter configuration attached to a bridge for sampling
-packets on a per-flow basis using OpenFlow \fBsample\fR actions.
-.IP "\fBAutoAttach\fR"
-Configuration for Auto Attach within a bridge.
+For a list of tables and their columns, see 
\fBovs-vswitchd.conf.db\fR(5) or
+see the table listing from the \fB--help\fR option.
  .PP
  Record names must be specified in full and with correct
  capitalization, except that UUIDs may be abbreviated to their first 4
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index e22baeb28..21fa18ddb 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -416,6 +416,7 @@ Switch commands:\n\
    emer-reset                  reset switch to known good state\n\
  \n\
  %s\
+%s\
  \n\
  Options:\n\
    --db=DATABASE               connect to DATABASE\n\
@@ -425,7 +426,8 @@ Options:\n\
    -t, --timeout=SECS          wait at most SECS seconds for 
ovs-vswitchd\n\
    --dry-run                   do not commit changes to database\n\
    --oneline                   print exactly one line of output per 
command\n",
-           program_name, program_name, ctl_get_db_cmd_usage(), 
ctl_default_db());
+           program_name, program_name, ctl_get_db_cmd_usage(),
+           ctl_list_db_tables_usage(), ctl_default_db());
      table_usage();
      vlog_usage();
      printf("\
@@ -2877,6 +2879,7 @@ static const struct ctl_command_syntax 
vsctl_commands[] = {
  static void
  vsctl_cmd_init(void)
  {
-    ctl_init(ovsrec_table_classes, tables, cmd_show_tables, vsctl_exit);
+    ctl_init(&ovsrec_idl_class, ovsrec_table_classes, tables, 
cmd_show_tables,
+             vsctl_exit);
      ctl_register_commands(vsctl_commands);
  }
diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
index 056dc687a..a8967ea1c 100644
--- a/vtep/vtep-ctl.c
+++ b/vtep/vtep-ctl.c
@@ -361,6 +361,7 @@ MAC binding commands:\n\
    list-remote-macs LS                 list remote mac entries\n\
  \n\
  %s\
+%s\
  \n\
  Options:\n\
    --db=DATABASE               connect to DATABASE\n\
@@ -368,7 +369,8 @@ Options:\n\
    -t, --timeout=SECS          wait at most SECS seconds\n\
    --dry-run                   do not commit changes to database\n\
    --oneline                   print exactly one line of output per 
command\n",
-           program_name, program_name, ctl_get_db_cmd_usage(), 
ctl_default_db());
+           program_name, program_name, ctl_get_db_cmd_usage(),
+           ctl_list_db_tables_usage(), ctl_default_db());
      table_usage();
      vlog_usage();
      printf("\
@@ -2499,6 +2501,7 @@ static const struct ctl_command_syntax 
vtep_commands[] = {
  static void
  vtep_ctl_cmd_init(void)
  {
-    ctl_init(vteprec_table_classes, tables, cmd_show_tables, 
vtep_ctl_exit);
+    ctl_init(&vteprec_idl_class, vteprec_table_classes, tables,
+             cmd_show_tables, vtep_ctl_exit);
      ctl_register_commands(vtep_commands);
  }
-- 
2.13.6


More information about the dev mailing list