Do you think we should print a warning (at least once) to indicate that the
integration bridge is missing? Otherwise, it could be confusing to someone why
nothing's happening if they typo-ed their bridge name.
Acked-by: Justin Pettit <jpet...@nicira.com>
--Justin
> On Jul 28, 2015, at 8:44 AM, Ben Pfaff <b...@nicira.com> wrote:
>
> Until now, if the integration bridge was missing, ovn-controller exited.
> This commit makes it wait until the integration bridge is created.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ovn/controller/binding.c | 4 +++-
> ovn/controller/encaps.c | 2 +-
> ovn/controller/ofctrl.c | 16 ++++++++++------
> ovn/controller/ovn-controller.c | 39 ++++++++++-----------------------------
> 4 files changed, 24 insertions(+), 37 deletions(-)
>
> diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c
> index 2cb0b42..b6b345e 100644
> --- a/ovn/controller/binding.c
> +++ b/ovn/controller/binding.c
> @@ -91,7 +91,9 @@ binding_run(struct controller_ctx *ctx, const struct
> ovsrec_bridge *br_int,
>
> sset_init(&lports);
> sset_init(&all_lports);
> - get_local_iface_ids(br_int, &lports);
> + if (br_int) {
> + get_local_iface_ids(br_int, &lports);
> + }
> sset_clone(&all_lports, &lports);
>
> ovsdb_idl_txn_add_comment(ctx->ovnsb_idl_txn,
> diff --git a/ovn/controller/encaps.c b/ovn/controller/encaps.c
> index 7aa523f..f150246 100644
> --- a/ovn/controller/encaps.c
> +++ b/ovn/controller/encaps.c
> @@ -231,7 +231,7 @@ void
> encaps_run(struct controller_ctx *ctx, const struct ovsrec_bridge *br_int,
> const char *chassis_id)
> {
> - if (!ctx->ovs_idl_txn) {
> + if (!ctx->ovs_idl_txn || !br_int) {
> return;
> }
>
> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
> index c548645..b1c421c 100644
> --- a/ovn/controller/ofctrl.c
> +++ b/ovn/controller/ofctrl.c
> @@ -92,13 +92,17 @@ ofctrl_init(void)
> void
> ofctrl_run(const struct ovsrec_bridge *br_int, struct hmap *flow_table)
> {
> - char *target;
> - target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
> - if (strcmp(target, rconn_get_target(swconn))) {
> - VLOG_INFO("%s: connecting to switch", target);
> - rconn_connect(swconn, target, target);
> + if (br_int) {
> + char *target;
> + target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
> + if (strcmp(target, rconn_get_target(swconn))) {
> + VLOG_INFO("%s: connecting to switch", target);
> + rconn_connect(swconn, target, target);
> + }
> + free(target);
> + } else {
> + rconn_disconnect(swconn);
> }
> - free(target);
>
> rconn_run(swconn);
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index e3c745a..d7ab7a3 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -108,7 +108,6 @@ get_core_config(struct controller_ctx *ctx, char
> **br_int_namep,
> exit(EXIT_FAILURE);
> }
>
> - const struct ovsrec_bridge *br_int;
> const char *remote, *system_id, *br_int_name;
>
> br_int_name = smap_get(&cfg->external_ids, "ovn-bridge");
> @@ -116,13 +115,6 @@ get_core_config(struct controller_ctx *ctx, char
> **br_int_namep,
> br_int_name = DEFAULT_BRIDGE_NAME;
> }
>
> - br_int = get_bridge(ctx, br_int_name);
> - if (!br_int) {
> - VLOG_INFO("Integration bridge '%s' does not exist. Waiting...",
> - br_int_name);
> - goto try_again;
> - }
> -
> remote = smap_get(&cfg->external_ids, "ovn-remote");
> if (!remote) {
> VLOG_INFO("OVN OVSDB remote not specified. Waiting...");
> @@ -280,24 +272,19 @@ main(int argc, char *argv[])
> ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop);
> ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
>
> - /* xxx If run into any surprising changes, we exit. We should
> - * xxx handle this more gracefully. */
> const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
> - if (!br_int) {
> - VLOG_ERR("Integration bridge '%s' disappeared", br_int_name);
> - retval = EXIT_FAILURE;
> - goto exit;
> - }
>
> chassis_run(&ctx, chassis_id);
> encaps_run(&ctx, br_int, chassis_id);
> binding_run(&ctx, br_int, chassis_id);
>
> - struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> - pipeline_run(&ctx, &flow_table);
> - physical_run(&ctx, br_int, chassis_id, &flow_table);
> - ofctrl_run(br_int, &flow_table);
> - hmap_destroy(&flow_table);
> + if (br_int) {
> + struct hmap flow_table = HMAP_INITIALIZER(&flow_table);
> + pipeline_run(&ctx, &flow_table);
> + physical_run(&ctx, br_int, chassis_id, &flow_table);
> + ofctrl_run(br_int, &flow_table);
> + hmap_destroy(&flow_table);
> + }
>
> unixctl_server_run(unixctl);
>
> @@ -309,7 +296,9 @@ main(int argc, char *argv[])
> idl_loop_commit_and_wait(&ovnsb_idl_loop);
> idl_loop_commit_and_wait(&ovs_idl_loop);
>
> - ofctrl_wait();
> + if (br_int) {
> + ofctrl_wait();
> + }
> poll_block();
> }
>
> @@ -319,14 +308,7 @@ main(int argc, char *argv[])
> ctx.ovnsb_idl_txn = idl_loop_run(&ovnsb_idl_loop);
> ctx.ovs_idl_txn = idl_loop_run(&ovs_idl_loop);
>
> - /* xxx If run into any surprising changes, we exit. We should
> - * xxx handle this more gracefully. */
> const struct ovsrec_bridge *br_int = get_bridge(&ctx, br_int_name);
> - if (!br_int) {
> - VLOG_ERR("Integration bridge '%s' disappeared", br_int_name);
> - retval = EXIT_FAILURE;
> - goto exit;
> - }
>
> /* Run all of the cleanup functions, even if one of them returns
> false.
> * We're done if all of them return true. */
> @@ -342,7 +324,6 @@ main(int argc, char *argv[])
> poll_block();
> }
>
> -exit:
> unixctl_server_destroy(unixctl);
> pipeline_destroy(&ctx);
> ofctrl_destroy();
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
