ovsdb: Implement garbage collection.

2 files changed, 100 insertions, 122 deletions

diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 3b35179ab..ee76b8321 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -478,9 +478,11 @@ A bridge port.  Records may be identified by port name.
 A network device attached to a port.  Records may be identified by
 name.
 .IP "\fBQoS\fR"
-Quality-of-service configuration for an \fBInterface\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.
+Configuration for one queue within a \fBQoS\fR configuration.  Records
+may only be identified by UUID.
 .IP "\fBMonitor\fR"
 Connectivity Monitoring attached to an \fBInterface\fR.  Records may be
 identified by \fBInterface\fR name.
@@ -637,10 +639,30 @@ If \fB@\fIname\fR is specified, then the UUID for the new row may be
 referred to by that name elsewhere in the same \fBovs\-vsctl\fR
 invocation in contexts where a UUID is expected.  Such references may
 precede or follow the \fBcreate\fR command.
+.IP
+Records in the Open vSwitch database are significant only when they
+can be reached directly or indirectly from the \fBOpen_vSwitch\fR
+table.  Except for records in the \fBQoS\fR or \fBQueue\fR tables,
+records that are not reachable from the \fBOpen_vSwitch\fR table are
+automatically deleted from the database.  This deletion happens
+immediately, without waiting for additional \fBovs\-vsctl\fR commands
+or other database activity.  Thus, a \fBcreate\fR command must
+generally be accompanied by additional commands \fIwithin the same
+\fBovs\-vsctl\fI invocation\fR to add a chain of references to the
+newly created record from the top-level \fBOpen_vSwitch\fR record.
+The \fBEXAMPLES\fR section gives some examples that show how to do
+this.
 .
 .IP "\fR[\fB\-\-if\-exists\fR] \fBdestroy \fItable record\fR..."
 Deletes each specified \fIrecord\fR from \fItable\fR.  Unless
 \fB\-\-if\-exists\fR is specified, each \fIrecord\fRs must exist.
+.IP
+It is often unnecessary to specify explicit \fBdestroy\fR commands,
+because (except for records in the \fBQoS\fR or \fBQueue\fR tables)
+records that are not reachable from the \fBOpen_vSwitch\fR table are
+automatically deleted from the database.  This means that deleting the
+last reference to a record is sufficient for deleting the record
+itself.  See the \fBEXAMPLES\fR section below for more information.
 .
 .IP "\fBwait\-until \fItable record \fR[\fIcolumn\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR]..."
 Waits until \fItable\fR contains a record named \fIrecord\fR whose
@@ -720,10 +742,10 @@ ignored):
 .IP
 .B "\-\- \-\-id=@m create Mirror name=mymirror select-dst-port=@eth0,@eth1 select-src-port=@eth0,@eth1 output-port=@eth2"
 .PP
-Remove the mirror created above from \fBbr0\fR and destroy the Mirror
-record (to avoid having an unreferenced record in the database):
+Remove the mirror created above from \fBbr0\fR, which also destroys
+the Mirror record (since it is now unreferenced):
 .IP
-.B "ovs\-vsctl destroy Mirror mymirror \-\- clear Bridge br0 mirrors"
+.B "remove Bridge br0 mirrors mymirror"
 .SS "Quality of Service (QoS)"
 .PP
 Create a \fBlinux\-htb\fR QoS record that points to a few queues and
@@ -744,7 +766,8 @@ Deconfigure the QoS record above from \fBeth1\fR only:
 .B "ovs\-vsctl clear Port eth1 qos"
 .PP
 To deconfigure the QoS record from both \fBeth0\fR and \fBeth1\fR and
-then delete the QoS record:
+then delete the QoS record (which must be done explicitly because
+unreferenced QoS records are not automatically destroyed):
 .IP
 .B "ovs\-vsctl \-\- destroy QoS eth0 \-\- clear Port eth0 qos \-\- clear Port eth1 qos"
 .PP
@@ -781,10 +804,10 @@ instead use an active timeout of 60 seconds:
 .IP
 .B "ovs\-vsctl set NetFlow br0 active_timeout=60"
 .PP
-Deconfigure the NetFlow settings from \fBbr0\fR and delete the NetFlow
-record (to avoid having an unreferenced record in the database):
+Deconfigure the NetFlow settings from \fBbr0\fR, which also destroys
+the NetFlow record (since it is now unreferenced):
 .IP
-.B "ovs\-vsctl destroy NetFlow br0 \-\- clear Bridge br0 netflow"
+.B "ovs\-vsctl clear Bridge br0 netflow"
 .SS "sFlow"
 .PP
 Configure bridge \fBbr0\fR to send sFlow records to a collector on
