From 28a14bf3d86efb8f0f29936e84cc9e4e384dffe8 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Wed, 16 Mar 2011 15:06:37 -0700 Subject: [PATCH] ovs-vsctl: Back out garbage collection changes. 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 2e8af852..77911fab 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 80c9048b..2baab003 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); @@ -1733,6 +1773,17 @@ cmd_get_controller(struct vsctl_context *ctx) free_info(&info); } +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) { @@ -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,13 +1937,29 @@ 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) { @@ -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]); -- 2.30.2