[ovs-dev] [PATCH] ovs-vsctl: Back out garbage collection changes.
Ethan Jackson
ethan at nicira.com
Thu Mar 17 00:23:45 UTC 2011
David wants this in as soon as possible. Could someone please have a
look at the lastest version of the patch. Simply changed the commit
message and added tests.
Ethan
On Wed, Mar 16, 2011 at 4:52 PM, Ethan Jackson <ethan at nicira.com> wrote:
> 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