@@ -795,10 +818,10 @@ with specific sampling parameters:
 .IP
 .B "\-\- set Bridge br0 sflow=@s"
 .PP
-Deconfigure sFlow from br0 and destroy the sFlow record (to avoid
-having an unreferenced record in the database):
+Deconfigure sFlow from br0, which also destroys the sFlow record
+(since it is now unreferenced):
 .IP
-.B "ovs\-vsctl \-\- destroy sFlow br0 \-\- clear Bridge br0 sflow"
+.B "ovs\-vsctl \-\- clear Bridge br0 sflow"
 .SH "EXIT STATUS"
 .IP "0"
 Successful program execution.
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index e5e03f738..80c9048b1 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1035,12 +1035,6 @@ cmd_emer_reset(struct vsctl_context *ctx)
     const struct ovsrec_bridge *br;
     const struct ovsrec_port *port;
     const struct ovsrec_interface *iface;
-    const struct ovsrec_mirror *mirror, *next_mirror;
-    const struct ovsrec_controller *ctrl, *next_ctrl;
-    const struct ovsrec_manager *mgr, *next_mgr;
-    const struct ovsrec_netflow *nf, *next_nf;
-    const struct ovsrec_ssl *ssl, *next_ssl;
-    const struct ovsrec_sflow *sflow, *next_sflow;
 
     /* Reset the Open_vSwitch table. */
     ovsrec_open_vswitch_set_manager_options(ctx->ovs, NULL, 0);
@@ -1084,30 +1078,6 @@ cmd_emer_reset(struct vsctl_context *ctx)
         ovsrec_interface_set_ingress_policing_rate(iface, 0);
         ovsrec_interface_set_ingress_policing_burst(iface, 0);
     }
-
-    OVSREC_MIRROR_FOR_EACH_SAFE (mirror, next_mirror, idl) {
-        ovsrec_mirror_delete(mirror);
-    }
-
-    OVSREC_CONTROLLER_FOR_EACH_SAFE (ctrl, next_ctrl, idl) {
-        ovsrec_controller_delete(ctrl);
-    }
-
-    OVSREC_MANAGER_FOR_EACH_SAFE (mgr, next_mgr, idl) {
-        ovsrec_manager_delete(mgr);
-    }
-
-    OVSREC_NETFLOW_FOR_EACH_SAFE (nf, next_nf, idl) {
-        ovsrec_netflow_delete(nf);
-    }
-
-    OVSREC_SSL_FOR_EACH_SAFE (ssl, next_ssl, idl) {
-        ovsrec_ssl_delete(ssl);
-    }
-
-    OVSREC_SFLOW_FOR_EACH_SAFE (sflow, next_sflow, idl) {
-        ovsrec_sflow_delete(sflow);
-    }
 }
 
 static void
@@ -1218,18 +1188,8 @@ cmd_add_br(struct vsctl_context *ctx)
 }
 
 static void
-del_port(struct vsctl_info *info, struct vsctl_port *port)
+del_port(struct vsctl_port *port)
 {
-    struct shash_node *node;
-
-    SHASH_FOR_EACH (node, &info->ifaces) {
-        struct vsctl_iface *iface = node->data;
-        if (iface->port == port) {
-            ovsrec_interface_delete(iface->iface_cfg);
-        }
-    }
-    ovsrec_port_delete(port->port_cfg);
-
     bridge_delete_port((port->bridge->parent
                         ? port->bridge->parent->br_cfg
                         : port->bridge->br_cfg), port->port_cfg);
@@ -1245,18 +1205,18 @@ cmd_del_br(struct vsctl_context *ctx)
     get_info(ctx, &info);
     bridge = find_bridge(&info, ctx->argv[1], must_exist);
     if (bridge) {
-        struct shash_node *node;
-
-        SHASH_FOR_EACH (node, &info.ports) {
-            struct vsctl_port *port = node->data;
-            if (port->bridge == bridge || port->bridge->parent == bridge
-                || !strcmp(port->port_cfg->name, bridge->name)) {
-                del_port(&info, port);
-            }
-        }
         if (bridge->br_cfg) {
-            ovsrec_bridge_delete(bridge->br_cfg);
             ovs_delete_bridge(ctx->ovs, bridge->br_cfg);
+        } else {
+            struct shash_node *node;
+
+            SHASH_FOR_EACH (node, &info.ports) {
+                struct vsctl_port *port = node->data;
+                if (port->bridge == bridge || port->bridge->parent == bridge
+                    || !strcmp(port->port_cfg->name, bridge->name)) {
+                    del_port(port);
+                }
+            }
         }
     }
     free_info(&info);
@@ -1641,7 +1601,7 @@ cmd_del_port(struct vsctl_context *ctx)
             }
         }
 
