[ovs-dev] The name of logical switch and router is null

[nickcooper-zhangtonghao]

Hi all,
	When I create one or more logical router named ’null', using 'ovn-nbctl lr-add’, there is no error information.
This is arguably an ovn-nbctl bug.

	# ovn-nbctl lr-add
	# ovn-nbctl lr-list
	3aeff07a-cd16-45f3-ac5a-e4da634b81f6 ()

	# ovn-nbctl lr-add
	# ovn-nbctl lr-add
	# ovn-nbctl lr-list
	3aeff07a-cd16-45f3-ac5a-e4da634b81f6 ()
	01dc4fe3-755f-4e88-9a53-8351b059d56a ()
	1a47070b-f2ca-48a2-9ab9-661f008cc75d ()

	What’s more, if you want to duplicate a logical router, you may use “--add-duplicate” option 
	# ovn-nbctl --add-duplicate lr-add lr1
	# ovn-nbctl lr-list
	33eadbf1-59d6-42f2-aedd-6d5c64e2105d (lr1)
	34cb1c7b-417d-4bde-b91a-3e0aae3e9e69 (lr1)


diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7789cdd..a1d71c2 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1206,37 +1206,31 @@ static void
 nbctl_lr_add(struct ctl_context *ctx)
 {
     const char *lr_name = ctx->argc == 2 ? ctx->argv[1] : NULL;
+    if (lr_name == NULL)
+        ctl_fatal("lr-add requires specifying a name");

     bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
     bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != NULL;
     if (may_exist && add_duplicate) {
         ctl_fatal("--may-exist and --add-duplicate may not be used together");
     }
-
-    if (lr_name) {
-        if (!add_duplicate) {
-            const struct nbrec_logical_router *lr;
-            NBREC_LOGICAL_ROUTER_FOR_EACH (lr, ctx->idl) {
-                if (!strcmp(lr->name, lr_name)) {
-                    if (may_exist) {
-                        return;
-                    }
-                    ctl_fatal("%s: a router with this name already exists",
-                              lr_name);
-                }
+
+    if (!add_duplicate) {
+        const struct nbrec_logical_router *lr;
+        NBREC_LOGICAL_ROUTER_FOR_EACH (lr, ctx->idl) {
+            if (!strcmp(lr->name, lr_name)) {
+                if (may_exist)
+                    return;
+
+                ctl_fatal("%s: a router with this name already exists",
+                        lr_name);
             }
         }
-    } else if (may_exist) {
-        ctl_fatal("--may-exist requires specifying a name");
-    } else if (add_duplicate) {
-        ctl_fatal("--add-duplicate requires specifying a name");
     }

     struct nbrec_logical_router *lr;
     lr = nbrec_logical_router_insert(ctx->txn);
-    if (lr_name) {
-        nbrec_logical_router_set_name(lr, lr_name);
-    }
+       nbrec_logical_router_set_name(lr, lr_name);
 }




diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7789cdd..520f7e8 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -526,6 +526,8 @@ static void
 nbctl_ls_add(struct ctl_context *ctx)
 {
     const char *ls_name = ctx->argc == 2 ? ctx->argv[1] : NULL;
+    if (ls_name == NULL)
+        ctl_fatal("ls-add requires specifying a name");

     bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
     bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != NULL;
@@ -533,30 +535,22 @@ nbctl_ls_add(struct ctl_context *ctx)
         ctl_fatal("--may-exist and --add-duplicate may not be used together");
     }

-    if (ls_name) {
-        if (!add_duplicate) {
-            const struct nbrec_logical_switch *ls;
-            NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->idl) {
-                if (!strcmp(ls->name, ls_name)) {
-                    if (may_exist) {
-                        return;
-                    }
-                    ctl_fatal("%s: a switch with this name already exists",
-                              ls_name);
-                }
+    if (!add_duplicate) {
+        const struct nbrec_logical_switch *ls;
+        NBREC_LOGICAL_SWITCH_FOR_EACH (ls, ctx->idl) {
+            if (!strcmp(ls->name, ls_name)) {
+                if (may_exist)
+                    return;
+
+                ctl_fatal("%s: a switch with this name already exists",
+                          ls_name);
             }
         }
-    } else if (may_exist) {
-        ctl_fatal("--may-exist requires specifying a name");
-    } else if (add_duplicate) {
-        ctl_fatal("--add-duplicate requires specifying a name");
     }

     struct nbrec_logical_switch *ls;
     ls = nbrec_logical_switch_insert(ctx->txn);
-    if (ls_name) {
-        nbrec_logical_switch_set_name(ls, ls_name);
-    }
+    nbrec_logical_switch_set_name(ls, ls_name);
 }



Thanks,
nick