diff mbox series

[ovs-dev,v8] db-ctl-base: add uuid specified method for create cmd

Message ID 20200325054039.16439-1-taoyunxiang@cmss.chinamobile.com
State Superseded
Headers show
Series [ovs-dev,v8] db-ctl-base: add uuid specified method for create cmd | expand

Commit Message

taoyunxiang March 25, 2020, 5:40 a.m. UTC
Commit a529e3cd1f (ovsdb-server: Allow OVSDB clients to specify the
UUID for inserted rows) solves ovsdb-client specifing the UUID for
insert operation.  OVSDB now can support directly using uuid to identify
a row. But for xxxctl tool,specifying uuid when creating a row is not
yet supported . This patch tried to specify uuid when creating a row
by the ctl tools. A new parameter --row_uuid is added to setup row's UUID.
e.g. ovn-nbctl --row_uuid=3da0398b-a5a8-4bc9-808d-fa662865138f  create
logical_switch name='abc'

Author: Liu Chang <liuchang@cmss.chinamobile.com>
Co-authored-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Co-authored-by: Rong Yin <rongyin@cmss.chinamobile.com>
Signed-off-by: Tao YunXiang <taoyunxiang@cmss.chinamobile.com>
Signed-off-by: Liu Chang <liuchang@cmss.chinamobile.com>
Signed-off-by: Rong Yin <rongyin@cmss.chinamobile.com>
CC: Ben Pfaff<blp@ovn.org>

---
v7: db-ctl-base: add uuid specified method for create cmd

v6: db-ctl-base: add uuid specified method for create cmd

v5: db-ctl-base: add uuid specified method for create cmd

---
 NEWS                |  1 +
 lib/db-ctl-base.c   | 17 +++++++++++++++-
 lib/db-ctl-base.xml | 11 ++++++++++-
 lib/ovsdb-idl.c     | 24 ++++++++++++++++++----
 lib/ovsdb-idl.h     |  1 +
 tests/ovs-vsctl.at  | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 105 insertions(+), 6 deletions(-)

Comments

0-day Robot March 25, 2020, 5:59 a.m. UTC | #1
Bleep bloop.  Greetings Tao YunXiang, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
WARNING: Line is 198 characters long (recommended limit is 79)
#105 FILE: lib/db-ctl-base.xml:293:
    <dt>[<code>--id=@</code><var>name</var>] [<code>--row-uuid=</code><var>uuid</var>] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt>

WARNING: Line is 93 characters long (recommended limit is 79)
WARNING: Line has trailing whitespace
#114 FILE: lib/db-ctl-base.xml:307:
        If <code></code><var>uuid</var> is supplied and is reasonable, then the UUID for the 

WARNING: Line has trailing whitespace
#115 FILE: lib/db-ctl-base.xml:308:
        new row may be specified as expected. 

WARNING: Line is 94 characters long (recommended limit is 79)
WARNING: Line has trailing whitespace
#118 FILE: lib/db-ctl-base.xml:311:
        If both <code>@</code><var>name</var> and <code></code><var>uuid</var> are specified, 

WARNING: Line is 92 characters long (recommended limit is 79)
WARNING: Line has trailing whitespace
#119 FILE: lib/db-ctl-base.xml:312:
        the new row with corresponding uuid may still be referred to by that name, but this 

WARNING: Line has trailing whitespace
#120 FILE: lib/db-ctl-base.xml:313:
        references can only follow the <code>create</code> command.   

WARNING: Line is 85 characters long (recommended limit is 79)
#179 FILE: lib/ovsdb-idl.c:4169:
                                        xasprintf(UUID_FMT, UUID_ARGS(&row->uuid))));

Lines checked: 277, Warnings: 10, Errors: 0


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/NEWS b/NEWS
index 32ca2e0c6..27fc3ae9d 100644
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,7 @@  v2.13.0 - 14 Feb 2020
        replication server. This value is configurable with the unixctl
        command - ovsdb-server/set-active-ovsdb-server-probe-interval.
      * ovsdb-server: New OVSDB extension to allow clients to specify row UUIDs.