-        del_port(&info, port);
+        del_port(port);
     }
 
     free_info(&info);
@@ -1774,30 +1734,15 @@ cmd_get_controller(struct vsctl_context *ctx)
 }
 
 static void
-delete_controllers(struct ovsrec_controller **controllers,
-                   size_t n_controllers)
-{
-    size_t i;
-
-    for (i = 0; i < n_controllers; i++) {
-        ovsrec_controller_delete(controllers[i]);
-    }
-}
-
-static void
 cmd_del_controller(struct vsctl_context *ctx)
 {
     struct vsctl_info info;
     struct vsctl_bridge *br;
 
     get_info(ctx, &info);
-    br = find_real_bridge(&info, ctx->argv[1], true);
-    verify_controllers(br->br_cfg);
 
-    if (br->ctrl) {
-        delete_controllers(br->ctrl, br->n_ctrl);
-        ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
-    }
+    br = find_real_bridge(&info, ctx->argv[1], true);
+    ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
 
     free_info(&info);
 }
@@ -1827,9 +1772,6 @@ cmd_set_controller(struct vsctl_context *ctx)
 
     get_info(ctx, &info);
     br = find_real_bridge(&info, ctx->argv[1], true);
-    verify_controllers(br->br_cfg);
-
-    delete_controllers(br->ctrl, br->n_ctrl);
 
     n = ctx->argc - 2;
     controllers = insert_controllers(ctx->txn, &ctx->argv[2], n);
@@ -1936,27 +1878,11 @@ cmd_get_manager(struct vsctl_context *ctx)
 }
 
 static void
-delete_managers(const struct vsctl_context *ctx)
-{
-    const struct ovsrec_open_vswitch *ovs = ctx->ovs;
-    size_t i;
-
-    /* Delete Manager rows pointed to by 'manager_options' column. */
-    for (i = 0; i < ovs->n_manager_options; i++) {
-        ovsrec_manager_delete(ovs->manager_options[i]);
-    }
-
-    /* Delete 'Manager' row refs in 'manager_options' column. */
-    ovsrec_open_vswitch_set_manager_options(ovs, NULL, 0);
-}
-
-static void
 cmd_del_manager(struct vsctl_context *ctx)
 {
     const struct ovsrec_open_vswitch *ovs = ctx->ovs;
 
-    verify_managers(ovs);
-    delete_managers(ctx);
+    ovsrec_open_vswitch_set_manager_options(ovs, NULL, 0);
 }
 
 static void
@@ -1982,8 +1908,6 @@ cmd_set_manager(struct vsctl_context *ctx)
 {
     const size_t n = ctx->argc - 1;
 
-    verify_managers(ctx->ovs);
-    delete_managers(ctx);
     insert_managers(ctx, &ctx->argv[1], n);
 }
 
@@ -2027,13 +1951,7 @@ pre_cmd_del_ssl(struct vsctl_context *ctx)
 static void
 cmd_del_ssl(struct vsctl_context *ctx)
 {
-    struct ovsrec_ssl *ssl = ctx->ovs->ssl;
-
-    if (ssl) {
-        ovsrec_open_vswitch_verify_ssl(ctx->ovs);
-        ovsrec_ssl_delete(ssl);
-        ovsrec_open_vswitch_set_ssl(ctx->ovs, NULL);
-    }
+    ovsrec_open_vswitch_set_ssl(ctx->ovs, NULL);
 }
 
 static void
@@ -2046,12 +1964,8 @@ static void
 cmd_set_ssl(struct vsctl_context *ctx)
 {
     bool bootstrap = shash_find(&ctx->options, "--bootstrap");
-    struct ovsrec_ssl *ssl = ctx->ovs->ssl;
+    struct ovsrec_ssl *ssl;
 
-    ovsrec_open_vswitch_verify_ssl(ctx->ovs);
-    if (ssl) {
-        ovsrec_ssl_delete(ssl);
-    }
     ssl = ovsrec_ssl_insert(ctx->txn);
 
     ovsrec_ssl_set_private_key(ssl, ctx->argv[1]);
@@ -2351,7 +2265,7 @@ get_column(const struct vsctl_table_class *table, const char *column_name,
     }
 }
 
