On Tue, Jul 18, 2017 at 2:05 AM, Venkata Anil Kommaddi
<vkomm...@redhat.com> wrote:
> From: Venkata Anil <vkomm...@redhat.com>
>
> This change adds commands to set, get and delete gateway chassis
> for logical router port.
>
> Signed-off-by: Venkata Anil Kommaddi <vkomm...@redhat.com>
> ---
>  ovn/utilities/ovn-nbctl.8.xml |  21 +++++
>  ovn/utilities/ovn-nbctl.c     | 178 
> ++++++++++++++++++++++++++++++++++++++++++
>  tests/ovn-nbctl.at            |  52 ++++++++++++
>  3 files changed, 251 insertions(+)

Thanks for the patch!  I applied this to master (with some changes, see below).


As a follow-up patch, I'd like to see gateway chassis listed in
"ovn-nbctl show" output.

At the end of the message, see the output from checkpatch.py.  I fixed
these, but wanted to show you for the future.

Finally, I made the following additional changes to the patch to fix a
couple of issues:


diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 0b4894911..53145daf3 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -2535,7 +2535,7 @@ lr_get_name(const struct nbrec_logical_router
*lr, char uuid_s[UUID_LEN + 1],
     return uuid_s;
 }

-static const struct nbrec_gateway_chassis*
+static const struct nbrec_gateway_chassis *
 gc_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist)
 {
     const struct nbrec_gateway_chassis *gc = NULL;
@@ -2565,7 +2565,7 @@ gc_by_name_or_uuid(struct ctl_context *ctx,
const char *id, bool must_exist)
 static void
 nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
 {
-    const char *gc_name;
+    char *gc_name;
     int64_t priority = 0;
     const char *lrp_name = ctx->argv[1];
     const struct nbrec_logical_router_port *lrp;
@@ -2581,15 +2581,17 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
     }

     gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
-    const struct nbrec_gateway_chassis *gc;
-    gc = gc_by_name_or_uuid(ctx, gc_name, false);
-    if (gc) {
-        nbrec_gateway_chassis_set_priority(gc, priority);
+    const struct nbrec_gateway_chassis *existing_gc;
+    existing_gc = gc_by_name_or_uuid(ctx, gc_name, false);
+    if (existing_gc) {
+        nbrec_gateway_chassis_set_priority(existing_gc, priority);
+        free(gc_name);
         return;
     }

     /* Create the logical gateway chassis. */
-    gc = nbrec_gateway_chassis_insert(ctx->txn);
+    struct nbrec_gateway_chassis *gc
+        = nbrec_gateway_chassis_insert(ctx->txn);
     nbrec_gateway_chassis_set_name(gc, gc_name);
     nbrec_gateway_chassis_set_chassis_name(gc, chassis_name);
     nbrec_gateway_chassis_set_priority(gc, priority);
@@ -2600,11 +2602,11 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
         sizeof *new_gc * (lrp->n_gateway_chassis + 1));
     nullable_memcpy(new_gc, lrp->gateway_chassis,
                     sizeof *new_gc * lrp->n_gateway_chassis);
-    new_gc[lrp->n_gateway_chassis] = CONST_CAST(
-        struct nbrec_gateway_chassis *, gc);
+    new_gc[lrp->n_gateway_chassis] = gc;
     nbrec_logical_router_port_set_gateway_chassis(
         lrp, new_gc, lrp->n_gateway_chassis + 1);
     free(new_gc);
+    free(gc_name);
 }

 /* Removes logical router port 'lrp->gateway_chassis[idx]'. */
