Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
On Mon, Feb 26, 2018 at 02:04:02PM -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 > Signed-off-by: Ben Pfaff Thanks, applied to master. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
On 02/26/2018 01:54 PM, Ben Pfaff wrote: On Mon, Feb 26, 2018 at 08:30:45AM -0600, Mark Michelson wrote: 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 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 Thanks. This came through word-wrapped, though. Can you re-send? Thanks, Ben. Sorry about that. I sent out a v2 of the patch: https://patchwork.ozlabs.org/patch/878063/ Mark! ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
On Mon, Feb 26, 2018 at 08:30:45AM -0600, Mark Michelson wrote: > 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 > > > >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 Thanks. This came through word-wrapped, though. Can you re-send? Thanks, Ben. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
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
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
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
Signed-off-by: Ben Pfaff
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
Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
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
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?
--8<--cut here-->8--
From: Mark Michelson
Date: Thu, 15 Feb 2018 16:49:57 -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
Signed-off-by: Ben Pfaff
---
lib/db-ctl-base.c | 71 ---
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, 105 insertions(+), 127 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 9fec6fa0d59e..763014df5bc1 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,65 @@ 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\
Re: [ovs-dev] [PATCH v2] Refer to database manpages in *ctl manpages
Sorry, but I realized I provided no information on the changes between
versions here.
Version 2 changes:
* Rebased against current master
* Added database table listing to ovn-nbctl, ovn-sbctl, and ovs-vsctl's
--help text.
* Added information to the manpages saying that database tables are
listed in the --help text.
On 02/15/2018 04:49 PM, 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
---
lib/db-ctl-base.c | 14 ++
lib/db-ctl-base.h | 3 +++
ovn/lib/ovn-util.h| 5
ovn/utilities/ovn-nbctl.8.xml | 61 +++
ovn/utilities/ovn-nbctl.c | 10 +++
ovn/utilities/ovn-sbctl.8.in | 5 ++--
ovn/utilities/ovn-sbctl.c | 10 +++
utilities/ovs-vsctl.8.in | 51 ++--
utilities/ovs-vsctl.c | 12 -
9 files changed, 62 insertions(+), 109 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index 9fec6fa0d..4a9ae3286 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -2170,6 +2170,20 @@ ctl_get_db_cmd_usage(void)
Potentially unsafe database commands require --force option.\n";
}
+const char *
+ctl_list_db_tables_usage(struct ds *tables, const struct ovsdb_idl_table_class
*class,
+ int n_tables)
+{
+if (!tables->length) {
+ds_put_cstr(tables, "Valid tables for this database are:\n");
+for (int i = 0; i < n_tables; i++) {
+ds_put_format(tables, " %s\n", class[i].name);
+}
+}
+
+return ds_cstr(tables);
+}
+
/* 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..71fb123c5 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -159,6 +159,9 @@ 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(struct ds *tables,
+const struct ovsdb_idl_table_class *class, int n_tables);
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 9b456426d..e7e84b98e 100644
--- a/ovn/lib/ovn-util.h
+++ b/ovn/lib/ovn-util.h
@@ -67,6 +67,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);
#endif
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
column parameters that identify a particular field within the
records in a table.
-The following tables are currently defined:
-
- Logical_Switch
-
-An L2 logical switch. Records may be identified by name.
-
-
- Logical_Switch_Port
-
-A port within an L2 logical switch. Records may be identified by name.
-
-
- ACL
-
-An ACL rule for a logical switch that points to it through its
acls column.
-
-
- Logical_Router
-
-An L3 logical router. Records may be identified by name.
-
-
- Logical_Router_Port
-
-A port within an L3 logical router. Records may be identified by name.
-
-
- Logical_Router_Static_Route
-
-A static route belonging to an L3 logical router.
-
- Address_Set
-
-An address set that can be used in ACLs.
-
-
- Load_Balancer
-
-A load balancer for a logical switch that points to it through its
load_balancer column.
-
-
- NAT
-
-A NAT rule for a Gateway router.
-
-
- DHCP_Options
-
-DHCP options.
-
-
- NB_Global
-
-North bound global configurations.
-
-
-
+
+ For a list of tables and their columns, see ovn-nb(5) or
+ see the table listing from the --help option.
+
Record names must be specified in full and with c
