[ovs-dev,v7,02/12] controller: Make use of Port_Binding:requested_chassis.

diff --git a/controller/binding.c b/controller/binding.c
index c037b2352..aac96694a 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1056,11 +1056,27 @@  is_binding_lport_this_chassis(struct binding_lport *b_lport,
 
 static bool
 can_bind_on_this_chassis(const struct sbrec_chassis *chassis_rec,
-                         const char *requested_chassis)
-{
-    return !requested_chassis || !requested_chassis[0]
-           || !strcmp(requested_chassis, chassis_rec->name)
-           || !strcmp(requested_chassis, chassis_rec->hostname);
+                         const struct sbrec_port_binding *pb)
+{
+    /* We need to check for presence of the requested-chassis option in
+     * addittion to checking the pb->requested_chassis column because this
+     * column will be set to NULL whenever the option points to a non-existent
+     * chassis.  As the controller routinely clears its own chassis record this
+     * might occur more often than one might think. */
+    const char *requested_chassis_option = smap_get(&pb->options,
+                                                    "requested-chassis");
+    if (requested_chassis_option && requested_chassis_option[0]
+        && !pb->requested_chassis) {
+        /* The requested-chassis option is set, but the requested_chassis
+         * column is not filled.  This means that the chassis the option
+         * points to is currently not running, or is in the process of starting
+         * up.  In this case we must fall back to comparing the strings to
+         * avoid release/claim thrashing. */
+        return !strcmp(requested_chassis_option, chassis_rec->name)
+               || !strcmp(requested_chassis_option, chassis_rec->hostname);
+    }
+    return !requested_chassis_option || !requested_chassis_option[0]
+           || chassis_rec == pb->requested_chassis;
 }
 
 /* Returns 'true' if the 'lbinding' has binding lports of type LP_CONTAINER,
@@ -1098,7 +1114,7 @@  release_binding_lport(const struct sbrec_chassis *chassis_rec,
 
 static bool
 consider_vif_lport_(const struct sbrec_port_binding *pb,
-                    bool can_bind, const char *vif_chassis,
+                    bool can_bind,
                     struct binding_ctx_in *b_ctx_in,
                     struct binding_ctx_out *b_ctx_out,
                     struct binding_lport *b_lport,
@@ -1139,7 +1155,10 @@  consider_vif_lport_(const struct sbrec_port_binding *pb,
                              "requested-chassis %s",
                              pb->logical_port,
                              b_ctx_in->chassis_rec->name,
-                             vif_chassis);
+                             pb->requested_chassis ?
+                             pb->requested_chassis->name : "(option points at "
+                                                           "non-existent "
+                                                           "chassis)");
         }
     }
 
@@ -1162,9 +1181,7 @@  consider_vif_lport(const struct sbrec_port_binding *pb,
                    struct local_binding *lbinding,
                    struct hmap *qos_map)
 {
-    const char *vif_chassis = smap_get(&pb->options, "requested-chassis");
-    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
-                                             vif_chassis);
+    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
 
     if (!lbinding) {
         lbinding = local_binding_find(&b_ctx_out->lbinding_data->bindings,
@@ -1194,8 +1211,8 @@  consider_vif_lport(const struct sbrec_port_binding *pb,
         }
     }
 
-    return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in,
-                               b_ctx_out, b_lport, qos_map);
+    return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
+                               b_lport, qos_map);
 }
 
 static bool
@@ -1279,12 +1296,9 @@  consider_container_lport(const struct sbrec_port_binding *pb,
     }
 
     ovs_assert(parent_b_lport && parent_b_lport->pb);
-    const char *vif_chassis = smap_get(&parent_b_lport->pb->options,
-                                       "requested-chassis");
-    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec,
-                                             vif_chassis);
+    bool can_bind = can_bind_on_this_chassis(b_ctx_in->chassis_rec, pb);
 
-    return consider_vif_lport_(pb, can_bind, vif_chassis, b_ctx_in, b_ctx_out,
+    return consider_vif_lport_(pb, can_bind, b_ctx_in, b_ctx_out,
                                container_b_lport, qos_map);
 }
 
@@ -1333,7 +1347,7 @@  consider_virtual_lport(const struct sbrec_port_binding *pb,
         }
     }
 
-    if (!consider_vif_lport_(pb, true, NULL, b_ctx_in, b_ctx_out,
+    if (!consider_vif_lport_(pb, true, b_ctx_in, b_ctx_out,
                              virtual_b_lport, qos_map)) {
         return false;
     }
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4202f32cc..a1382a67b 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -232,6 +232,9 @@  update_sb_monitors(struct ovsdb_idl *ovnsb_idl,
         sbrec_port_binding_add_clause_chassis(&pb, OVSDB_F_EQ,
                                               &chassis->header_.uuid);
 
+        sbrec_port_binding_add_clause_requested_chassis(
+            &pb, OVSDB_F_EQ, &chassis->header_.uuid);
+
         /* Ensure that we find out about l2gateway and l3gateway ports that
          * should be present on this chassis.  Otherwise, we might never find
          * out about those ports, if their datapaths don't otherwise have a VIF
diff --git a/tests/ovn.at b/tests/ovn.at
index 05eac4e5f..7cfdede77 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -28690,3 +28690,102 @@  grep lr0-sw1], [1], [])
 OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller requested-chassis localport])
+ovn_start
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovs-vsctl add-port br-int lport \
+    -- set interface lport external_ids:iface-id=sw0-lport ofport-request=13
+
+sim_add hv2
+as hv2
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.2
+ovs-vsctl add-port br-int lport \
+    -- set interface lport external_ids:iface-id=sw0-lport ofport-request=13
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 sw0-lport
+check ovn-nbctl lsp-set-addresses sw0-lport "50:54:00:00:00:01 10.0.0.2"
+check ovn-nbctl lsp-set-type sw0-lport localport
+
+# First confirm operation without requested-chassis
+check_row_count Port_Binding 1 logical_port=sw0-lport 'up=true'
+
+AT_CHECK([as hv1 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [0], [])
+AT_CHECK([as hv2 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [0], [])
+
+# Set requested-chassis to one of the HVs
+check ovn-nbctl --wait=sb lsp-set-options sw0-lport requested-chassis="hv1"
+AT_CHECK([as hv1 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [0], [])
+AT_CHECK([as hv2 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [1], [])
+
+# Set requested-chassis to empty string
+check ovn-nbctl --wait=sb lsp-set-options sw0-lport requested-chassis=""
+AT_CHECK([as hv1 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [0], [])
+AT_CHECK([as hv2 ovs-ofctl -O OpenFlow15 dump-flows br-int table=0 | grep -q in_port=13], [0], [])
+
+OVN_CLEANUP([hv1],[hv2])
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn-controller - requested-chassis claim lport on startup])
+ovn_start
+
+net_add n1
+sim_add hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+check ovs-vsctl -- add-port br-int vif1 -- \
+    set Interface vif1 external-ids:iface-id=lsp1
+
+check ovn-nbctl ls-add lsw0
+
+check ovn-nbctl lsp-add lsw0 lsp1
+check ovn-nbctl lsp-set-addresses lsp1 "f0:00:00:00:00:01 172.16.0.101"
+check ovn-nbctl set Logical_Switch_Port lsp1 \
+    options:requested-chassis=hv1
+
+wait_for_ports_up lsp1
+
+lsp1_pb_uuid=$(ovn-sbctl --bare --columns _uuid \
+               find Port_Binding logical_port=lsp1)
+
+# Stop ovn-controller on hv1 without the --restart flag
+check as hv1 ovn-appctl -t ovn-controller exit
+
+# Pause northd to guarantee that ovn-controller starts before requested_chassis
+# column is filled.
+check as northd-backup ovn-appctl -t ovn-northd pause
+check as northd ovn-appctl -t ovn-northd pause
+
+# Wait until requested_chassis is empty
+OVS_WAIT_UNTIL([
+    test x"3" = x$(ovn-sbctl get Port_Binding ${lsp1_pb_uuid} requested_chassis) | wc -c
+])
+
+# Start controller and wait for it to register itself
+as hv1 start_daemon ovn-controller
+wait_column "hv1" Chassis name
+
+# Start northd and wait for events to be processed
+check as northd ovn-appctl -t ovn-northd resume
+check as northd-backup ovn-appctl -t ovn-northd resume
+
+wait_for_ports_up lsp1
+
+# Confirm that the controller did not refuse to claim its port on the initial
+# run.
+AT_CHECK([grep -q "Not claiming" hv1/ovn-controller.log], [1], [])
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])