@@ -2612,17 +2614,22 @@ static void
 remove_gc(const struct nbrec_logical_router_port *lrp, size_t idx)
 {
     const struct nbrec_gateway_chassis *gc = lrp->gateway_chassis[idx];
-    /* First remove 'gc' from the array of gateway_chassis.  This is what will
-     * actually cause the gateway chassis to be deleted when the transaction is
-     * sent to the database server (due to garbage collection). */
-    struct nbrec_gateway_chassis **new_gc
-        = xmemdup(lrp->gateway_chassis,
-                  sizeof *new_gc * lrp->n_gateway_chassis);
-    new_gc[idx] = new_gc[lrp->n_gateway_chassis - 1];
-    nbrec_logical_router_port_verify_gateway_chassis(lrp);
-    nbrec_logical_router_port_set_gateway_chassis(
-        lrp, new_gc, lrp->n_gateway_chassis - 1);
-    free(new_gc);
+
+    if (lrp->n_gateway_chassis == 1) {
+        nbrec_logical_router_port_set_gateway_chassis(lrp, NULL, 0);
+    } else {
+        /* First remove 'gc' from the array of gateway_chassis.  This
is what will
+         * actually cause the gateway chassis to be deleted when the
transaction is
+         * sent to the database server (due to garbage collection). */
+        struct nbrec_gateway_chassis **new_gc
+            = xmemdup(lrp->gateway_chassis,
+                      sizeof *new_gc * lrp->n_gateway_chassis);
+        new_gc[idx] = new_gc[lrp->n_gateway_chassis - 1];
+        nbrec_logical_router_port_verify_gateway_chassis(lrp);
+        nbrec_logical_router_port_set_gateway_chassis(
+            lrp, new_gc, lrp->n_gateway_chassis - 1);
+        free(new_gc);
+    }

     /* Delete 'gc' from the IDL.  This won't have a real effect on
      * the database server (the IDL will suppress it in fact) but it
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index ded21fcb2..29496c5c7 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -803,6 +803,9 @@ AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0], [0], [dnl
 lrp0-chassis2     5
 ])

+AT_CHECK([ovn-nbctl lrp-del-gateway-chassis lrp0 chassis2])
+AT_CHECK([ovn-nbctl lrp-get-gateway-chassis lrp0])
+
 OVN_NBCTL_TEST_STOP
 AT_CLEANUP


-----------------------

$ utilities/checkpatch.py -1
== Checking b709f7ee06bd ("ovn: l3ha, CLI for logical router port
gateway chassis") ==
WARNING: Line length is >79-characters long
#25 FILE: ovn/utilities/ovn-nbctl.8.xml:393:
      <dt><code>lrp-set-gateway-chassis</code> <var>port</var>
<var>chassis</var> [<var>priority</var>]</dt>

WARNING: Line has non-spaces leading whitespace
#27 FILE: ovn/utilities/ovn-nbctl.8.xml:395:
Set gateway chassis for <var>port</var>. <var>chassis</var>

WARNING: Line has non-spaces leading whitespace
#28 FILE: ovn/utilities/ovn-nbctl.8.xml:396:
is the name of the chassis. This creates a gateway chassis entry

WARNING: Line has non-spaces leading whitespace
#29 FILE: ovn/utilities/ovn-nbctl.8.xml:397:
in Gateway_Chassis table. It won't check if chassis really exists

WARNING: Line has non-spaces leading whitespace
#30 FILE: ovn/utilities/ovn-nbctl.8.xml:398:
in OVN_Southbound database. Priority will be set to 0

WARNING: Line has non-spaces leading whitespace
#31 FILE: ovn/utilities/ovn-nbctl.8.xml:399:
if <var>priority</var> is not provided by user. <var>priority</var>

WARNING: Line has non-spaces leading whitespace
#32 FILE: ovn/utilities/ovn-nbctl.8.xml:400:
must be between <code>0</code> and <code>32767</code>, inclusive.

WARNING: Line length is >79-characters long
#34 FILE: ovn/utilities/ovn-nbctl.8.xml:402:
      <dt><code>lrp-del-gateway-chassis</code> <var>port</var>
<var>chassis</var></dt>

WARNING: Line has non-spaces leading whitespace
#36 FILE: ovn/utilities/ovn-nbctl.8.xml:404:
Deletes gateway chassis from <var>port</var>.  It is an error if

WARNING: Line has non-spaces leading whitespace
#37 FILE: ovn/utilities/ovn-nbctl.8.xml:405:
gateway chassis with <var>chassis</var> for <var>port</var> does

Lines checked: 317, Warnings: 10, Errors: 0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to