| Message ID | 20181008202353.29585-1-mmichels@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] ovn-nbctl: Add basic port group commands. | expand |
Bleep bloop. Greetings Mark Michelson, 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. git-am: <stdin>:194: new blank line at EOF. + warning: 1 line adds whitespace errors. Failed to merge in the changes. Patch failed at 0001 ovn-nbctl: Add basic port group commands. The copy of the patch that failed is found in: /var/lib/jenkins/jobs/upstream_build_from_pw/workspace/.git/rebase-apply/patch When you have resolved this problem, run "git am --resolved". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Please check this out. If you feel there has been an error, please email aconole@bytheb.org Thanks, 0-day Robot
On Mon, Oct 8, 2018 at 1:24 PM Mark Michelson <mmichels@redhat.com> wrote: > > This adds the following commands: > > pg-add: Add a new port group, optionally adding switch ports at > creation. > pg-set-ports: Sets the logical switch ports on a port group > pg-del: Remove a port group. > > The main motivation for these commands is that it allows for adding > logical switch ports by name rather than UUID. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > v1 -> v2 > Moved tests to tests/ovn-nbctl.at. This revealed issues when listing > database information in ovn-nbctl daemon mode. These issues are fixed in > series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636 > Therefore this patch is dependent on the linked series. > --- > ovn/utilities/ovn-nbctl.8.xml | 22 +++++++++++++ > ovn/utilities/ovn-nbctl.c | 72 +++++++++++++++++++++++++++++++++++++++++++ > tests/ovn-nbctl.at | 34 +++++++++++++++++++- > tests/ovn.at | 1 + > 4 files changed, 128 insertions(+), 1 deletion(-) > > diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml > index 5e18fa7c0..14cc02f53 100644 > --- a/ovn/utilities/ovn-nbctl.8.xml > +++ b/ovn/utilities/ovn-nbctl.8.xml > @@ -881,6 +881,28 @@ > </dd> > </dl> > > + <h1>Port Group commands</h1> > + > + <dl> > + <dt><code>pg-add</code> <var>group</var> [<var>port</var>]...</dt> > + <dd> > + Creates a new port group in the <code>Port_Group</code> table named > + <code>group</code> with optional <code>ports</code> added to the group. > + </dd> > + > + <dt><code>pg-set-ports</code> <var>group</var> <var>port</var>...</dt> > + <dd> > + Sets <code>ports</code> on the port group named <code>group</code>. It > + is an error if <code>group</code> does not exist. > + </dd> > + > + <dt><code>pg-del</code> <var>group</var></dt> > + <dd> > + Deletes port group <code>group</code>. It is an error if > + <code>group</code> does not exist. > + </dd> > + </dl> > + > <h1>Database Commands</h1> > <p>These commands query and modify the contents of <code>ovsdb</code> tables. > They are a slight abstraction of the <code>ovsdb</code> interface and > diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c > index bff6d1380..89f0d6213 100644 > --- a/ovn/utilities/ovn-nbctl.c > +++ b/ovn/utilities/ovn-nbctl.c > @@ -4655,6 +4655,73 @@ cmd_set_ssl(struct ctl_context *ctx) > nbrec_nb_global_set_ssl(nb_global, ssl); > } > > +static char * > +set_ports_on_pg(struct ctl_context *ctx, const struct nbrec_port_group *pg, > + char **new_ports, size_t num_new_ports) > +{ > + struct nbrec_logical_switch_port **lports; > + lports = xmalloc(sizeof *lports * num_new_ports); > + > + size_t i; > + char *error = NULL; > + for (i = 0; i < num_new_ports; i++) { > + const struct nbrec_logical_switch_port *lsp; > + error = lsp_by_name_or_uuid(ctx, new_ports[i], true, &lsp); > + if (error) { > + goto out; > + } > + lports[i] = (struct nbrec_logical_switch_port *) lsp; > + } > + > + nbrec_port_group_set_ports(pg, lports, num_new_ports); > + > +out: > + free(lports); > + return error; > +} > + > +static void > +cmd_pg_add(struct ctl_context *ctx) > +{ > + const struct nbrec_port_group *pg; > + > + pg = nbrec_port_group_insert(ctx->txn); > + nbrec_port_group_set_name(pg, ctx->argv[1]); > + if (ctx->argc > 2) { > + ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2); > + } > +} > + > +static void > +cmd_pg_set_ports(struct ctl_context *ctx) > +{ > + const struct nbrec_port_group *pg; > + > + char *error; > + error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg); > + if (error) { > + ctx->error = error; > + return; > + } > + > + ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2); > +} > + > +static void > +cmd_pg_del(struct ctl_context *ctx) > +{ > + const struct nbrec_port_group *pg; > + > + char *error; > + error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg); > + if (error) { > + ctx->error = error; > + return; > + } > + > + nbrec_port_group_delete(pg); > +} > + > static const struct ctl_table_class tables[NBREC_N_TABLES] = { > [NBREC_TABLE_DHCP_OPTIONS].row_ids > = {{&nbrec_logical_switch_port_col_name, NULL, > @@ -5123,6 +5190,11 @@ static const struct ctl_command_syntax nbctl_commands[] = { > "PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]", > pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW}, > > + /* Port Group Commands */ > + {"pg-add", 1, INT_MAX, "", NULL, cmd_pg_add, NULL, "", RW }, > + {"pg-set-ports", 2, INT_MAX, "", NULL, cmd_pg_set_ports, NULL, "", RW }, > + {"pg-del", 1, 1, "", NULL, cmd_pg_del, NULL, "", RW }, > + > {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO}, > }; > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index 3f89874ba..25414b8bd 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -1518,7 +1518,6 @@ AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl > "pg0" > ])]) > > - > OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [ > dnl This test addresses a specific issue seen when running ovn-nbctl in > dnl daemon mode. All we have to do is ensure that each time we list database > @@ -1539,3 +1538,36 @@ AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore]) > AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl > sw1 > ])]) > +dnl --------------------------------------------------------------------- > + > +OVN_NBCTL_TEST([ovn_nbctl_port_group_commands], [port group commands], [ > +AT_CHECK([ovn-nbctl pg-add pg1], [0], [ignore]) > +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0], > +[pg1 > +]) > + > +AT_CHECK([ovn-nbctl pg-del pg1], [0], [ignore]) > +AT_CHECK([ovn-nbctl list port_group], [0], []) > + > +AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore]) > +AT_CHECK([ovn-nbctl lsp-add sw1 sw1-p1], [0], [ignore]) > +SW1P1=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p1) > +AT_CHECK([ovn-nbctl lsp-add sw1 sw1-p2], [0], [ignore]) > +SW1P2=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p2) > + > +AT_CHECK([ovn-nbctl pg-add pg1 sw1-p1], [0], [ignore]) > +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0],[dnl > +pg1 > +]) > +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0], [dnl > +$SW1P1 > +]) > + > +AT_CHECK([ovn-nbctl pg-set-ports pg1 sw1-p2], [0], [ignore]) > +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0], [dnl > +$SW1P2 > +]) > + > +AT_CHECK([ovn-nbctl pg-del pg1], [0], [ignore]) > +AT_CHECK([ovn-nbctl list port_group], [0], []) > +]) > diff --git a/tests/ovn.at b/tests/ovn.at > index 769e09f81..b747a7992 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -11253,3 +11253,4 @@ AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 192.168.0.3"]) > AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 aef0::1"]) > > AT_CLEANUP > + > -- > 2.14.4 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev Thanks Mark. There is an unnecessary file change - ovn.at, where only a blank line is added. Otherwise: Acked-by: Han Zhou <hzhou8@ebay.com>
On Mon, Oct 08, 2018 at 04:23:53PM -0400, Mark Michelson wrote: > This adds the following commands: > > pg-add: Add a new port group, optionally adding switch ports at > creation. > pg-set-ports: Sets the logical switch ports on a port group > pg-del: Remove a port group. > > The main motivation for these commands is that it allows for adding > logical switch ports by name rather than UUID. > > Signed-off-by: Mark Michelson <mmichels@redhat.com> > --- > v1 -> v2 > Moved tests to tests/ovn-nbctl.at. This revealed issues when listing > database information in ovn-nbctl daemon mode. These issues are fixed in > series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636 > Therefore this patch is dependent on the linked series. Besides Han's minor comment, would you mind updating the usage message also? Thanks, Ben.
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml index 5e18fa7c0..14cc02f53 100644 --- a/ovn/utilities/ovn-nbctl.8.xml +++ b/ovn/utilities/ovn-nbctl.8.xml @@ -881,6 +881,28 @@ </dd> </dl> + <h1>Port Group commands</h1> + + <dl> + <dt><code>pg-add</code> <var>group</var> [<var>port</var>]...</dt> + <dd> + Creates a new port group in the <code>Port_Group</code> table named + <code>group</code> with optional <code>ports</code> added to the group. + </dd> + + <dt><code>pg-set-ports</code> <var>group</var> <var>port</var>...</dt> + <dd> + Sets <code>ports</code> on the port group named <code>group</code>. It + is an error if <code>group</code> does not exist. + </dd> + + <dt><code>pg-del</code> <var>group</var></dt> + <dd> + Deletes port group <code>group</code>. It is an error if + <code>group</code> does not exist. + </dd> + </dl> + <h1>Database Commands</h1> <p>These commands query and modify the contents of <code>ovsdb</code> tables. They are a slight abstraction of the <code>ovsdb</code> interface and diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index bff6d1380..89f0d6213 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -4655,6 +4655,73 @@ cmd_set_ssl(struct ctl_context *ctx) nbrec_nb_global_set_ssl(nb_global, ssl); } +static char * +set_ports_on_pg(struct ctl_context *ctx, const struct nbrec_port_group *pg, + char **new_ports, size_t num_new_ports) +{ + struct nbrec_logical_switch_port **lports; + lports = xmalloc(sizeof *lports * num_new_ports); + + size_t i; + char *error = NULL; + for (i = 0; i < num_new_ports; i++) { + const struct nbrec_logical_switch_port *lsp; + error = lsp_by_name_or_uuid(ctx, new_ports[i], true, &lsp); + if (error) { + goto out; + } + lports[i] = (struct nbrec_logical_switch_port *) lsp; + } + + nbrec_port_group_set_ports(pg, lports, num_new_ports); + +out: + free(lports); + return error; +} + +static void +cmd_pg_add(struct ctl_context *ctx) +{ + const struct nbrec_port_group *pg; + + pg = nbrec_port_group_insert(ctx->txn); + nbrec_port_group_set_name(pg, ctx->argv[1]); + if (ctx->argc > 2) { + ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2); + } +} + +static void +cmd_pg_set_ports(struct ctl_context *ctx) +{ + const struct nbrec_port_group *pg; + + char *error; + error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg); + if (error) { + ctx->error = error; + return; + } + + ctx->error = set_ports_on_pg(ctx, pg, ctx->argv + 2, ctx->argc - 2); +} + +static void +cmd_pg_del(struct ctl_context *ctx) +{ + const struct nbrec_port_group *pg; + + char *error; + error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, &pg); + if (error) { + ctx->error = error; + return; + } + + nbrec_port_group_delete(pg); +} + static const struct ctl_table_class tables[NBREC_N_TABLES] = { [NBREC_TABLE_DHCP_OPTIONS].row_ids = {{&nbrec_logical_switch_port_col_name, NULL, @@ -5123,6 +5190,11 @@ static const struct ctl_command_syntax nbctl_commands[] = { "PRIVATE-KEY CERTIFICATE CA-CERT [SSL-PROTOS [SSL-CIPHERS]]", pre_cmd_set_ssl, cmd_set_ssl, NULL, "--bootstrap", RW}, + /* Port Group Commands */ + {"pg-add", 1, INT_MAX, "", NULL, cmd_pg_add, NULL, "", RW }, + {"pg-set-ports", 2, INT_MAX, "", NULL, cmd_pg_set_ports, NULL, "", RW }, + {"pg-del", 1, 1, "", NULL, cmd_pg_del, NULL, "", RW }, + {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO}, }; diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 3f89874ba..25414b8bd 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1518,7 +1518,6 @@ AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl "pg0" ])]) - OVN_NBCTL_TEST([ovn_nbctl_extra_newlines], [extra newlines], [ dnl This test addresses a specific issue seen when running ovn-nbctl in dnl daemon mode. All we have to do is ensure that each time we list database @@ -1539,3 +1538,36 @@ AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore]) AT_CHECK([ovn-nbctl --bare --columns=name list logical_switch sw1], [0], [dnl sw1 ])]) +dnl --------------------------------------------------------------------- + +OVN_NBCTL_TEST([ovn_nbctl_port_group_commands], [port group commands], [ +AT_CHECK([ovn-nbctl pg-add pg1], [0], [ignore]) +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0], +[pg1 +]) + +AT_CHECK([ovn-nbctl pg-del pg1], [0], [ignore]) +AT_CHECK([ovn-nbctl list port_group], [0], []) + +AT_CHECK([ovn-nbctl ls-add sw1], [0], [ignore]) +AT_CHECK([ovn-nbctl lsp-add sw1 sw1-p1], [0], [ignore]) +SW1P1=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p1) +AT_CHECK([ovn-nbctl lsp-add sw1 sw1-p2], [0], [ignore]) +SW1P2=$(ovn-nbctl --bare --columns=_uuid list logical_switch_port sw1-p2) + +AT_CHECK([ovn-nbctl pg-add pg1 sw1-p1], [0], [ignore]) +AT_CHECK([ovn-nbctl --bare --columns=name list port_group pg1], [0],[dnl +pg1 +]) +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0], [dnl +$SW1P1 +]) + +AT_CHECK([ovn-nbctl pg-set-ports pg1 sw1-p2], [0], [ignore]) +AT_CHECK_UNQUOTED([ovn-nbctl --bare --columns=ports list port_group pg1], [0], [dnl +$SW1P2 +]) + +AT_CHECK([ovn-nbctl pg-del pg1], [0], [ignore]) +AT_CHECK([ovn-nbctl list port_group], [0], []) +]) diff --git a/tests/ovn.at b/tests/ovn.at index 769e09f81..b747a7992 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -11253,3 +11253,4 @@ AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 192.168.0.3"]) AT_CHECK([ovn-nbctl lsp-set-addresses sw2-p1 "00:00:00:00:00:04 aef0::1"]) AT_CLEANUP +
This adds the following commands: pg-add: Add a new port group, optionally adding switch ports at creation. pg-set-ports: Sets the logical switch ports on a port group pg-del: Remove a port group. The main motivation for these commands is that it allows for adding logical switch ports by name rather than UUID. Signed-off-by: Mark Michelson <mmichels@redhat.com> --- v1 -> v2 Moved tests to tests/ovn-nbctl.at. This revealed issues when listing database information in ovn-nbctl daemon mode. These issues are fixed in series https://patchwork.ozlabs.org/project/openvswitch/list/?series=69636 Therefore this patch is dependent on the linked series. --- ovn/utilities/ovn-nbctl.8.xml | 22 +++++++++++++ ovn/utilities/ovn-nbctl.c | 72 +++++++++++++++++++++++++++++++++++++++++++ tests/ovn-nbctl.at | 34 +++++++++++++++++++- tests/ovn.at | 1 + 4 files changed, 128 insertions(+), 1 deletion(-)