+     * db-ctl-base: New row_uuid parameter specified for create cmd
    - 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
      partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
      type filter supports new filters: "dpdk" and "partially-offloaded".
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab2af9eda..8035cdebf 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1718,16 +1718,24 @@  static void
 cmd_create(struct ctl_context *ctx)
 {
     const char *id = shash_find_data(&ctx->options, "--id");
+    const char *row_uuid = shash_find_data(&ctx->options, "--row-uuid");
     const char *table_name = ctx->argv[1];
     const struct ovsdb_idl_table_class *table;
     const struct ovsdb_idl_row *row;
     const struct uuid *uuid = NULL;
+    struct uuid uuid_from_cmd;
     int i;
 
     ctx->error = get_table(table_name, &table);
     if (ctx->error) {
         return;
     }
+    if (row_uuid) {
+        if (!uuid_from_string(&uuid_from_cmd, row_uuid)) {
+            ctl_error(ctx, "row-uuid '%s' is not a valid UUID", row_uuid);
+            return;
+        }
+    }
     if (id) {
         struct ovsdb_symbol *symbol = NULL;
 
@@ -1741,7 +1749,14 @@  cmd_create(struct ctl_context *ctx)
              * warnings about that by pretending that there is a reference. */
             symbol->strong_ref = true;
         }
+        if (row_uuid) {
+            symbol->uuid = uuid_from_cmd;
+            ovsdb_idl_txn_set_uuid_specified(ctx->txn);
+        }
         uuid = &symbol->uuid;
+    } else if (row_uuid) {
+        uuid = &uuid_from_cmd;
+        ovsdb_idl_txn_set_uuid_specified(ctx->txn);
     }
 
     row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
@@ -2465,7 +2480,7 @@  static const struct ctl_command_syntax db_ctl_commands[] = {
     {"clear", 3, INT_MAX, "TABLE RECORD COLUMN...", pre_cmd_clear, cmd_clear,
      NULL, "--if-exists", RW},
     {"create", 2, INT_MAX, "TABLE COLUMN[:KEY]=VALUE...", pre_create,
-     cmd_create, post_create, "--id=", RW},
+     cmd_create, post_create, "--id=,--row-uuid=", RW},
     {"destroy", 1, INT_MAX, "TABLE [RECORD]...", pre_cmd_destroy, cmd_destroy,
      NULL, "--if-exists,--all", RW},
     {"wait-until", 2, INT_MAX, "TABLE RECORD [COLUMN[:KEY]=VALUE]...",
diff --git a/lib/db-ctl-base.xml b/lib/db-ctl-base.xml
index 10124c3ad..2804f0865 100644
--- a/lib/db-ctl-base.xml
+++ b/lib/db-ctl-base.xml
@@ -290,7 +290,7 @@ 
       </p>
     </dd>
 
-    <dt>[<code>--id=@</code><var>name</var>] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt>
+    <dt>[<code>--id=@</code><var>name</var>] [<code>--row-uuid=</code><var>uuid</var>] <code>create</code> <var>table column</var>[<code>:</code><var>key</var>]<code>=</code><var>value</var>...</dt>
     <dd>
       <p>
         Creates a new record in <var>table</var> and sets the initial values of
@@ -303,6 +303,15 @@ 
         invocation in contexts where a UUID is expected.  Such references may
         precede or follow the <code>create</code> command.
       </p>
+      <p>
+        If <code></code><var>uuid</var> is supplied and is reasonable, then the UUID for the 
+        new row may be specified as expected. 
+      </p>
+      <p>
+        If both <code>@</code><var>name</var> and <code></code><var>uuid</var> are specified, 
+        the new row with corresponding uuid may still be referred to by that name, but this 
+        references can only follow the <code>create</code> command.   
+      </p>
       <dl>
         <dt>Caution (ovs-vsctl as example)</dt>
         <dd>
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 190143f36..505ed00d9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -281,6 +281,7 @@  struct ovsdb_idl_txn {
     char *error;
     bool dry_run;
     struct ds comment;
+    bool uuid_specified;
 
     /* Increments. */
     const char *inc_table;
@@ -3550,6 +3551,7 @@  ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     txn->status = TXN_UNCOMMITTED;
     txn->error = NULL;
     txn->dry_run = false;
+    txn->uuid_specified = false;
     ds_init(&txn->comment);
 
     txn->inc_table = NULL;
@@ -3591,6 +3593,11 @@  ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *txn)
     txn->dry_run = true;
 }
 
