db-ctl-base: Drop redundant 'table' field from struct ctl_row_id.

The 'table' field is redundant because the required 'column' field implies the table that the column is a part of. This simplifies the users and makes it harder to get these things wrong. Signed-off-by: Ben Pfaff <[email protected]> Acked-by: Andy Zhou <[email protected]>
The-Mule · May 3, 2017 · a0b0289 · a0b0289
1 parent 5d476f2
commit a0b0289
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 66 deletions.
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
@@ -246,7 +246,7 @@ get_row_by_id(struct ctl_context *ctx,
               const struct ctl_row_id *id, const char *record_id)
 {
 
-    if (!id->table || !id->name_column) {
+    if (!id->name_column) {
         return NULL;
     }
 
@@ -262,8 +262,11 @@ get_row_by_id(struct ctl_context *ctx,
         ovs_assert(key == OVSDB_TYPE_STRING);
     }
 
+    const struct ovsdb_idl_class *class = ovsdb_idl_get_class(ctx->idl);
+    const struct ovsdb_idl_table_class *id_table
+        = ovsdb_idl_table_class_from_column(class, id->name_column);
     for (const struct ovsdb_idl_row *row = ovsdb_idl_first_row(ctx->idl,
-                                                               id->table);
+                                                               id_table);
          row != NULL;
          row = ovsdb_idl_next_row(row)) {
         const struct ovsdb_datum *name = ovsdb_idl_get(
@@ -275,7 +278,7 @@ get_row_by_id(struct ctl_context *ctx,
                 : atom->integer == strtoll(record_id, NULL, 10)) {
                 if (referrer) {
                     ctl_fatal("multiple rows in %s match \"%s\"",
-                              table->name, record_id);
+                              id_table->name, record_id);
                 }
                 referrer = row;
             }
@@ -404,9 +407,6 @@ pre_get_table(struct ctl_context *ctx, const char *table_name)
     const struct ctl_table_class *ctl = &ctl_classes[table - idl_classes];
     for (int i = 0; i < ARRAY_SIZE(ctl->row_ids); i++) {
         const struct ctl_row_id *id = &ctl->row_ids[i];
-        if (id->table) {
-            ovsdb_idl_add_table(ctx->idl, id->table);
-        }
         if (id->name_column) {
             ovsdb_idl_add_column(ctx->idl, id->name_column);
         }

diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
@@ -237,8 +237,17 @@ void ctl_context_init(struct ctl_context *, struct ctl_command *,
 void ctl_context_done_command(struct ctl_context *, struct ctl_command *);
 void ctl_context_done(struct ctl_context *, struct ctl_command *);
 
+/* A way to identify a particular row in the database based on a user-provided
+ * string.  If all fields are NULL, the struct is ignored.  Otherwise,
+ * 'name_column' designates a column whose table is searched for rows that
+ * match with the user string.  If a matching row is found, then:
+ *
+ *    - If 'uuid_column' is NULL, the matching row is the final row.
+ *
+ *    - Otherwise 'uuid_column' must designate a UUID-typed column whose value
+ *      refers to exactly one row, which is the final row.
+ */
 struct ctl_row_id {
-    const struct ovsdb_idl_table_class *table;
     const struct ovsdb_idl_column *name_column;
     const struct ovsdb_idl_column *uuid_column;
 };

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
+/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -798,26 +798,48 @@ ovsdb_idl_check_consistency(const struct ovsdb_idl *idl)
     ovs_assert(ok);
 }
 
-static unsigned char *
-ovsdb_idl_get_mode(struct ovsdb_idl *idl,
-                   const struct ovsdb_idl_column *column)
+const struct ovsdb_idl_class *
+ovsdb_idl_get_class(const struct ovsdb_idl *idl)
 {
-    size_t i;
-
-    ovs_assert(!idl->change_seqno);
-
-    for (i = 0; i < idl->class->n_tables; i++) {
-        const struct ovsdb_idl_table *table = &idl->tables[i];
-        const struct ovsdb_idl_table_class *tc = table->class;
+    return idl->class;
+}
 
+/* Given 'column' in some table in 'class', returns the table's class. */
+const struct ovsdb_idl_table_class *
+ovsdb_idl_table_class_from_column(const struct ovsdb_idl_class *class,
+                                  const struct ovsdb_idl_column *column)
+{
+    for (size_t i = 0; i < class->n_tables; i++) {
+        const struct ovsdb_idl_table_class *tc = &class->tables[i];
         if (column >= tc->columns && column < &tc->columns[tc->n_columns]) {
-            return &table->modes[column - tc->columns];
+            return tc;
         }
     }
 
     OVS_NOT_REACHED();
 }
 
+/* Given 'column' in some table in 'idl', returns the table. */
+static struct ovsdb_idl_table *
+ovsdb_idl_table_from_column(struct ovsdb_idl *idl,
+                            const struct ovsdb_idl_column *column)
+{
+    const struct ovsdb_idl_table_class *tc =
+        ovsdb_idl_table_class_from_column(idl->class, column);
+    return &idl->tables[tc - idl->class->tables];
+}
+
+static unsigned char *
+ovsdb_idl_get_mode(struct ovsdb_idl *idl,
+                   const struct ovsdb_idl_column *column)
+{
+    ovs_assert(!idl->change_seqno);
+
+    const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(idl,
+                                                                      column);
+    return &table->modes[column - table->class->columns];
+}
+
 static void
 add_ref_table(struct ovsdb_idl *idl, const struct ovsdb_base_type *base)
 {

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
@@ -77,6 +77,10 @@ int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
 void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int probe_interval);
 
 void ovsdb_idl_check_consistency(const struct ovsdb_idl *);
+
+const struct ovsdb_idl_class *ovsdb_idl_get_class(const struct ovsdb_idl *);
+const struct ovsdb_idl_table_class *ovsdb_idl_table_class_from_column(
+    const struct ovsdb_idl_class *, const struct ovsdb_idl_column *);
 
 /* Choosing columns and tables to replicate. */
 

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
@@ -3045,21 +3045,18 @@ cmd_set_ssl(struct ctl_context *ctx)
 
 static const struct ctl_table_class tables[NBREC_N_TABLES] = {
     [NBREC_TABLE_LOGICAL_SWITCH].row_ids[0]
-    = {&nbrec_table_logical_switch, &nbrec_logical_switch_col_name, NULL},
+    = {&nbrec_logical_switch_col_name, NULL},
 
     [NBREC_TABLE_LOGICAL_SWITCH_PORT].row_ids[0]
-    = {&nbrec_table_logical_switch_port, &nbrec_logical_switch_port_col_name,
-       NULL},
+    = {&nbrec_logical_switch_port_col_name, NULL},
 
     [NBREC_TABLE_LOGICAL_ROUTER].row_ids[0]
-    = {&nbrec_table_logical_router, &nbrec_logical_router_col_name, NULL},
+    = {&nbrec_logical_router_col_name, NULL},
 
     [NBREC_TABLE_LOGICAL_ROUTER_PORT].row_ids[0]
-    = {&nbrec_table_logical_router_port, &nbrec_logical_router_port_col_name,
-       NULL},
+    = {&nbrec_logical_router_port_col_name, NULL},
 
-    [NBREC_TABLE_ADDRESS_SET].row_ids[0]
-    = {&nbrec_table_address_set, &nbrec_address_set_col_name, NULL},
+    [NBREC_TABLE_ADDRESS_SET].row_ids[0] = {&nbrec_address_set_col_name, NULL},
 };
 
 static void

diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
@@ -1050,17 +1050,15 @@ cmd_set_ssl(struct ctl_context *ctx)
 
 
 static const struct ctl_table_class tables[SBREC_N_TABLES] = {
-    [SBREC_TABLE_CHASSIS].row_ids[0] =
-    {&sbrec_table_chassis, &sbrec_chassis_col_name, NULL},
+    [SBREC_TABLE_CHASSIS].row_ids[0] = {&sbrec_chassis_col_name, NULL},
 
     [SBREC_TABLE_PORT_BINDING].row_ids[0] =
-    {&sbrec_table_port_binding, &sbrec_port_binding_col_logical_port, NULL},
+    {&sbrec_port_binding_col_logical_port, NULL},
 
     [SBREC_TABLE_MAC_BINDING].row_ids[0] =
-    {&sbrec_table_mac_binding, &sbrec_mac_binding_col_logical_port, NULL},
+    {&sbrec_mac_binding_col_logical_port, NULL},
 
-    [SBREC_TABLE_ADDRESS_SET].row_ids[0] =
-    {&sbrec_table_address_set, &sbrec_address_set_col_name, NULL},
+    [SBREC_TABLE_ADDRESS_SET].row_ids[0] = {&sbrec_address_set_col_name, NULL},
 };
 
 

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
@@ -2291,51 +2291,39 @@ cmd_get_aa_mapping(struct ctl_context *ctx)
 
 
 static const struct ctl_table_class tables[OVSREC_N_TABLES] = {
-    [OVSREC_TABLE_BRIDGE].row_ids[0]
-    = {&ovsrec_table_bridge, &ovsrec_bridge_col_name, NULL},
+    [OVSREC_TABLE_BRIDGE].row_ids[0] = {&ovsrec_bridge_col_name, NULL},
 
     [OVSREC_TABLE_CONTROLLER].row_ids[0]
-    = {&ovsrec_table_bridge, &ovsrec_bridge_col_name,
-       &ovsrec_bridge_col_controller},
+    = {&ovsrec_bridge_col_name, &ovsrec_bridge_col_controller},
 
-    [OVSREC_TABLE_INTERFACE].row_ids[0]
-    = {&ovsrec_table_interface, &ovsrec_interface_col_name, NULL},
+    [OVSREC_TABLE_INTERFACE].row_ids[0] = {&ovsrec_interface_col_name, NULL},
 
-    [OVSREC_TABLE_MIRROR].row_ids[0]
-    = {&ovsrec_table_mirror, &ovsrec_mirror_col_name, NULL},
+    [OVSREC_TABLE_MIRROR].row_ids[0] = {&ovsrec_mirror_col_name, NULL},
 
-    [OVSREC_TABLE_MANAGER].row_ids[0]
-    = {&ovsrec_table_manager, &ovsrec_manager_col_target, NULL},
+    [OVSREC_TABLE_MANAGER].row_ids[0] = {&ovsrec_manager_col_target, NULL},
 
     [OVSREC_TABLE_NETFLOW].row_ids[0]
-    = {&ovsrec_table_bridge, &ovsrec_bridge_col_name,
-       &ovsrec_bridge_col_netflow},
+    = {&ovsrec_bridge_col_name, &ovsrec_bridge_col_netflow},
 
-    [OVSREC_TABLE_PORT].row_ids[0]
-    = {&ovsrec_table_port, &ovsrec_port_col_name, NULL},
+    [OVSREC_TABLE_PORT].row_ids[0] = {&ovsrec_port_col_name, NULL},
 
     [OVSREC_TABLE_QOS].row_ids[0]
-    = {&ovsrec_table_port, &ovsrec_port_col_name, &ovsrec_port_col_qos},
+    = {&ovsrec_port_col_name, &ovsrec_port_col_qos},
 
     [OVSREC_TABLE_SFLOW].row_ids[0]
-    = {&ovsrec_table_bridge, &ovsrec_bridge_col_name,
-       &ovsrec_bridge_col_sflow},
+    = {&ovsrec_bridge_col_name, &ovsrec_bridge_col_sflow},
 
     [OVSREC_TABLE_FLOW_TABLE].row_ids[0]
-    = {&ovsrec_table_flow_table, &ovsrec_flow_table_col_name, NULL},
+    = {&ovsrec_flow_table_col_name, NULL},
 
-    [OVSREC_TABLE_IPFIX].row_ids[0] =
-     {&ovsrec_table_bridge, &ovsrec_bridge_col_name,
-      &ovsrec_bridge_col_ipfix},
+    [OVSREC_TABLE_IPFIX].row_ids[0]
+    = {&ovsrec_bridge_col_name, &ovsrec_bridge_col_ipfix},
 
     [OVSREC_TABLE_AUTOATTACH].row_ids[0]
-    = {&ovsrec_table_bridge, &ovsrec_bridge_col_name,
-       &ovsrec_bridge_col_auto_attach},
+    = {&ovsrec_bridge_col_name, &ovsrec_bridge_col_auto_attach},
 
     [OVSREC_TABLE_FLOW_SAMPLE_COLLECTOR_SET].row_ids[0]
-    = {&ovsrec_table_flow_sample_collector_set,
-       &ovsrec_flow_sample_collector_set_col_id,
-       NULL},
+    = {&ovsrec_flow_sample_collector_set_col_id, NULL},
 };
 
 static void

diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c
@@ -2178,20 +2178,18 @@ cmd_set_manager(struct ctl_context *ctx)
 /* Parameter commands. */
 static const struct ctl_table_class tables[VTEPREC_N_TABLES] = {
     [VTEPREC_TABLE_LOGICAL_SWITCH].row_ids[0]
-    = {&vteprec_table_logical_switch, &vteprec_logical_switch_col_name, NULL},
+    = {&vteprec_logical_switch_col_name, NULL},
 
-    [VTEPREC_TABLE_MANAGER].row_ids[0]
-    = {&vteprec_table_manager, &vteprec_manager_col_target, NULL},
+    [VTEPREC_TABLE_MANAGER].row_ids[0] = {&vteprec_manager_col_target, NULL},
 
     [VTEPREC_TABLE_PHYSICAL_PORT].row_ids[0]
-    = {&vteprec_table_physical_port, &vteprec_physical_port_col_name, NULL},
+    = {&vteprec_physical_port_col_name, NULL},
 
     [VTEPREC_TABLE_PHYSICAL_SWITCH].row_ids[0]
-    = {&vteprec_table_physical_switch, &vteprec_physical_switch_col_name,
-       NULL},
+    = {&vteprec_physical_switch_col_name, NULL},
 
     [VTEPREC_TABLE_LOGICAL_ROUTER].row_ids[0]
-    = {&vteprec_table_logical_router, &vteprec_logical_router_col_name, NULL},
+    = {&vteprec_logical_router_col_name, NULL},
 };