-static struct uuid *
+static struct ovsdb_symbol *
 create_symbol(struct ovsdb_symbol_table *symtab, const char *id, bool *newp)
 {
     struct ovsdb_symbol *symbol;
@@ -2370,7 +2284,7 @@ create_symbol(struct ovsdb_symbol_table *symtab, const char *id, bool *newp)
                     id);
     }
     symbol->created = true;
-    return &symbol->uuid;
+    return symbol;
 }
 
 static void
@@ -2578,13 +2492,19 @@ cmd_get(struct vsctl_context *ctx)
     table = get_table(table_name);
     row = must_get_row(ctx, table, record_id);
     if (id) {
+        struct ovsdb_symbol *symbol;
         bool new;
 
-        *create_symbol(ctx->symtab, id, &new) = row->uuid;
+        symbol = create_symbol(ctx->symtab, id, &new);
         if (!new) {
             vsctl_fatal("row id \"%s\" specified on \"get\" command was used "
                         "before it was defined", id);
         }
+        symbol->uuid = row->uuid;
+
+        /* This symbol refers to a row that already exists, so disable warnings
+         * about it being unreferenced. */
+        symbol->strong_ref = true;
     }
     for (i = 3; i < ctx->argc; i++) {
         const struct ovsdb_idl_column *column;
@@ -3093,18 +3013,42 @@ cmd_clear(struct vsctl_context *ctx)
 }
 
 static void
-cmd_create(struct vsctl_context *ctx)
+pre_create(struct vsctl_context *ctx)
 {
     const char *id = shash_find_data(&ctx->options, "--id");
     const char *table_name = ctx->argv[1];
     const struct vsctl_table_class *table;
+
+    table = get_table(table_name);
+    if (!id && !table->class->is_root) {
+        VLOG_WARN("applying \"create\" command to table %s without --id "
+                  "option will have no effect", table->class->name);
+    }
+}
+
+static void
+cmd_create(struct vsctl_context *ctx)
+{
+    const char *id = shash_find_data(&ctx->options, "--id");
+    const char *table_name = ctx->argv[1];
+    const struct vsctl_table_class *table = get_table(table_name);
     const struct ovsdb_idl_row *row;
     const struct uuid *uuid;
     int i;
 
-    uuid = id ? create_symbol(ctx->symtab, id, NULL) : NULL;
+    if (id) {
+        struct ovsdb_symbol *symbol = create_symbol(ctx->symtab, id, NULL);
+        if (table->class->is_root) {
+            /* This table is in the root set, meaning that rows created in it
+             * won't disappear even if they are unreferenced, so disable
+             * warnings about that by pretending that there is a reference. */
+            symbol->strong_ref = true;
+        }
+        uuid = &symbol->uuid;
+    } else {
+        uuid = NULL;
+    }
 
-    table = get_table(table_name);
     row = ovsdb_idl_txn_insert(ctx->txn, table->class, uuid);
     for (i = 2; i < ctx->argc; i++) {
         set_column(table, row, ctx->argv[i], ctx->symtab);
@@ -3402,6 +3346,17 @@ do_vsctl(const char *args, struct vsctl_command *commands, size_t n_commands,
                         "with \"-- --id=%s create ...\")",
                         node->name, node->name);
         }
+        if (!symbol->strong_ref) {
+            if (!symbol->weak_ref) {
+                VLOG_WARN("row id \"%s\" was created but no reference to it "
+                          "was inserted, so it will not actually appear in "
+                          "the database", node->name);
+            } else {
+                VLOG_WARN("row id \"%s\" was created but only a weak "
+                          "reference to it was inserted, so it will not "
+                          "actually appear in the database", node->name);
+            }
+        }
     }
 
     status = ovsdb_idl_txn_commit_block(txn);
@@ -3577,7 +3532,7 @@ static const struct vsctl_command_syntax all_commands[] = {
     {"add", 4, INT_MAX, pre_cmd_add, cmd_add, NULL, "", RW},
     {"remove", 4, INT_MAX, pre_cmd_remove, cmd_remove, NULL, "", RW},
     {"clear", 3, INT_MAX, pre_cmd_clear, cmd_clear, NULL, "", RW},
-    {"create", 2, INT_MAX, NULL, cmd_create, post_create, "--id=", RW},
+    {"create", 2, INT_MAX, pre_create, cmd_create, post_create, "--id=", RW},
     {"destroy", 1, INT_MAX, pre_cmd_destroy, cmd_destroy, NULL, "--if-exists",
      RW},
     {"wait-until", 2, INT_MAX, pre_cmd_wait_until, cmd_wait_until, NULL, "",