| Message ID | 20170902011410.25228-2-russell@ovn.org |
|---|---|
| State | Accepted |
| Delegated to: | Russell Bryant |
| Headers | show |
| Series | [ovs-dev,1/2] .gitignore: Ignore generated file cxxtest.cc. | expand |
> From: "Russell Bryant" <russell@ovn.org> > To: dev@openvswitch.org > Cc: lrichard@redhat.com, "Russell Bryant" <russell@ovn.org> > Sent: Friday, September 1, 2017 9:14:10 PM > Subject: [PATCH 2/2] ovn: Support chassis hostname in requested-chassis. > > Previously, OVN expected the Chassis "name" in the "requested-chassis" > option for a Logical_Switch_Port. It turns out that in the two OVN > integrations I've checked with that plan to use this option, > specifying the Chassis "hostname" is much more convenient. This patch > extends the "requested-chassis" option to support both the Chassis > name or the hostname as a value. > > Signed-off-by: Russell Bryant <russell@ovn.org> > --- LGTM. Acked-by: Lance Richardson <lrichard@redhat.com>
On Tue, Sep 5, 2017 at 3:45 PM, Lance Richardson <lrichard@redhat.com> wrote: >> From: "Russell Bryant" <russell@ovn.org> >> To: dev@openvswitch.org >> Cc: lrichard@redhat.com, "Russell Bryant" <russell@ovn.org> >> Sent: Friday, September 1, 2017 9:14:10 PM >> Subject: [PATCH 2/2] ovn: Support chassis hostname in requested-chassis. >> >> Previously, OVN expected the Chassis "name" in the "requested-chassis" >> option for a Logical_Switch_Port. It turns out that in the two OVN >> integrations I've checked with that plan to use this option, >> specifying the Chassis "hostname" is much more convenient. This patch >> extends the "requested-chassis" option to support both the Chassis >> name or the hostname as a value. >> >> Signed-off-by: Russell Bryant <russell@ovn.org> >> --- > > LGTM. > > Acked-by: Lance Richardson <lrichard@redhat.com> Thanks. I added a note to NEWS and then applied this to master and branch-2.8.
I was thinking that it could have been convenient to do it that way in: https://github.com/openvswitch/ovs/blob/master/ovn/ovn-nb.ovsschema#L315 But we picked up chassis_name instead, not sure if it'd make any sense to use the hostname instead, it'd be much nicer for listing, so you can just see the host and avoid the mental exercise of cross referencing the uuid to the chassis. On Wed, Sep 6, 2017 at 11:16 PM, Russell Bryant <russell@ovn.org> wrote: > On Tue, Sep 5, 2017 at 3:45 PM, Lance Richardson <lrichard@redhat.com> > wrote: > >> From: "Russell Bryant" <russell@ovn.org> > >> To: dev@openvswitch.org > >> Cc: lrichard@redhat.com, "Russell Bryant" <russell@ovn.org> > >> Sent: Friday, September 1, 2017 9:14:10 PM > >> Subject: [PATCH 2/2] ovn: Support chassis hostname in requested-chassis. > >> > >> Previously, OVN expected the Chassis "name" in the "requested-chassis" > >> option for a Logical_Switch_Port. It turns out that in the two OVN > >> integrations I've checked with that plan to use this option, > >> specifying the Chassis "hostname" is much more convenient. This patch > >> extends the "requested-chassis" option to support both the Chassis > >> name or the hostname as a value. > >> > >> Signed-off-by: Russell Bryant <russell@ovn.org> > >> --- > > > > LGTM. > > > > Acked-by: Lance Richardson <lrichard@redhat.com> > > Thanks. I added a note to NEWS and then applied this to master and > branch-2.8. > > -- > Russell Bryant > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
I'd be supportive of expanding that field to allow either name or hostname as well. On Thu, Sep 7, 2017 at 9:44 AM, Miguel Angel Ajo Pelayo <majopela@redhat.com> wrote: > I was thinking that it could have been convenient to do it that way in: > > https://github.com/openvswitch/ovs/blob/master/ovn/ovn-nb.ovsschema#L315 > > But we picked up chassis_name instead, not sure if it'd make any sense to > use the hostname instead, it'd be much nicer for listing, so you can just > see the host and avoid the mental exercise of cross referencing the uuid to > the chassis. > > On Wed, Sep 6, 2017 at 11:16 PM, Russell Bryant <russell@ovn.org> wrote: >> >> On Tue, Sep 5, 2017 at 3:45 PM, Lance Richardson <lrichard@redhat.com> >> wrote: >> >> From: "Russell Bryant" <russell@ovn.org> >> >> To: dev@openvswitch.org >> >> Cc: lrichard@redhat.com, "Russell Bryant" <russell@ovn.org> >> >> Sent: Friday, September 1, 2017 9:14:10 PM >> >> Subject: [PATCH 2/2] ovn: Support chassis hostname in >> >> requested-chassis. >> >> >> >> Previously, OVN expected the Chassis "name" in the "requested-chassis" >> >> option for a Logical_Switch_Port. It turns out that in the two OVN >> >> integrations I've checked with that plan to use this option, >> >> specifying the Chassis "hostname" is much more convenient. This patch >> >> extends the "requested-chassis" option to support both the Chassis >> >> name or the hostname as a value. >> >> >> >> Signed-off-by: Russell Bryant <russell@ovn.org> >> >> --- >> > >> > LGTM. >> > >> > Acked-by: Lance Richardson <lrichard@redhat.com> >> >> Thanks. I added a note to NEWS and then applied this to master and >> branch-2.8. >> >> -- >> Russell Bryant >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
Ack, I'll submit a patch for it :) On Thu, Sep 7, 2017 at 4:28 PM, Russell Bryant <russell@ovn.org> wrote: > I'd be supportive of expanding that field to allow either name or > hostname as well. > > On Thu, Sep 7, 2017 at 9:44 AM, Miguel Angel Ajo Pelayo > <majopela@redhat.com> wrote: > > I was thinking that it could have been convenient to do it that way in: > > > > https://github.com/openvswitch/ovs/blob/master/ovn/ovn-nb.ovsschema#L315 > > > > But we picked up chassis_name instead, not sure if it'd make any sense to > > use the hostname instead, it'd be much nicer for listing, so you can just > > see the host and avoid the mental exercise of cross referencing the uuid > to > > the chassis. > > > > On Wed, Sep 6, 2017 at 11:16 PM, Russell Bryant <russell@ovn.org> wrote: > >> > >> On Tue, Sep 5, 2017 at 3:45 PM, Lance Richardson <lrichard@redhat.com> > >> wrote: > >> >> From: "Russell Bryant" <russell@ovn.org> > >> >> To: dev@openvswitch.org > >> >> Cc: lrichard@redhat.com, "Russell Bryant" <russell@ovn.org> > >> >> Sent: Friday, September 1, 2017 9:14:10 PM > >> >> Subject: [PATCH 2/2] ovn: Support chassis hostname in > >> >> requested-chassis. > >> >> > >> >> Previously, OVN expected the Chassis "name" in the > "requested-chassis" > >> >> option for a Logical_Switch_Port. It turns out that in the two OVN > >> >> integrations I've checked with that plan to use this option, > >> >> specifying the Chassis "hostname" is much more convenient. This > patch > >> >> extends the "requested-chassis" option to support both the Chassis > >> >> name or the hostname as a value. > >> >> > >> >> Signed-off-by: Russell Bryant <russell@ovn.org> > >> >> --- > >> > > >> > LGTM. > >> > > >> > Acked-by: Lance Richardson <lrichard@redhat.com> > >> > >> Thanks. I added a note to NEWS and then applied this to master and > >> branch-2.8. > >> > >> -- > >> Russell Bryant > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > > > > > -- > Russell Bryant >
diff --git a/ovn/controller/binding.c b/ovn/controller/binding.c index 6a56e26ca..ca1d43395 100644 --- a/ovn/controller/binding.c +++ b/ovn/controller/binding.c @@ -442,8 +442,9 @@ consider_local_datapath(struct controller_ctx *ctx, if (ctx->ovnsb_idl_txn) { const char *vif_chassis = smap_get(&binding_rec->options, "requested-chassis"); - bool can_bind = !vif_chassis || !vif_chassis[0] || - !strcmp(vif_chassis, chassis_rec->name); + bool can_bind = !vif_chassis || !vif_chassis[0] + || !strcmp(vif_chassis, chassis_rec->name) + || !strcmp(vif_chassis, chassis_rec->hostname); if (can_bind && our_chassis) { if (binding_rec->chassis != chassis_rec) { diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml index be72610c0..9869d7ed7 100644 --- a/ovn/ovn-nb.xml +++ b/ovn/ovn-nb.xml @@ -436,11 +436,12 @@ </p> <column name="options" key="requested-chassis"> - If set, identifies a specific chassis (by name) that is allowed to - bind this port. Using this option will prevent thrashing between - two chassis trying to bind the same port during a live migration. - It can also prevent similar thrashing due to a mis-configuration, - if a port is accidentally created on more than one chassis. + If set, identifies a specific chassis (by name or hostname) that + is allowed to bind this port. Using this option will prevent + thrashing between two chassis trying to bind the same port during + a live migration. It can also prevent similar thrashing due to a + mis-configuration, if a port is accidentally created on more than + one chassis. </column> <column name="options" key="qos_max_rate"> diff --git a/ovn/ovn-sb.xml b/ovn/ovn-sb.xml index 7098194c6..0a894f8cb 100644 --- a/ovn/ovn-sb.xml +++ b/ovn/ovn-sb.xml @@ -2160,11 +2160,12 @@ tcp.flags = RST; </p> <column name="options" key="requested-chassis"> - If set, identifies a specific chassis (by name) that is allowed to - bind this port. Using this option will prevent thrashing between - two chassis trying to bind the same port during a live migration. - It can also prevent similar thrashing due to a mis-configuration, - if a port is accidentally created on more than one chassis. + If set, identifies a specific chassis (by name or hostname) that + is allowed to bind this port. Using this option will prevent + thrashing between two chassis trying to bind the same port during + a live migration. It can also prevent similar thrashing due to a + mis-configuration, if a port is accidentally created on more than + one chassis. </column> <column name="options" key="qos_max_rate"> diff --git a/tests/ovn.at b/tests/ovn.at index fb9fc7352..bb9999ce0 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -8541,3 +8541,36 @@ OVS_WAIT_UNTIL([test $(ovn-sbctl --bare --columns chassis find port_binding logi OVN_CLEANUP([hv1],[hv2]) AT_CLEANUP + +AT_SETUP([ovn -- options:requested-chassis with hostname]) + +ovn_start + +ovn-nbctl ls-add ls0 +ovn-nbctl lsp-add ls0 lsp0 + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.11 +ovs-vsctl -- add-port br-int hv1-vif0 + +hv1_hostname=$(ovn-sbctl --bare --columns hostname find Chassis name=hv1) +echo "hv1_hostname=${hv1_hostname}" +ovn-nbctl --wait=hv --timeout=3 lsp-set-options lsp0 requested-chassis=${hv1_hostname} +as hv1 ovs-vsctl set interface hv1-vif0 external-ids:iface-id=lsp0 + +hv1_uuid=$(ovn-sbctl --bare --columns _uuid find Chassis name=hv1) +echo "hv1_uuid=${hv1_uuid}" +OVS_WAIT_UNTIL([test 1 = $(grep -c "Claiming lport lsp0" hv1/ovn-controller.log)]) +AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x"$hv1_uuid"], [0], []) + +ovn-nbctl --wait=hv --timeout=3 lsp-set-options lsp0 requested-chassis=non-existant-chassis +OVS_WAIT_UNTIL([test 1 = $(grep -c "Releasing lport lsp0 from this chassis" hv1/ovn-controller.log)]) +ovn-nbctl --wait=hv --timeout=3 sync +AT_CHECK([test x$(ovn-sbctl --bare --columns chassis find port_binding logical_port=lsp0) = x], [0], []) + +OVN_CLEANUP([hv1]) + +AT_CLEANUP
Previously, OVN expected the Chassis "name" in the "requested-chassis" option for a Logical_Switch_Port. It turns out that in the two OVN integrations I've checked with that plan to use this option, specifying the Chassis "hostname" is much more convenient. This patch extends the "requested-chassis" option to support both the Chassis name or the hostname as a value. Signed-off-by: Russell Bryant <russell@ovn.org> --- ovn/controller/binding.c | 5 +++-- ovn/ovn-nb.xml | 11 ++++++----- ovn/ovn-sb.xml | 11 ++++++----- tests/ovn.at | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 12 deletions(-)