[ovs-dev] [PATCH] ovs-vsctl: Back out garbage collection changes.

Ethan Jackson ethan at nicira.com
Wed Mar 16 23:52:49 UTC 2011


Garbage collection introduced in
c5f341ab193b9126dffef8c77bf8ed35e91290fd changed ovs-vsctl so that
it would allow the garbage collector to reclaim unused tables
instead of manually deleting them itself.  Since garbage collection
runs at transaction completion, undeleted tables would hang around
and could conflict with future actions in a given transaction.
This commit backs out this change.

The following command is an example of something that would have
failed before this commit.

ovs-vsctl -- add-br b \
-- del-br b           \
-- add-br b           \
-- set Interface b other_config:test=test
---
 tests/ovs-vsctl.at    |   20 ++++++++
 utilities/ovs-vsctl.c |  119 ++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 123 insertions(+), 16 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 2e8af85..77911fa 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -190,6 +190,26 @@ CHECK_IFACES([b])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
+AT_SETUP([add-br a, del-br a, add-br a])
+AT_KEYWORDS([ovs-vsctl])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
+  [add-br a],
+  [del-br a],
+  [add-br a],
+  [set Interface a other_config:key=value],
+  [get Interface a other_config:key])], [0], [
+
+
+
+value
+], [], [OVS_VSCTL_CLEANUP])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+CHECK_IFACES([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
 AT_SETUP([add-br a, add-port a a1, add-port a a2])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 80c9048..2baab00 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1035,6 +1035,12 @@ 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);
@@ -1078,6 +1084,30 @@ 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
@@ -1188,8 +1218,18 @@ cmd_add_br(struct vsctl_context *ctx)
 }
 
 static void
-del_port(struct vsctl_port *port)
+del_port(struct vsctl_info *info, 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);
@@ -1205,19 +1245,19 @@ cmd_del_br(struct vsctl_context *ctx)
     get_info(ctx, &info);
     bridge = find_bridge(&info, ctx->argv[1], must_exist);
     if (bridge) {
-        if (bridge->br_cfg) {
-            ovs_delete_bridge(ctx->ovs, bridge->br_cfg);
-        } else {
-            struct shash_node *node;
+        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);
-                }
+        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);
+        }
     }
     free_info(&info);
 }
@@ -1601,7 +1641,7 @@ cmd_del_port(struct vsctl_context *ctx)
             }
         }
 
-        del_port(port);
+        del_port(&info, port);
     }
 
     free_info(&info);
@@ -1734,6 +1774,17 @@ 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;
@@ -1742,7 +1793,12 @@ cmd_del_controller(struct vsctl_context *ctx)
     get_info(ctx, &info);
 
     br = find_real_bridge(&info, ctx->argv[1], true);
-    ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
+    verify_controllers(br->br_cfg);
+
+    if (br->ctrl) {
+        delete_controllers(br->ctrl, br->n_ctrl);
+        ovsrec_bridge_set_controller(br->br_cfg, NULL, 0);
+    }
 
     free_info(&info);
 }
@@ -1772,6 +1828,9 @@ 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);
@@ -1878,14 +1937,30 @@ cmd_get_manager(struct vsctl_context *ctx)
 }
 
 static void
-cmd_del_manager(struct vsctl_context *ctx)
+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);
+}
+
+static void
 insert_managers(struct vsctl_context *ctx, char *targets[], size_t n)
 {
     struct ovsrec_manager **managers;
@@ -1908,6 +1983,8 @@ 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);
 }
 
@@ -1951,7 +2028,13 @@ pre_cmd_del_ssl(struct vsctl_context *ctx)
 static void
 cmd_del_ssl(struct vsctl_context *ctx)
 {
-    ovsrec_open_vswitch_set_ssl(ctx->ovs, NULL);
+    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);
+    }
 }
 
 static void
@@ -1964,8 +2047,12 @@ static void
 cmd_set_ssl(struct vsctl_context *ctx)
 {
     bool bootstrap = shash_find(&ctx->options, "--bootstrap");
-    struct ovsrec_ssl *ssl;
+    struct ovsrec_ssl *ssl = ctx->ovs->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]);
-- 
1.7.4.1




More information about the dev mailing list