[ovs-dev,v2] db-ctl-base: add uuid specified method for create cmd
diff mbox series

Message ID 20200206091733.27899-1-taoyunxiang@cmss.chinamobile.com
State New
Headers show
Series
  • [ovs-dev,v2] db-ctl-base: add uuid specified method for create cmd
Related show

Commit Message

Tao YunXiang Feb. 6, 2020, 9:17 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'

Co-authored-by: Liu Chang <liuchang@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: Han Zhou <hzhou@ovn.org>

---
 lib/db-ctl-base.c | 15 +++++++++++++--
 lib/ovsdb-data.c  |  9 +++++++++
 lib/ovsdb-data.h  |  1 +
 lib/ovsdb-idl.c   | 13 +++++++++++++
 lib/ovsdb-idl.h   |  1 +
 5 files changed, 37 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Feb. 6, 2020, 8:54 p.m. UTC | #1
On Thu, Feb 06, 2020 at 05:17:33PM +0800, Tao YunXiang wrote:
> 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'
> 
> Co-authored-by: Liu Chang <liuchang@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: Han Zhou <hzhou@ovn.org>

Thanks for working on this.

This warrants an item in NEWS and documentation for the new feature.

I don't think ovsdb_data_row_uuid() is a very useful function.  I would
drop it.

ovsdb_idl_txn_commit() can be simplified if uuids are specified, because
it's no longer necessary to specify named-uuids or to call
substitute_uuids().
Tao YunXiang Feb. 18, 2020, 8:13 a.m. UTC | #2
Hi,Ben,
           Thanks for your reply.  Below is is my commet .

--------------
taoyunxiang@cmss.chinamobile.com
>On Thu, Feb 06, 2020 at 05:17:33PM +0800, Tao YunXiang wrote:
>> 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'
>>
>> Co-authored-by: Liu Chang <liuchang@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: Han Zhou <hzhou@ovn.org>
>
>Thanks for working on this.
>
>This warrants an item in NEWS and documentation for the new feature.
> 
Done. I have summit a new patch set3.

>I don't think ovsdb_data_row_uuid() is a very useful function.  I would
>drop it.
Done. I have summit a new patch set3.

>ovsdb_idl_txn_commit() can be simplified if uuids are specified, because
>it's no longer necessary to specify named-uuids or to call
>substitute_uuids(). 

We have try to work on this , but it is complicated for us , I can't even distinguish between named-uuid and uuid-name. Could you give some advice or work on this  patch? Thanks :)
Ben Pfaff Feb. 28, 2020, 11:47 p.m. UTC | #3
On Tue, Feb 18, 2020 at 04:13:04PM +0800, taoyunxiang@cmss.chinamobile.com wrote:
> >ovsdb_idl_txn_commit() can be simplified if uuids are specified, because
> >it's no longer necessary to specify named-uuids or to call
> >substitute_uuids(). 
> 
> We have try to work on this , but it is complicated for us , I can't even distinguish between named-uuid and uuid-name. Could you give some advice or work on this  patch? Thanks :)

I followed up to v3 with a suggestion.  If you like it, please fold it
into your patch and submit as v4.

Thanks,

Ben.
Tao YunXiang March 2, 2020, 3:29 a.m. UTC | #4
Hi, ben

I haved submit it as v4. Thanks very much for your help. :)

Regards,
Yun

--------------
taoyunxiang@cmss.chinamobile.com
>On Tue, Feb 18, 2020 at 04:13:04PM +0800, taoyunxiang@cmss.chinamobile.com wrote:
>> >ovsdb_idl_txn_commit() can be simplified if uuids are specified, because
>> >it's no longer necessary to specify named-uuids or to call
>> >substitute_uuids().
>>
>> We have try to work on this , but it is complicated for us , I can't even distinguish between named-uuid and uuid-name. Could you give some advice or work on this  patch? Thanks :)
>
>I followed up to v3 with a suggestion.  If you like it, please fold it
>into your patch and submit as v4.
>
>Thanks,
>
>Ben.

Patch
diff mbox series

diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab2af9eda..522273210 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1718,10 +1718,12 @@  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);
@@ -1741,7 +1743,16 @@  cmd_create(struct ctl_context *ctx)
              * warnings about that by pretending that there is a reference. */
             symbol->strong_ref = true;
         }
-        uuid = &symbol->uuid;
+        if (!row_uuid) {
+            uuid = &symbol->uuid;
+        }
+    }
+    if (row_uuid) {
+        if (!uuid_from_string(&uuid_from_cmd, row_uuid)) {
+            return ;
+        }
+        uuid = &uuid_from_cmd;
+        ovsdb_idl_txn_set_uuid_specified(ctx->txn);
     }
 
     row = ovsdb_idl_txn_insert(ctx->txn, table, uuid);
@@ -2465,7 +2476,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/ovsdb-data.c b/lib/ovsdb-data.c
index 4828624f6..ea87206a7 100644
--- a/lib/ovsdb-data.c
+++ b/lib/ovsdb-data.c
@@ -2239,3 +2239,12 @@  ovsdb_data_row_name(const struct uuid *uuid)
 
     return name;
 }
+
+char *
+ovsdb_data_row_uuid(const struct uuid *uuid)
+{
+    char *row_uuid;
+
+    row_uuid = xasprintf(UUID_FMT, UUID_ARGS(uuid));
+    return row_uuid;
+}
diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h
index c5a80ee39..3b0439b3e 100644
--- a/lib/ovsdb-data.h
+++ b/lib/ovsdb-data.h
@@ -259,6 +259,7 @@  void ovsdb_datum_add_unsafe(struct ovsdb_datum *,
 struct json *ovsdb_datum_to_json_with_row_names(const struct ovsdb_datum *,
                                                 const struct ovsdb_type *);
 char *ovsdb_data_row_name(const struct uuid *);
+char *ovsdb_data_row_uuid(const struct uuid *);
 
 /* Type checking. */
 static inline bool
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 190143f36..d801c22a0 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
@@ -4157,6 +4164,12 @@  ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                                 json_string_create_nocopy(
                                     ovsdb_data_row_name(&row->uuid)));
 
+                if (txn->uuid_specified) {
+                    json_object_put(op, "uuid",
+                                    json_string_create_nocopy(
+                                        ovsdb_data_row_uuid(&row->uuid)));
+                }
+
                 insert = xmalloc(sizeof *insert);
                 insert->dummy = row->uuid;
                 insert->op_index = operations->array.n - 1;
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 *,