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

[Timo_Liu]

Hi Ben:




     Thanks!

     The answer is as below:
>Command line option names never contain _. Use - instead.

Already fixed in v7

>This introduces duplicate code blocks into cmd_create(). Don't do that.

We can't understand: do you mean that the row-uuid code block under if(id) is redundant ? Or we should add some additional code block ?

If 

>There should be no space in "return;"
Already fixed in v7


>Every error message in ovs-vsctl should have a test under "database
>commands -- negative checks" that provokes it.
Already added in v7


>Please add a test that using an id before creating the record works with
>--row-uuid. I do not think that it currently works.

In patch v6 

We have added a test in ovs-vsctl.at which is about --row-uuid after --id for 

create cmd.

Any other tests should we add then ?









Best Regards

Timo

 



 Re: [ovs-dev] [PATCH v6] db-ctl-base: add uuid specified method for create cmdOn Thu, Mar 19, 2020 at 06:24:48PM +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'
>
> Author: Liu Chang <liuchang at cmss.chinamobile.com>
> Co-authored-by: Tao YunXiang <taoyunxiang at cmss.chinamobile.com>
> Co-authored-by: Rong Yin <rongyin at cmss.chinamobile.com>
> Signed-off-by: Tao YunXiang <taoyunxiang at cmss.chinamobile.com>
> Signed-off-by: Liu Chang <liuchang at cmss.chinamobile.com>
> Signed-off-by: Rong Yin <rongyin at cmss.chinamobile.com>
> CC: Ben Pfaff<blp at ovn.org>

Thanks for the updated patch.

Command line option names never contain _.  Use - instead.

This introduces duplicate code blocks into cmd_create().  Don't do that.

There should be no space in "return;"

Every error message in ovs-vsctl should have a test under "database
commands -- negative checks" that provokes it.

Please add a test that using an id before creating the record works with
--row-uuid.  I do not think that it currently works.