+void
+ovsdb_idl_txn_set_uuid_specified(struct ovsdb_idl_txn *txn)
+{
+    txn->uuid_specified = true;
+}
 /* Causes 'txn', when committed, to increment the value of 'column' within
  * 'row' by 1.  'column' must have an integer type.  After 'txn' commits
  * successfully, the client may retrieve the final (incremented) value of
@@ -3689,7 +3696,9 @@  ovsdb_idl_txn_get_row(const struct ovsdb_idl_txn *txn, const struct uuid *uuid)
 static struct json *
 substitute_uuids(struct json *json, const struct ovsdb_idl_txn *txn)
 {
-    if (json->type == JSON_ARRAY) {
+    if (txn->uuid_specified) {
+        return json;
+    } else if (json->type == JSON_ARRAY) {
         struct uuid uuid;
         size_t i;
 
@@ -4153,9 +4162,16 @@  ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
 
                 any_updates = true;
 
-                json_object_put(op, "uuid-name",
-                                json_string_create_nocopy(
-                                    ovsdb_data_row_name(&row->uuid)));
+
+                if (txn->uuid_specified) {
+                    json_object_put(op, "uuid",
+                                    json_string_create_nocopy(
+                                        xasprintf(UUID_FMT, UUID_ARGS(&row->uuid))));
+                } else {
+                    json_object_put(op, "uuid-name",
+                                    json_string_create_nocopy(
+                                        ovsdb_data_row_name(&row->uuid)));
+                }
 
                 insert = xmalloc(sizeof *insert);
                 insert->dummy = row->uuid;
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index 9f12ce320..ae9dc10e4 100644
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -316,6 +316,7 @@  struct ovsdb_idl_txn *ovsdb_idl_txn_create(struct ovsdb_idl *);
 void ovsdb_idl_txn_add_comment(struct ovsdb_idl_txn *, const char *, ...)
     OVS_PRINTF_FORMAT (2, 3);
 void ovsdb_idl_txn_set_dry_run(struct ovsdb_idl_txn *);
+void ovsdb_idl_txn_set_uuid_specified(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_increment(struct ovsdb_idl_txn *,
                              const struct ovsdb_idl_row *,
                              const struct ovsdb_idl_column *,
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 55c7a6e17..d91ec24bb 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1118,6 +1118,13 @@  AT_CHECK([RUN_OVS_VSCTL([list-dp-cap nosystem])],
   [1], [], [ovs-vsctl: datapath "nosystem" record not found
 ])
 
+AT_CHECK(
+  [RUN_OVS_VSCTL_TOGETHER([--id=@br0 --row-uuid=a22557c4-bf1b-4ed5-8dd8 create bridge name=br123],
+                          [set b br123 name=br0],
+                          [set o . bridges=@br0])],
+  [1], [ignore], [ovs-vsctl: row-uuid 'a22557c4-bf1b-4ed5-8dd8' is not a valid UUID
+])
+
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
@@ -1349,6 +1356,56 @@  name                : br0
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
+AT_SETUP([--row-uuid option on create, together with --id])
+AT_KEYWORDS([ovs-vsctl])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL([add-br br0],
+                        [add-port br0 eth0],
+                        [add-port br0 eth1])])
+
+AT_CHECK([RUN_OVS_VSCTL([--no-wait  --oneline  -- --id=@eth0 get port eth0 -- --id=@eth1 get port eth1 -- --id=@m --row-uuid=ee07ca13-b85e-4b65-a0a2-e6f60d08dfbd create mirror name=mymirror select-dst-port=@eth0 select-src-port=@eth0 output-port=@eth1 -- set bridge br0 mirrors=@m])],
+ [0], [stdout])
+
+AT_CHECK(
+  [uuidfilt stdout], [0], [dnl
+
+
+<0>
+
+])
+AT_CHECK([cat stdout], [0], [
+
+ee07ca13-b85e-4b65-a0a2-e6f60d08dfbd
+
+])
+
+AT_CHECK(
+  [RUN_OVS_VSCTL(
+    [list port eth0 eth1],
+    [list mirror],
+    [list bridge br0])],
+  [0], [stdout])
+AT_CHECK(
+  [sed -n -e '/uuid/p' -e '/name/p' -e '/mirrors/p' -e '/select/p' -e '/output/p' < stdout | uuidfilt], [0], [dnl
+[_uuid               : <0>
+name                : eth0
+_uuid               : <1>
+name                : eth1
+_uuid               : <2>
+name                : mymirror
+output_port         : <1>
+output_vlan         : []
+select_all          : false
+select_dst_port     : [<0>]
+select_src_port     : [<0>]
+select_vlan         : []
+_uuid               : <3>
+mirrors             : [<2>]
+name                : br0
+]])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
 AT_SETUP([unreferenced record warnings])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP