Re: [ovs-dev] [PATCH] AUTHORS: Update email address for Jakub Sitnicki.
On Thu, 2 Aug 2018 13:00:38 -0700 Ben Pfaff wrote: > On Thu, Aug 02, 2018 at 11:48:26AM -0400, Aaron Conole wrote: > > Jakub Sitnicki writes: > > > > > Signed-off-by: Jakub Sitnicki > > > --- > > > > > > The j...@redhat.com address will be valid only until August 4th. > > > > :'( > > > > With sadness: > > > > Acked-by: Aaron Conole > > Jakub, I'm sorry to see you go. > > I applied this to all relevant branches. Thank you for the kind words, guys. I've really enjoyed being part of OVS/OVN community. Until next time, Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v6] ovn: Allow for automatic dynamic updates of IPAM
Hi Mark,
On Thu, 2 Aug 2018 08:18:12 -0400
Mark Michelson wrote:
> OVN offers a method of IP address management that allows for an IPv4
> subnet or IPv6 prefix to be specified on a logical switch. Then by
> specifying a switch port's address as "dynamic" or "
> dynamic", OVN will automatically assign addresses to the switch port.
>
> While this works great for initial assignment of addresses, addresses
> do not automatically adjust when changes are made to the switch's
> configuration. For instance:
> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
> changes, the affected switch ports are not updated.
> * If a switch port with a static IP address is added to the switch,
> and that address conflicts with a dynamically assigned IP address, the
> dynamic address is not updated.
> * If a MAC address switched from being statically assigned to
> dynamically assigned, the MAC address would not be updated.
> * If a statically assigned MAC address changed, then the IPv6 address
> would not be updated.
>
> This patch solves all of the above issues by changing the algorithm
> for IPAM assignment. There are essentially three steps.
> 1) While joining logical ports, all statically-assigned addresses
> (i.e. any ports without "dynamic" addresses) have their addresses
> registered to IPAM. This gives them top priority.
> 2) All logical ports with dynamic addresses are inspected. Any changes
> that must be made to the addresses are collected to be made later. Any
> addresses that do not require change are registered to IPAM. This
> allows for previously assigned dynamic addresses to be kept.
> 3) All gathered changes are enacted.
>
> The change contains new tests that ensure that dynamic addresses are
> updated when appropriate.
>
> This patch also alters some existing IPAM tests. Those tests assumed
> that dynamic addresses would not be updated automatically, so those
> tests either had to be altered or removed.
>
> Signed-off-by: Mark Michelson
> Acked-by: Jakub Sitnicki
> ---
> v5->v6:
> * Rebased
>
> v4->v5:
> Cleanups suggested by Jakub Sitnicki + rebase
> * Add some convenience pointers for shortened code.
> * Separate checking of updates of dynamic addresses and registration
>of unchanged addresses.
> * Use OVS_NOT_REACHED() instead of ovs_assert(0)
>
> v3->v4:
> Print warning when multiple dynamic addresses are configured on a
> switch port. Ensure that dynamic addresses beyond the first on a
> switch port are ignored. Found by Ben Pfaff.
>
> v2->v3:
> Fixed a checkpatch problem (line too long)
>
> v1->v2:
> Rebased
> ---
Recent fix pushed to master, 4d0214a365ae ("ovn: Fix typos in "ovn --
Address Set generation... test."), will cause the following test to
fail with this patch applied:
2578: ovn -- Address Set generation from Port Groups (dynamic addressing)
To fix the test you need to revert the mentioned commit and squash the
diff with your changes.
Thanks,
Jakub
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] AUTHORS: Update email address for Jakub Sitnicki.
Signed-off-by: Jakub Sitnicki --- The j...@redhat.com address will be valid only until August 4th. AUTHORS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 25ee39d75..f3ac8e32b 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -161,7 +161,7 @@ Isaku Yamahata yamah...@valinux.co.jp Ivan Dyukovi.dyu...@samsung.com IWASE Yusuke iwase.yus...@gmail.com Jakub Libosvar libos...@redhat.com -Jakub Sitnicki j...@redhat.com +Jakub Sitnicki jsitni...@gmail.com James P. roamp...@gmail.com James Page james.p...@ubuntu.com Jamie Lennox jamielen...@gmail.com -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 3/3] ovn-northd: Propagate dynamic addresses to port group address sets.
Hi Simon, On Wed, 1 Aug 2018 11:23:30 +0200 Simon Horman wrote: > Hi Jakub, > > On Mon, Jul 30, 2018 at 04:37:49PM +0200, Jakub Sitnicki wrote: > > If a logical switch port belongs to a port group and has dynamic > > addresses assigned, propagate the addresses to the auto-generated > > address sets for the port group. > > > > Signed-off-by: Jakub Sitnicki > > Acked-by: Han Zhou > > --- > > ovn/northd/ovn-northd.c | 34 > > tests/ovn.at| 59 > > + > > 2 files changed, 84 insertions(+), 9 deletions(-) > > The test added by this patch, which is now present in the master branch, > seems to fail. > > https://travis-ci.org/openvswitch/ovs/jobs/410550769 Thanks for the heads-up. These patches depend on another patch that is still under review. Hence the failure. I've already notified Ben, who he has proposed a fix-up similar to yours: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/350390.html -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] ovn: Fix typos in "ovn -- Address Set generation..." test.
On Tue, 31 Jul 2018 12:45:41 -0700
Ben Pfaff wrote:
> These caused the test to fail.
>
> CC: Jakub Sitnicki
> Fixes: 984c7d5ea8fe ("ovn-northd: Propagate dynamic addresses to port group
> address sets.")
> Signed-off-by: Ben Pfaff
> ---
> tests/ovn.at | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 17c740aa2352..1b25b6e4d6b9 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -10361,10 +10361,10 @@ ovn-nbctl set Logical_Switch ls1 \
>
> dnl Check if updated address got propagated to the port group address sets
> AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
> - [0], [[["10.11.0.2", "10.2.0.2"]]
> + [0], [[["10.1.0.2", "10.2.0.2"]]
> ])
> AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
> - [0], [[["2001:db8:11::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
> + [0], [[["2001:db8:1::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
> ])
>
> AT_CLEANUP
Oops, these weren't typos. It fails because the series depends on
Mark's IPAM changes:
https://patchwork.ozlabs.org/patch/947155/
Without it the dynamic addresses don't get updated when subnet/prefix
for the switch changes.
FWIW, I've mentioned it in the cover letter:
https://mail.openvswitch.org/pipermail/ovs-dev/2018-July/350143.html
-Jakub
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] Introduce ovs-appctl command to monitor HVs sb connection status
On Tue, 31 Jul 2018 17:35:00 +0200 Lorenzo Bianconi wrote: > Add 'connection-status' command to ovs-appctl utility in order to check > if a given chassis is currently connected to SB db > > Acked-by: Mark Michelson > Co-authored-by: aginwala > Signed-off-by: aginwala > Signed-off-by: Lorenzo Bianconi > --- > Changes since v3: > - add const qualifier to result and idl pointers in > ovn_controller_conn_show routine > - remove idl check before running ovsdb_idl_is_connected routine > Changes since v2: > - check idl->session pointer in ovsdb_idl_is_connected routine > Changes since v1: > - add unit tests for ovs-appctl -t ovn-controller connection-status > command > - use jsonrpc_session_is_connected() in ovsdb_idl_is_connected routine > --- Thanks for the fix-ups. Acked-by: Jakub Sitnicki ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3] Introduce ovs-appctl command to monitor HVs sb connection status
On Tue, 31 Jul 2018 15:22:38 +0200
Lorenzo Bianconi wrote:
> Add 'connection-status' command to ovs-appctl utility in order to check
> if a given chassis is currently connected to SB db
>
> Acked-by: Mark Michelson
> Co-authored-by: aginwala
> Signed-off-by: aginwala
> Signed-off-by: Lorenzo Bianconi
> ---
> Changes since v2:
> - check idl->session pointer in ovsdb_idl_is_connected routine
>
> Changes since v1:
> - add unit tests for ovs-appctl -t ovn-controller connection-status
> command
> - use jsonrpc_session_is_connected() in ovsdb_idl_is_connected routine
> ---
> lib/ovsdb-idl.c | 6 +
> lib/ovsdb-idl.h | 1 +
> ovn/controller/ovn-controller.8.xml | 5 +
> ovn/controller/ovn-controller.c | 17 +++
> tests/ovn-controller.at | 34 +
> 5 files changed, 63 insertions(+)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index ae0a55c3a..40c29cbd1 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
> idl->state != IDL_S_ERROR;
> }
>
> +bool
> +ovsdb_idl_is_connected(const struct ovsdb_idl *idl)
> +{
> +return idl->session && jsonrpc_session_is_connected(idl->session);
> +}
> +
> /* Returns the last error reported on a connection by 'idl'. The return
> value
> * is 0 only if no connection made by 'idl' has ever encountered an error and
> * a negative response to a schema request has never been received. See
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index ea18b22f5..5f933b628 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
> void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
>
> bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
> +bool ovsdb_idl_is_connected(const struct ovsdb_idl *idl);
> int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
>
> void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int
> probe_interval);
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 2e4e53d6b..8035638b3 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -398,6 +398,11 @@
> 0x800.
>
>
> +
> + connection-status
> +
> +Show OVN SBDB connection status for the chassis.
> +
>
>
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 470e02ae8..bc4e8014b 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -68,6 +68,7 @@ static unixctl_cb_func ct_zone_list;
> static unixctl_cb_func meter_table_list;
> static unixctl_cb_func group_table_list;
> static unixctl_cb_func inject_pkt;
> +static unixctl_cb_func ovn_controller_conn_show;
>
> #define DEFAULT_BRIDGE_NAME "br-int"
> #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> @@ -594,6 +595,9 @@ main(int argc, char *argv[])
> ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
> ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false);
>
> +unixctl_command_register("connection-status", "", 0, 0,
> + ovn_controller_conn_show, ovnsb_idl_loop.idl);
> +
> struct ovsdb_idl_index *sbrec_chassis_by_name
> = chassis_index_create(ovnsb_idl_loop.idl);
> struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
> @@ -1122,3 +1126,16 @@ update_probe_interval(const struct
> ovsrec_open_vswitch_table *ovs_table,
>
> ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
> }
> +
> +static void
> +ovn_controller_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *idl_)
> +{
> +char *result = "not connected";
> +struct ovsdb_idl *idl = idl_;
'result' definitely should be pointing to const data. 'idl' could be
declared as const as well.
> +
> +if (idl && ovsdb_idl_is_connected(idl)) {
It doesn't look like 'idl' can be null or we would be in trouble in
other code paths.
Sorry I didn't notice these the first time.
-Jakub
> + result = "connected";
> +}
> +unixctl_command_reply(conn, result);
> +}
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index ecc6a50da..13e88a0cc 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -228,3 +228,37 @@ as ovn-sb
> OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
> AT_CLEANUP
> +
> +# Check ovn-controller connection status to Southbound database
> +AT_SETUP([ovn-controller - check sbdb connection])
> +AT_KEYWORDS([ovn])
> +ovn_init_db ovn-sb
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl \
> +-- add-br br-phys \
> +-- add-br br-eth0 \
> +-- add-br br-eth1 \
> +-- add-br br-eth2
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +check_sbdb_connection () {
> +test
Re: [ovs-dev] [PATCH v2] Introduce ovs-appctl command to monitor HVs sb connection status
On Mon, 30 Jul 2018 17:10:23 +0200
Lorenzo Bianconi wrote:
> Add 'connection-status' command to ovs-appctl utility in order to check
> if a given chassis is currently connected to SB db
>
> Co-authored-by: aginwala
> Signed-off-by: aginwala
> Signed-off-by: Lorenzo Bianconi
> ---
> Changes since v1:
> - add unit tests for ovs-appctl -t ovn-controller connection-status
> command
> - use jsonrpc_session_is_connected() in ovsdb_idl_is_connected routine
> ---
> lib/ovsdb-idl.c | 6 +
> lib/ovsdb-idl.h | 1 +
> ovn/controller/ovn-controller.8.xml | 5 +
> ovn/controller/ovn-controller.c | 17 +++
> tests/ovn-controller.at | 34 +
> 5 files changed, 63 insertions(+)
>
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index ae0a55c3a..f181ae831 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -922,6 +922,12 @@ ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
> idl->state != IDL_S_ERROR;
> }
>
> +bool
> +ovsdb_idl_is_connected(const struct ovsdb_idl *idl)
> +{
> +return jsonrpc_session_is_connected(idl->session);
> +}
> +
This won't play well with changes proposed in:
https://patchwork.ozlabs.org/patch/931134/
As the patch description mentions, 'session' can now be null, while
jsonrpc_session_is_connected() expects it to be nonnull.
> /* Returns the last error reported on a connection by 'idl'. The return
> value
> * is 0 only if no connection made by 'idl' has ever encountered an error and
> * a negative response to a schema request has never been received. See
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index ea18b22f5..5f933b628 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -80,6 +80,7 @@ void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
> void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
>
> bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
> +bool ovsdb_idl_is_connected(const struct ovsdb_idl *idl);
> int ovsdb_idl_get_last_error(const struct ovsdb_idl *);
>
> void ovsdb_idl_set_probe_interval(const struct ovsdb_idl *, int
> probe_interval);
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 0eff2113f..0861edbc4 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -388,6 +388,11 @@
> 0x800.
>
>
> +
> + connection-status
> +
> +Show OVN SBDB connection status for the chassis.
> +
>
>
>
> diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c
> index 6ee72a9fa..3338d3318 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -66,6 +66,7 @@ VLOG_DEFINE_THIS_MODULE(main);
> static unixctl_cb_func ovn_controller_exit;
> static unixctl_cb_func ct_zone_list;
> static unixctl_cb_func inject_pkt;
> +static unixctl_cb_func ovn_controller_conn_show;
>
> #define DEFAULT_BRIDGE_NAME "br-int"
> #define DEFAULT_PROBE_INTERVAL_MSEC 5000
> @@ -588,6 +589,9 @@ main(int argc, char *argv[])
> ovsdb_idl_create(ovnsb_remote, _idl_class, true, true));
> ovsdb_idl_set_leader_only(ovnsb_idl_loop.idl, false);
>
> +unixctl_command_register("connection-status", "", 0, 0,
> + ovn_controller_conn_show, ovnsb_idl_loop.idl);
> +
> struct ovsdb_idl_index *sbrec_chassis_by_name
> = chassis_index_create(ovnsb_idl_loop.idl);
> struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath
> @@ -1061,3 +1065,16 @@ update_probe_interval(const struct
> ovsrec_open_vswitch_table *ovs_table,
>
> ovsdb_idl_set_probe_interval(ovnsb_idl, interval);
> }
> +
> +static void
> +ovn_controller_conn_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
> + const char *argv[] OVS_UNUSED, void *idl_)
> +{
> +char *result = "not connected";
> +struct ovsdb_idl *idl = idl_;
> +
> +if (idl && ovsdb_idl_is_connected(idl)) {
> + result = "connected";
> +}
> +unixctl_command_reply(conn, result);
> +}
> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at
> index ecc6a50da..13e88a0cc 100644
> --- a/tests/ovn-controller.at
> +++ b/tests/ovn-controller.at
> @@ -228,3 +228,37 @@ as ovn-sb
> OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>
> AT_CLEANUP
> +
> +# Check ovn-controller connection status to Southbound database
> +AT_SETUP([ovn-controller - check sbdb connection])
> +AT_KEYWORDS([ovn])
> +ovn_init_db ovn-sb
> +
> +net_add n1
> +sim_add hv
> +as hv
> +ovs-vsctl \
> +-- add-br br-phys \
> +-- add-br br-eth0 \
> +-- add-br br-eth1 \
> +-- add-br br-eth2
> +ovn_attach n1 br-phys 192.168.0.1
> +
> +check_sbdb_connection () {
> +test "$(ovs-appctl -t ovn-controller connection-status)" = "$1"
> +}
> +
> +OVS_WAIT_UNTIL([check_sbdb_connection connected])
> +
> +ovs-vsctl set open .
Re: [ovs-dev] [PATCH v2 2/2] ovn: Modify restart_controller in ovn-ctl to use --restart
On Mon, 30 Jul 2018 09:47:45 -0400
Mark Michelson wrote:
> The --restart flag allows for uninterrupted packet flowage when exiting
> ovn-controller. This patch modifies the restart_controller argument to
> ovn-ctl to use --restart.
>
> Signed-off-by: Mark Michelson
> ---
> ovn/utilities/ovn-ctl | 4 ++--
> utilities/ovs-lib.in | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> index 2fce47714..05feed2b6 100755
> --- a/ovn/utilities/ovn-ctl
> +++ b/ovn/utilities/ovn-ctl
> @@ -350,7 +350,7 @@ stop_northd () {
> }
>
> stop_controller () {
> -OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller
> +OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller $@
> }
It needs to be "$@" or the arguments containing whitespace (or
characters from IFS) will get split up :-C
>
> stop_controller_vtep () {
> @@ -367,7 +367,7 @@ restart_northd () {
> }
>
> restart_controller () {
> -stop_controller
> +stop_controller --restart
> start_controller
> }
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 92f98ad92..9f62dfd25 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -258,7 +258,7 @@ stop_daemon () {
> case $action in
> EXIT)
> action "Exiting $1 ($pid)" \
> -${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl
> exit
> +${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl
> exit $2
> ;;
> TERM)
> action "Killing $1 ($pid)" kill $pid
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 3/3] ovn-northd: Propagate dynamic addresses to port group address sets.
On Sat, 28 Jul 2018 23:54:32 -0700 Han Zhou wrote: > On Thu, Jul 26, 2018 at 3:51 AM, Jakub Sitnicki wrote: > > > > If a logical switch port belongs to a port group and has dynamic > > addresses assigned, propagate the addresses to the auto-generated > > address sets for the port group. > > > > Signed-off-by: Jakub Sitnicki > > --- (...) > Thanks for the fix! Just a minor problem: > > It would be better to check if the address is dynamic using > is_dynamic_lsp_address() before calling split_addresses(). Otherwise, there > seems to be annoying INFO log: "invalid syntax '%s' in address" printed if > the address is "dynamic". > > Acked-by: Han Zhou Thanks a lot for the review, Han. I've posted v2 with the suggested fix and kept your acks: https://patchwork.ozlabs.org/project/openvswitch/list/?series=58323 -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 2/3] ovn-northd: Make use of svec for storing lists of addresses.
Get rid of what is, esentially, an open-coded version of svec.
Signed-off-by: Jakub Sitnicki
Acked-by: Han Zhou
---
ovn/northd/ovn-northd.c | 47 +++---
tests/ovn.at| 50 +
2 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 572022897..52f47c5ed 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -39,6 +39,7 @@
#include "openvswitch/poll-loop.h"
#include "smap.h"
#include "sset.h"
+#include "svec.h"
#include "stream.h"
#include "stream-ssl.h"
#include "unixctl.h"
@@ -6675,54 +6676,36 @@ sync_address_sets(struct northd_context *ctx)
/* sync port group generated address sets first */
const struct nbrec_port_group *nb_port_group;
NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {
-char **ipv4_addrs = xcalloc(1, sizeof *ipv4_addrs);
-size_t n_ipv4_addrs = 0;
-size_t n_ipv4_addrs_buf = 1;
-char **ipv6_addrs = xcalloc(1, sizeof *ipv6_addrs);
-size_t n_ipv6_addrs = 0;
-size_t n_ipv6_addrs_buf = 1;
+struct svec ipv4_addrs = SVEC_EMPTY_INITIALIZER;
+struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER;
for (size_t i = 0; i < nb_port_group->n_ports; i++) {
for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) {
struct lport_addresses laddrs;
extract_lsp_addresses(nb_port_group->ports[i]->addresses[j],
);
-while (n_ipv4_addrs_buf < n_ipv4_addrs + laddrs.n_ipv4_addrs) {
-n_ipv4_addrs_buf *= 2;
-ipv4_addrs = xrealloc(ipv4_addrs,
-n_ipv4_addrs_buf * sizeof *ipv4_addrs);
-}
for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
-ipv4_addrs[n_ipv4_addrs++] =
-xstrdup(laddrs.ipv4_addrs[k].addr_s);
-}
-while (n_ipv6_addrs_buf < n_ipv6_addrs + laddrs.n_ipv6_addrs) {
-n_ipv6_addrs_buf *= 2;
-ipv6_addrs = xrealloc(ipv6_addrs,
-n_ipv6_addrs_buf * sizeof *ipv6_addrs);
+svec_add(_addrs, laddrs.ipv4_addrs[k].addr_s);
}
for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
-ipv6_addrs[n_ipv6_addrs++] =
-xstrdup(laddrs.ipv6_addrs[k].addr_s);
+svec_add(_addrs, laddrs.ipv6_addrs[k].addr_s);
}
destroy_lport_addresses();
}
}
char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name);
char *ipv6_addrs_name = xasprintf("%s_ip6", nb_port_group->name);
-sync_address_set(ctx, ipv4_addrs_name, (const char **)ipv4_addrs,
- n_ipv4_addrs, _address_sets);
-sync_address_set(ctx, ipv6_addrs_name, (const char **)ipv6_addrs,
- n_ipv6_addrs, _address_sets);
+sync_address_set(ctx, ipv4_addrs_name,
+ /* "char **" is not compatible with "const char **" */
+ (const char **)ipv4_addrs.names,
+ ipv4_addrs.n, _address_sets);
+sync_address_set(ctx, ipv6_addrs_name,
+ /* "char **" is not compatible with "const char **" */
+ (const char **)ipv6_addrs.names,
+ ipv6_addrs.n, _address_sets);
free(ipv4_addrs_name);
free(ipv6_addrs_name);
-for (size_t i = 0; i < n_ipv4_addrs; i++) {
-free(ipv4_addrs[i]);
-}
-free(ipv4_addrs);
-for (size_t i = 0; i < n_ipv6_addrs; i++) {
-free(ipv6_addrs[i]);
-}
-free(ipv6_addrs);
+svec_destroy(_addrs);
+svec_destroy(_addrs);
}
/* sync user defined address sets, which may overwrite port group
diff --git a/tests/ovn.at b/tests/ovn.at
index e80c965a3..e7bdfe250 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10217,6 +10217,56 @@ done
OVN_CLEANUP([hv1], [hv2], [hv3])
AT_CLEANUP
+AT_SETUP([ovn -- Address Set generation from Port Groups (static addressing)])
+ovn_start
+
+ovn-nbctl ls-add ls1
+
+ovn-nbctl lsp-add ls1 lp1
+ovn-nbctl lsp-add ls1 lp2
+ovn-nbctl lsp-add ls1 lp3
+
+ovn-nbctl lsp-set-addresses lp1 "02:00:00:00:00:01 10.0.0.1 2001:db8::1"
+ovn-nbctl lsp-set-addresses lp2 "02:00:00:00:00:02 10.0.0.2 2001:db8::2"
+ovn-nbctl lsp-set-addresses lp3 "02:00:00:00:00:03 10.0.0.3 2001:db8::3"
+
+ovn-nbctl create Port_Group name=pg1
+ovn-nbctl create Port_Group name=
[ovs-dev] [PATCH v2 3/3] ovn-northd: Propagate dynamic addresses to port group address sets.
If a logical switch port belongs to a port group and has dynamic
addresses assigned, propagate the addresses to the auto-generated
address sets for the port group.
Signed-off-by: Jakub Sitnicki
Acked-by: Han Zhou
---
ovn/northd/ovn-northd.c | 34
tests/ovn.at| 59 +
2 files changed, 84 insertions(+), 9 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 52f47c5ed..d744836d3 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -6653,6 +6653,24 @@ sync_address_set(struct northd_context *ctx, const char
*name,
addrs, n_addrs);
}
+/* Go through 'addresses' and add found IPv4 addresses to 'ipv4_addrs' and IPv6
+ * addresses to 'ipv6_addrs'.
+ */
+static void
+split_addresses(const char *addresses, struct svec *ipv4_addrs,
+struct svec *ipv6_addrs)
+{
+struct lport_addresses laddrs;
+extract_lsp_addresses(addresses, );
+for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
+svec_add(ipv4_addrs, laddrs.ipv4_addrs[k].addr_s);
+}
+for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
+svec_add(ipv6_addrs, laddrs.ipv6_addrs[k].addr_s);
+}
+destroy_lport_addresses();
+}
+
/* OVN_Southbound Address_Set table contains same records as in north
* bound, plus the records generated from Port_Group table in north bound.
*
@@ -6680,16 +6698,14 @@ sync_address_sets(struct northd_context *ctx)
struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER;
for (size_t i = 0; i < nb_port_group->n_ports; i++) {
for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) {
-struct lport_addresses laddrs;
-extract_lsp_addresses(nb_port_group->ports[i]->addresses[j],
- );
-for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
-svec_add(_addrs, laddrs.ipv4_addrs[k].addr_s);
-}
-for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
-svec_add(_addrs, laddrs.ipv6_addrs[k].addr_s);
+const char *addrs = nb_port_group->ports[i]->addresses[j];
+if (!is_dynamic_lsp_address(addrs)) {
+split_addresses(addrs, _addrs, _addrs);
}
-destroy_lport_addresses();
+}
+if (nb_port_group->ports[i]->dynamic_addresses) {
+split_addresses(nb_port_group->ports[i]->dynamic_addresses,
+_addrs, _addrs);
}
}
char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name);
diff --git a/tests/ovn.at b/tests/ovn.at
index e7bdfe250..32e6c1738 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10267,6 +10267,65 @@ AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
AT_CLEANUP
+AT_SETUP([ovn -- Address Set generation from Port Groups (dynamic addressing)])
+ovn_start
+
+ovn-nbctl ls-add ls1
+ovn-nbctl ls-add ls2
+ovn-nbctl ls-add ls3
+
+ovn-nbctl set Logical_Switch ls1 \
+other_config:subnet=10.1.0.0/24 other_config:ipv6_prefix="2001:db8:1::"
+ovn-nbctl set Logical_Switch ls2 \
+other_config:subnet=10.2.0.0/24 other_config:ipv6_prefix="2001:db8:2::"
+ovn-nbctl set Logical_Switch ls3 \
+other_config:subnet=10.3.0.0/24 other_config:ipv6_prefix="2001:db8:3::"
+
+ovn-nbctl lsp-add ls1 lp1
+ovn-nbctl lsp-add ls2 lp2
+ovn-nbctl lsp-add ls3 lp3
+
+ovn-nbctl lsp-set-addresses lp1 "02:00:00:00:00:01 dynamic"
+ovn-nbctl lsp-set-addresses lp2 "02:00:00:00:00:02 dynamic"
+ovn-nbctl lsp-set-addresses lp3 "02:00:00:00:00:03 dynamic"
+
+ovn-nbctl create Port_Group name=pg1
+ovn-nbctl create Port_Group name=pg2
+
+ovn-nbctl --id=@p get Logical_Switch_Port lp1 -- add Port_Group pg1 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg1 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg2 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp3 -- add Port_Group pg2 ports @p
+
+ovn-nbctl --wait=sb sync
+
+dnl Check if port group address sets were populated with ports' addresses
+AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
+ [0], [[["10.1.0.2", "10.2.0.2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg2_ip4 addresses],
+ [0], [[["10.2.0.2", "10.3.0.2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
+ [0], [[["2001:db8:1::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg2_ip6 addresses],
+ [0], [[["2001:db8:2::ff:fe00:2", "2001:db8:3::ff:fe00:3"]]
+])
+
+ovn-nbctl set Logical_Switch ls1 \
+other_config:subnet=10.11.0.0/24 other_config
[ovs-dev] [PATCH v2 1/3] ovn-nbctl: Allow referring to port groups by name.
Be user-friendly and allow using port group's name as its identifier in
database commands.
Signed-off-by: Jakub Sitnicki
Acked-by: Han Zhou
---
ovn/utilities/ovn-nbctl.c | 3 +++
tests/ovn-nbctl.at| 14 ++
2 files changed, 17 insertions(+)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3c3e582cb..f99b81bc0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4369,6 +4369,9 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ADDRESS_SET].row_ids[0]
= {_address_set_col_name, NULL, NULL},
+[NBREC_TABLE_PORT_GROUP].row_ids[0]
+= {_port_group_col_name, NULL, NULL},
+
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 64e217654..069b7b5b6 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1505,3 +1505,17 @@ AT_CHECK([grep 'command takes at most .* arguments'
stderr], [0], [ignore])
OVN_NBCTL_TEST_STOP
AT_CLEANUP
+
+dnl -
+
+AT_SETUP([ovn-nbctl - Port Groups])
+OVN_NBCTL_TEST_START
+
+dnl Check that port group can be looked up by name
+AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
+AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
+"pg0"
+])
+
+OVN_NBCTL_TEST_STOP
+AT_CLEANUP
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v2 0/3] Port Group fixes
This series addresses two issues noticed with Port Groups: (1) Port Group name cannot be used to refer to a row in NB DB. (2) LSP dynamic addresses don't get copied to port group address sets. The series depends on recent fixes for IPAM from Mark Michelson [1]. Thanks, Jakub [1] https://patchwork.ozlabs.org/patch/947155/ v1 -> v2: - Check if LSP addresses are static before treating them as such to avoid a warning. Reported by Han Zhou. Jakub Sitnicki (3): ovn-nbctl: Allow referring to port groups by name. ovn-northd: Make use of svec for storing lists of addresses. ovn-northd: Propagate dynamic addresses to port group address sets. ovn/northd/ovn-northd.c | 77 ovn/utilities/ovn-nbctl.c | 3 ++ tests/ovn-nbctl.at| 14 ++ tests/ovn.at | 109 ++ 4 files changed, 164 insertions(+), 39 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 0/2] Allow for smoother restarting of ovn-controller
On Tue, 24 Jul 2018 16:30:04 -0400 Mark Michelson wrote: > When ovn-controller is restarted, the ovn-controller process is stopped > and then started again. When the process is stopped, the process cleans > itself up by removing all traces of itself from the central southbound > database and the OVS database on the hypervisor. The issue with this is > that this removes tunnels, meaning that traffic from other hypervisors > in the cluster is unable to reach the local hypervisor. When restarting, > it would be better to not clean up, thus allowing for uninterrupted > traffic flow. > > This patchset allows for ovn-controller to be stopped without cleaning > itself up. This way, during a restart, traffic can still flow freely > while ovn-controller is down. > > Mark Michelson (2): > ovn: Add '--restart' flag to ovn-controller exit. > ovn: Modify restart_controller in ovn-ctl to use --restart > > ovn/controller/ovn-controller.c | 92 +++- > ovn/utilities/ovn-ctl | 4 +- > tests/ovn.at| 186 > > utilities/ovs-lib.in| 2 +- > 4 files changed, 241 insertions(+), 43 deletions(-) > I had one minor comment. Otherwise LGTM: Acked-by: Jakub Sitnicki ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 2/2] ovn: Modify restart_controller in ovn-ctl to use --restart
On Tue, 24 Jul 2018 16:30:06 -0400
Mark Michelson wrote:
> The --restart flag allows for uninterrupted packet flowage when exiting
> ovn-controller. This patch modifies the restart_controller argument to
> ovn-ctl to use --restart.
>
> Signed-off-by: Mark Michelson
> ---
> ovn/utilities/ovn-ctl | 4 ++--
> utilities/ovs-lib.in | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ovn/utilities/ovn-ctl b/ovn/utilities/ovn-ctl
> index 2fce47714..f76248e84 100755
> --- a/ovn/utilities/ovn-ctl
> +++ b/ovn/utilities/ovn-ctl
> @@ -350,7 +350,7 @@ stop_northd () {
> }
>
> stop_controller () {
> -OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller
> +OVS_RUNDIR=${OVN_RUNDIR} stop_daemon ovn-controller $1
> }
Might be less surprising if the wrapper passed all arguments
to the target command, but this works too.
>
> stop_controller_vtep () {
> @@ -367,7 +367,7 @@ restart_northd () {
> }
>
> restart_controller () {
> -stop_controller
> +stop_controller --restart
> start_controller
> }
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 92f98ad92..9f62dfd25 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -258,7 +258,7 @@ stop_daemon () {
> case $action in
> EXIT)
> action "Exiting $1 ($pid)" \
> -${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl
> exit
> +${bindir}/ovs-appctl -T 1 -t $rundir/$1.$pid.ctl
> exit $2
> ;;
> TERM)
> action "Killing $1 ($pid)" kill $pid
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 2/3] ovn-northd: Make use of svec for storing lists of addresses.
Get rid of what is, esentially, an open-coded version of svec.
Signed-off-by: Jakub Sitnicki
---
ovn/northd/ovn-northd.c | 47 +++---
tests/ovn.at| 50 +
2 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 572022897..52f47c5ed 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -39,6 +39,7 @@
#include "openvswitch/poll-loop.h"
#include "smap.h"
#include "sset.h"
+#include "svec.h"
#include "stream.h"
#include "stream-ssl.h"
#include "unixctl.h"
@@ -6675,54 +6676,36 @@ sync_address_sets(struct northd_context *ctx)
/* sync port group generated address sets first */
const struct nbrec_port_group *nb_port_group;
NBREC_PORT_GROUP_FOR_EACH (nb_port_group, ctx->ovnnb_idl) {
-char **ipv4_addrs = xcalloc(1, sizeof *ipv4_addrs);
-size_t n_ipv4_addrs = 0;
-size_t n_ipv4_addrs_buf = 1;
-char **ipv6_addrs = xcalloc(1, sizeof *ipv6_addrs);
-size_t n_ipv6_addrs = 0;
-size_t n_ipv6_addrs_buf = 1;
+struct svec ipv4_addrs = SVEC_EMPTY_INITIALIZER;
+struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER;
for (size_t i = 0; i < nb_port_group->n_ports; i++) {
for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) {
struct lport_addresses laddrs;
extract_lsp_addresses(nb_port_group->ports[i]->addresses[j],
);
-while (n_ipv4_addrs_buf < n_ipv4_addrs + laddrs.n_ipv4_addrs) {
-n_ipv4_addrs_buf *= 2;
-ipv4_addrs = xrealloc(ipv4_addrs,
-n_ipv4_addrs_buf * sizeof *ipv4_addrs);
-}
for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
-ipv4_addrs[n_ipv4_addrs++] =
-xstrdup(laddrs.ipv4_addrs[k].addr_s);
-}
-while (n_ipv6_addrs_buf < n_ipv6_addrs + laddrs.n_ipv6_addrs) {
-n_ipv6_addrs_buf *= 2;
-ipv6_addrs = xrealloc(ipv6_addrs,
-n_ipv6_addrs_buf * sizeof *ipv6_addrs);
+svec_add(_addrs, laddrs.ipv4_addrs[k].addr_s);
}
for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
-ipv6_addrs[n_ipv6_addrs++] =
-xstrdup(laddrs.ipv6_addrs[k].addr_s);
+svec_add(_addrs, laddrs.ipv6_addrs[k].addr_s);
}
destroy_lport_addresses();
}
}
char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name);
char *ipv6_addrs_name = xasprintf("%s_ip6", nb_port_group->name);
-sync_address_set(ctx, ipv4_addrs_name, (const char **)ipv4_addrs,
- n_ipv4_addrs, _address_sets);
-sync_address_set(ctx, ipv6_addrs_name, (const char **)ipv6_addrs,
- n_ipv6_addrs, _address_sets);
+sync_address_set(ctx, ipv4_addrs_name,
+ /* "char **" is not compatible with "const char **" */
+ (const char **)ipv4_addrs.names,
+ ipv4_addrs.n, _address_sets);
+sync_address_set(ctx, ipv6_addrs_name,
+ /* "char **" is not compatible with "const char **" */
+ (const char **)ipv6_addrs.names,
+ ipv6_addrs.n, _address_sets);
free(ipv4_addrs_name);
free(ipv6_addrs_name);
-for (size_t i = 0; i < n_ipv4_addrs; i++) {
-free(ipv4_addrs[i]);
-}
-free(ipv4_addrs);
-for (size_t i = 0; i < n_ipv6_addrs; i++) {
-free(ipv6_addrs[i]);
-}
-free(ipv6_addrs);
+svec_destroy(_addrs);
+svec_destroy(_addrs);
}
/* sync user defined address sets, which may overwrite port group
diff --git a/tests/ovn.at b/tests/ovn.at
index e80c965a3..e7bdfe250 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10217,6 +10217,56 @@ done
OVN_CLEANUP([hv1], [hv2], [hv3])
AT_CLEANUP
+AT_SETUP([ovn -- Address Set generation from Port Groups (static addressing)])
+ovn_start
+
+ovn-nbctl ls-add ls1
+
+ovn-nbctl lsp-add ls1 lp1
+ovn-nbctl lsp-add ls1 lp2
+ovn-nbctl lsp-add ls1 lp3
+
+ovn-nbctl lsp-set-addresses lp1 "02:00:00:00:00:01 10.0.0.1 2001:db8::1"
+ovn-nbctl lsp-set-addresses lp2 "02:00:00:00:00:02 10.0.0.2 2001:db8::2"
+ovn-nbctl lsp-set-addresses lp3 "02:00:00:00:00:03 10.0.0.3 2001:db8::3"
+
+ovn-nbctl create Port_Group name=pg1
+ovn-nbctl create Port_Group name=pg2
+
+ovn-nbctl --id=
[ovs-dev] [PATCH 3/3] ovn-northd: Propagate dynamic addresses to port group address sets.
If a logical switch port belongs to a port group and has dynamic
addresses assigned, propagate the addresses to the auto-generated
address sets for the port group.
Signed-off-by: Jakub Sitnicki
---
ovn/northd/ovn-northd.c | 34 +++-
tests/ovn.at| 59 +
2 files changed, 83 insertions(+), 10 deletions(-)
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 52f47c5ed..a93a46f07 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -6653,6 +6653,24 @@ sync_address_set(struct northd_context *ctx, const char
*name,
addrs, n_addrs);
}
+/* Go through 'addresses' and add found IPv4 addresses to 'ipv4_addrs' and IPv6
+ * addresses to 'ipv6_addrs'.
+ */
+static void
+split_addresses(const char *addresses, struct svec *ipv4_addrs,
+struct svec *ipv6_addrs)
+{
+struct lport_addresses laddrs;
+extract_lsp_addresses(addresses, );
+for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
+svec_add(ipv4_addrs, laddrs.ipv4_addrs[k].addr_s);
+}
+for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
+svec_add(ipv6_addrs, laddrs.ipv6_addrs[k].addr_s);
+}
+destroy_lport_addresses();
+}
+
/* OVN_Southbound Address_Set table contains same records as in north
* bound, plus the records generated from Port_Group table in north bound.
*
@@ -6680,16 +6698,12 @@ sync_address_sets(struct northd_context *ctx)
struct svec ipv6_addrs = SVEC_EMPTY_INITIALIZER;
for (size_t i = 0; i < nb_port_group->n_ports; i++) {
for (size_t j = 0; j < nb_port_group->ports[i]->n_addresses; j++) {
-struct lport_addresses laddrs;
-extract_lsp_addresses(nb_port_group->ports[i]->addresses[j],
- );
-for (size_t k = 0; k < laddrs.n_ipv4_addrs; k++) {
-svec_add(_addrs, laddrs.ipv4_addrs[k].addr_s);
-}
-for (size_t k = 0; k < laddrs.n_ipv6_addrs; k++) {
-svec_add(_addrs, laddrs.ipv6_addrs[k].addr_s);
-}
-destroy_lport_addresses();
+split_addresses(nb_port_group->ports[i]->addresses[j],
+_addrs, _addrs);
+}
+if (nb_port_group->ports[i]->dynamic_addresses) {
+split_addresses(nb_port_group->ports[i]->dynamic_addresses,
+_addrs, _addrs);
}
}
char *ipv4_addrs_name = xasprintf("%s_ip4", nb_port_group->name);
diff --git a/tests/ovn.at b/tests/ovn.at
index e7bdfe250..32e6c1738 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -10267,6 +10267,65 @@ AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
AT_CLEANUP
+AT_SETUP([ovn -- Address Set generation from Port Groups (dynamic addressing)])
+ovn_start
+
+ovn-nbctl ls-add ls1
+ovn-nbctl ls-add ls2
+ovn-nbctl ls-add ls3
+
+ovn-nbctl set Logical_Switch ls1 \
+other_config:subnet=10.1.0.0/24 other_config:ipv6_prefix="2001:db8:1::"
+ovn-nbctl set Logical_Switch ls2 \
+other_config:subnet=10.2.0.0/24 other_config:ipv6_prefix="2001:db8:2::"
+ovn-nbctl set Logical_Switch ls3 \
+other_config:subnet=10.3.0.0/24 other_config:ipv6_prefix="2001:db8:3::"
+
+ovn-nbctl lsp-add ls1 lp1
+ovn-nbctl lsp-add ls2 lp2
+ovn-nbctl lsp-add ls3 lp3
+
+ovn-nbctl lsp-set-addresses lp1 "02:00:00:00:00:01 dynamic"
+ovn-nbctl lsp-set-addresses lp2 "02:00:00:00:00:02 dynamic"
+ovn-nbctl lsp-set-addresses lp3 "02:00:00:00:00:03 dynamic"
+
+ovn-nbctl create Port_Group name=pg1
+ovn-nbctl create Port_Group name=pg2
+
+ovn-nbctl --id=@p get Logical_Switch_Port lp1 -- add Port_Group pg1 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg1 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp2 -- add Port_Group pg2 ports @p
+ovn-nbctl --id=@p get Logical_Switch_Port lp3 -- add Port_Group pg2 ports @p
+
+ovn-nbctl --wait=sb sync
+
+dnl Check if port group address sets were populated with ports' addresses
+AT_CHECK([ovn-sbctl get Address_Set pg1_ip4 addresses],
+ [0], [[["10.1.0.2", "10.2.0.2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg2_ip4 addresses],
+ [0], [[["10.2.0.2", "10.3.0.2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg1_ip6 addresses],
+ [0], [[["2001:db8:1::ff:fe00:1", "2001:db8:2::ff:fe00:2"]]
+])
+AT_CHECK([ovn-sbctl get Address_Set pg2_ip6 addresses],
+ [0], [[["2001:db8:2::ff:fe00:2", "2001:db8:3::ff:fe00:3"]]
+])
+
+ovn-nbctl set Logical_Switch ls1 \
+other_config:subnet=10.11.0.0/24 other_config:ipv6_prefix="2001:db8:11::"
+
+dnl Check if updated address got propagated to the po
[ovs-dev] [PATCH 1/3] ovn-nbctl: Allow referring to port groups by name.
Be user-friendly and allow using port group's name as its identifier in
database commands.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 3 +++
tests/ovn-nbctl.at| 14 ++
2 files changed, 17 insertions(+)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3c3e582cb..f99b81bc0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4369,6 +4369,9 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ADDRESS_SET].row_ids[0]
= {_address_set_col_name, NULL, NULL},
+[NBREC_TABLE_PORT_GROUP].row_ids[0]
+= {_port_group_col_name, NULL, NULL},
+
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 64e217654..069b7b5b6 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1505,3 +1505,17 @@ AT_CHECK([grep 'command takes at most .* arguments'
stderr], [0], [ignore])
OVN_NBCTL_TEST_STOP
AT_CLEANUP
+
+dnl -
+
+AT_SETUP([ovn-nbctl - Port Groups])
+OVN_NBCTL_TEST_START
+
+dnl Check that port group can be looked up by name
+AT_CHECK([ovn-nbctl create Port_Group name=pg0], [0], [ignore])
+AT_CHECK([ovn-nbctl get Port_Group pg0 name], [0], [dnl
+"pg0"
+])
+
+OVN_NBCTL_TEST_STOP
+AT_CLEANUP
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 0/3] Port Group fixes
This series addresses two issues noticed with Port Groups: (1) Port Group name cannot be used to refer to a row in NB DB. (2) LSP dynamic addresses don't get copied to port group address sets. The series depends on recent fixes for IPAM from Mark Michelson [1]. Thanks, Jakub [1] https://patchwork.ozlabs.org/patch/947155/ Jakub Sitnicki (3): ovn-nbctl: Allow referring to port groups by name. ovn-northd: Make use of svec for storing lists of addresses. ovn-northd: Propagate dynamic addresses to port group address sets. ovn/northd/ovn-northd.c | 77 ovn/utilities/ovn-nbctl.c | 3 ++ tests/ovn-nbctl.at| 14 ++ tests/ovn.at | 109 ++ 4 files changed, 163 insertions(+), 40 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 1/1] ovn-nbctl: Initialize arguments to avoid compilation warnings.
On Wed, 25 Jul 2018 15:00:43 +0100
Ian Stokes wrote:
> Output arguments for parse_priority() and dhcp_options_get() may not be
> initialized when either function returns an error.
>
> This causes compilation warnings for GCC 6.3.x regarding use of
> uninitialized variable use and null-pointer-arithmetic.
>
> Fix this by initializing priority_p* value to 0 for priority_parse()
> when an error occurs during parsing.
>
> For dhcp_options_get() set *dhcp_opts_p = dhcp_opts regardless as
> dhcp_opts will be equal to NULL when an error occurs within the function
> anyhow.
>
> Cc: Jakub Sitnicki
> Fixes: 3844c85de979 ("ovn-nbctl: Don't die in dhcp_options_get()."
> Fixes: bc8223df3b01 ("ovn-nbctl: Don't die in parse_priority().")
> Signed-off-by: Ian Stokes
> ---
Thank you for fixing it up.
Acked-by: Jakub Sitnicki
> ovn/utilities/ovn-nbctl.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index 3c3e582..c3703a3 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1870,6 +1870,9 @@ parse_priority(const char *arg, int64_t *priority_p)
> int64_t priority;
> if (!ovs_scan(arg, "%"SCNd64, )
> || priority < 0 || priority > 32767) {
> +/* Priority_p could be uninitialized as no valid priority was
> + * input, initialize it to a valid value of 0 before returning */
> +*priority_p = 0;
> return xasprintf("%s: priority must in range 0...32767", arg);
> }
> *priority_p = priority;
> @@ -2899,10 +2902,11 @@ dhcp_options_get(struct ctl_context *ctx, const char
> *id, bool must_exist,
> dhcp_opts = nbrec_dhcp_options_get_for_uuid(ctx->idl,
> _opts_uuid);
> }
>
> +*dhcp_opts_p = dhcp_opts;
> if (!dhcp_opts && must_exist) {
> return xasprintf("%s: dhcp options UUID not found", id);
> }
> -*dhcp_opts_p = dhcp_opts;
> +
> return NULL;
> }
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 4/4] [RFC] ovn-nbctl: Make daemon mode more transparent.
On Tue, Jul 24, 2018 at 07:53 PM GMT, Ben Pfaff wrote:
> This has some flaws in the details; for example, command-line options
> parsing is very inflexible.
>
> Signed-off-by: Ben Pfaff
> ---
Thank you for working on this. I hooked it up to ovn-nbct.at test suite
and it worked quite nicely. Only test cases that are failing are:
2594: ovn-nbctl - LBs FAILED (ovn-nbctl.at:486)
2595: ovn-nbctl - LBs IPv6FAILED (ovn-nbctl.at:711)
... because the logging options (-v) are not understood by the daemon.
In this case, I think, the option should not be passed in the JSON
request.
One thing I had to do to get the other tests passing is adjust the
format of error messages to match the format of ovn-nbctl regular mode:
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a1717d153..36ecd3fa7 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -147,7 +147,7 @@ main(int argc, char *argv[])
int exit_status;
if (cmd_error) {
exit_status = EXIT_FAILURE;
-fputs(cmd_error, stderr);
+fprintf(stderr, "ovn-nbctl: %s", cmd_error);
} else {
exit_status = EXIT_SUCCESS;
fputs(cmd_result, stdout);
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] ovn-nbctl: Detect unrecognized short options in server mode.
Because getopt() will set optopt for both known and unknown options,
we need to differentiate between them ourselves by checking if we
know the option. Do that by looking up its value.
Also, because we are using GNU extensions to getopt(), we need to be
resetting getopt() state by setting optind to 0 instead of 1 as
pointed out in NOTES in getopt(3) man-page. Not doing so results in
invalid reads and optopt being set to a garbarge value.
Fixes: 3ec06ea9c668 ("ovn-nbctl: Initial support for daemon mode.")
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 8b888294e..a1717d153 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4844,6 +4844,19 @@ nbctl_cmd_init(void)
ctl_register_commands(nbctl_commands);
}
+static const struct option *
+find_option_by_value(const struct option *options, int value)
+{
+const struct option *o;
+
+for (o = options; o->name; o++) {
+if (o->val == value) {
+return o;
+}
+}
+return NULL;
+}
+
static char * OVS_WARN_UNUSED_RESULT
server_parse_options(int argc, char *argv[], struct shash *local_options,
int *n_options_p)
@@ -4863,7 +4876,7 @@ server_parse_options(int argc, char *argv[], struct shash
*local_options,
short_options = build_short_options(global_long_options, false);
options = append_command_options(global_long_options, OPT_LOCAL);
-optind = 1;
+optind = 0;
opterr = 0;
for (;;) {
int idx;
@@ -4899,9 +4912,11 @@ server_parse_options(int argc, char *argv[], struct
shash *local_options,
TABLE_OPTION_HANDLERS(_style)
case '?':
-if (optopt) {
+if (find_option_by_value(options, optopt)) {
error = xasprintf("option '%s' doesn't allow an argument",
argv[optind-1]);
+} else if (optopt) {
+error = xasprintf("unrecognized option '%c'", optopt);
} else {
error = xasprintf("unrecognized option '%s'", argv[optind-1]);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] NEWS: Mention daemon mode support for ovn-nbctl.
Signed-off-by: Jakub Sitnicki --- NEWS | 4 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 057e8bfd3..2414b9199 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,10 @@ Post-v2.9.0 * ACL match conditions can now match on Port_Groups as well as address sets that are automatically generated by Port_Groups. ACLs can be applied directly to Port_Groups as well. + * ovn-nbctl can now run as a daemon (long-lived, background process) + running commands in response to JSON-RPC requests received over a UNIX + socket. Requests to run commands can be sent using ovs-appctl tool, same + as for any other OVS/OVN daemon. See ovn-nbctl(8) for details. - DPDK: * New 'check-dpdk' Makefile target to run a new system testsuite. See Testing topic for the details. -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v5 00/21] Daemon mode for ovn-nbctl
On Mon, 23 Jul 2018 16:47:52 -0700 Ben Pfaff wrote: > On Thu, Jul 19, 2018 at 03:51:05PM +0200, Jakub Sitnicki wrote: > > This series extends ovn-nbctl tool with support for the daemon mode, where > > ovn-nbctl acts a long-lived process that accepts commands over a UNIX > > socket. > > The daemon can be started the same way as any other OVS/OVN server: > > > > ovn-nbctl --detach --pidfile --log-file > > Thanks a lot. I applied this series to master. > > Would you mind sending an additional patch to add an appropriate item to > NEWS? With pleasure. > I'm going to play with some ideas for tests. Thanks, I don't have it figured out yet. -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 10/11] ovn-nbctl: Remove pointless "return; " at ends of functions.
On Mon, 23 Jul 2018 15:38:55 -0700 Ben Pfaff wrote: > On Tue, Jul 17, 2018 at 03:34:14PM +0200, Jakub Sitnicki wrote: > > Fix fall-out from applying a semantic patch that converts ctl_fatal() > > calls to use ctl_error(). > > > > Signed-off-by: Jakub Sitnicki > > Oh, you fixed that up later ;-). Ha, I'll revert it to your way > instead, never mind. Yes, I've decided to do fix-ups separately so that the Coccinelle-generated patches could be reviewed based on the semantic patch. Not sure if that worked out fine or just created confusion. -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 08/11] ovn-nbctl: Propagate error thru the context.
On Mon, 23 Jul 2018 15:36:28 -0700
Ben Pfaff wrote:
> On Tue, Jul 17, 2018 at 03:34:12PM +0200, Jakub Sitnicki wrote:
> > Instead of dying let the main loop handle the error.
> > This will allow us to report errors when running in daemon mode.
> >
> > This is a result of applying the following semantic patch:
> >
> > @@
> > identifier F;
> > identifier C;
> > identifier E;
> > @@
> > static void F(struct ctl_context *C) {
> > <...
> > if (E) {
> > - ctl_fatal("%s", E);
> > + C->error = E;
> > + return;
> > }
> > ...>
> > }
> >
> > Signed-off-by: Jakub Sitnicki
>
> It's too bad that this generates code like this:
>
> char *error = ls_by_name_or_uuid(ctx, ctx->argv[1], false, );
> if (error) {
> ctx->error = error;
> return;
> }
>
> instead of the simpler:
>
> ctx->error = ls_by_name_or_uuid(ctx, ctx->argv[1], false, );
> if (ctx->error) {
> return;
> }
>
> Oh well.
Let me see if I can come up with a semantic patch that corrects that.
-Jakub
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 06/11] ovn-nbctl: Don't die in parse_direction().
On Mon, 23 Jul 2018 15:33:26 -0700
Ben Pfaff wrote:
> On Tue, Jul 17, 2018 at 03:34:10PM +0200, Jakub Sitnicki wrote:
> > Let the caller handle the error. This prepares us for reporting errors
> > in daemon mode.
> >
> > Signed-off-by: Jakub Sitnicki
>
> I got a lot of "possibly uninitialized" warnings from GCC for this one:
>
> ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_del’:
> ../ovn/utilities/ovn-nbctl.c:2068:17: error: ‘direction’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> if (strcmp(direction, ls->qos_rules[i]->direction)) {
> ^~
> ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_add’:
> ../ovn/utilities/ovn-nbctl.c:1739:5: error: ‘direction’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> nbrec_acl_set_direction(acl, direction);
> ^~~
> ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_acl_del’:
> ../ovn/utilities/ovn-nbctl.c:1829:17: error: ‘direction’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> if (strcmp(direction, acls[i]->direction)) {
> ^
> ../ovn/utilities/ovn-nbctl.c: In function ‘nbctl_qos_add’:
> ../ovn/utilities/ovn-nbctl.c:1994:5: error: ‘direction’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
> nbrec_qos_set_direction(qos, direction);
> ^~~
> ../ovn/utilities/ovn-nbctl.c: At top level:
>
> I think it's wrong but the following incremental solved it so I folded
> it in:
>
> diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
> index c18fa28256af..a4a533740cb9 100644
> --- a/ovn/utilities/ovn-nbctl.c
> +++ b/ovn/utilities/ovn-nbctl.c
> @@ -1680,6 +1680,7 @@ parse_direction(const char *arg, const char
> **direction_p)
> } else if (arg[0] == 'f') {
> *direction_p = "from-lport";
> } else {
> +*direction_p = NULL;
> return xasprintf("%s: direction must be \"to-lport\" or "
> "\"from-lport\"", arg);
> }
>
Thank you for fixing it up. It's an oversight on my side, I've only
build tested with Clang. Wondering why 0-day bot wasn't complaining.
Maybe it's just newer versions of GCC that detect this.
Thanks,
Jakub
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v4] ovn: Allow for automatic dynamic updates of IPAM
On Fri, Jul 13, 2018 at 06:45 PM GMT, Mark Michelson wrote:
> OVN offers a method of IP address management that allows for an IPv4 subnet or
> IPv6 prefix to be specified on a logical switch. Then by specifying a
> switch port's address as "dynamic" or " dynamic", OVN will
> automatically assign addresses to the switch port.
>
> While this works great for initial assignment of addresses, addresses do
> not automatically adjust when changes are made to the switch's
> configuration. For instance:
> * If the subnet, ipv6_prefix, or exclude_ips for a logical switch
> changes, the affected switch ports are not updated.
> * If a switch port with a static IP address is added to the switch, and
> that address conflicts with a dynamically assigned IP address, the
> dynamic address is not updated.
> * If a MAC address switched from being statically assigned to
> dynamically assigned, the MAC address would not be updated.
> * If a statically assigned MAC address changed, then the IPv6 address
> would not be updated.
>
> This patch solves all of the above issues by changing the algorithm for
> IPAM assignment. There are essentially three steps.
> 1) While joining logical ports, all statically-assigned addresses (i.e.
> any ports without "dynamic" addresses) have their addresses registered
> to IPAM. This gives them top priority.
> 2) All logical ports with dynamic addresses are inspected. Any changes
> that must be made to the addresses are collected to be made later. Any
> addresses that do not require change are registered to IPAM. This allows
> for previously assigned dynamic addresses to be kept.
> 3) All gathered changes are enacted.
>
> The change contains new tests that ensure that dynamic addresses are
> updated when appropriate.
>
> This patch also alters some existing IPAM tests. Those tests assumed
> that dynamic addresses would not be updated automatically, so those
> tests either had to be altered or removed.
>
> Signed-off-by: Mark Michelson
I've come up with a couple suggestions (see in-line) but otherwise LGTM:
Acked-by: Jakub Sitnicki
> ---
> v3->v4:
> Print warning when multiple dynamic addresses are configured on a
> switch port. Ensure that dynamic addresses beyond the first on a switch
> port are ignored. Found by Ben Pfaff.
>
> v2->v3:
> Fixed a checkpatch problem (line too long)
>
> v1->v2:
> Rebased
> ---
> ovn/northd/ovn-northd.c | 385
> ++--
> tests/ovn.at| 97 +---
> 2 files changed, 350 insertions(+), 132 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 04a072ba8..b75febb44 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -985,9 +985,6 @@ ipam_add_port_addresses(struct ovn_datapath *od, struct
> ovn_port *op)
> for (size_t i = 0; i < op->nbsp->n_addresses; i++) {
> ipam_insert_lsp_addresses(od, op, op->nbsp->addresses[i]);
> }
> -if (op->nbsp->dynamic_addresses) {
> -ipam_insert_lsp_addresses(od, op, op->nbsp->dynamic_addresses);
> -}
> } else if (op->nbrp) {
> struct lport_addresses lrp_networks;
> if (!extract_lrp_networks(op->nbrp, _networks)) {
> @@ -1060,64 +1057,255 @@ ipam_get_unused_ip(struct ovn_datapath *od)
> return od->ipam_info.start_ipv4 + new_ip_index;
> }
>
> +enum dynamic_update_type {
> +NONE,/* No change to the address */
> +REMOVE, /* Address is no longer dynamic */
> +STATIC, /* Use static address (MAC only) */
> +DYNAMIC, /* Assign a new dynamic address */
> +};
> +
> +struct dynamic_address_update {
> +struct ovs_list node; /* In build_ipam()'s list of updates. */
> +
> +struct ovn_port *op;
> +
> +struct lport_addresses current_addresses;
> +struct eth_addr static_mac;
> +enum dynamic_update_type mac;
> +enum dynamic_update_type ipv4;
> +enum dynamic_update_type ipv6;
> +};
> +
> +static enum dynamic_update_type
> +dynamic_mac_changed(const char *lsp_addresses,
> +struct dynamic_address_update *update)
> +{
> + struct eth_addr ea;
> +
> + if (ovs_scan(lsp_addresses, ETH_ADDR_SCAN_FMT, ETH_ADDR_SCAN_ARGS(ea))) {
> + if (eth_addr_equals(ea, update->current_addresses.ea)) {
> + return NONE;
> + } else {
> + /* MAC is still static, but it has changed */
> + update->static_mac = ea;
> + return STATIC;
> + }
> + }
> +
> + uint64_t mac64 = eth_addr_to_uint64(update->current_addresses.ea);
> + i
Re: [ovs-dev] [PATCH v5 00/21] Daemon mode for ovn-nbctl
Thanks for starting the discussion, Mark.
On Thu, 19 Jul 2018 12:26:24 -0400
Mark Michelson wrote:
> On 07/19/2018 09:51 AM, Jakub Sitnicki wrote:
(...)
> > The changes are functional to the point that all test cases in the ovn-nbctl
> > test suite (tests/ovn-nbctl.at) and OVN test suite (tests/ovn.at) pass when
> > running ovn-nbctl as a daemon [2].
> >
> > However, I'm still thinking how to nicely integrate the daemon mode with the
> > test suite so that the existing tests can be run using either the normal or
> > the
> > daemon mode. Any ideas?
>
> When I think about this, I think of two obstacles:
> 1) What is a good way to run a test under both daemon mode and
> non-daemon mode?
> 2) What is the best way to compose a test that will run under both
> daemon mode and non-daemon mode
>
> (2) seems like the easier problem to mechanically solve. You can perform
> a find and replace to change all current instances of `ovn-nbctl` to
> some bash function. The bash function could determine whether daemon
> mode is in use. If not in daemon mode, call `ovn-nbctl` directly. If in
> daemon mode, call `ovs-appctl -t ovn-nbctl run`.
>
> This isn't a good long term solution though. It will be difficult to
> enforce the use of the bash function instead of calling ovn-nbctl. A
> potentially better long-term solution would be not to require ovs-appctl
> when communicating with the ovn-nbctl daemon. If ovn-nbctl could be
> called directly, it would make things easier.
As it happens, in the test suite, ovn-nbctl is an alias for a shell
function already [1].
That is something I thought we can leverage. At least until we turn
ovn-nbctl tool into a JSON-RPC client that will control the daemon
(which ideally should work out-of-the-box if the daemon is running).
For now, we could have a wrapper around 'ovs-appctl -t ovn-nbctl run'
that tries to act like ovn-nbctl. I have a very crude prototype of
that [2] which screams for a rewrite in Python :-)
>
> (1) is a bit more tricky in my opinion.
>
> One idea would be to pass in a flag to the testsuite that says to start
> an ovn-nbctl daemon when ovn_start() is called. This way, the entire
> runthrough of the test would be for ovn-nbctl daemon mode. In CI
> systems, we'd want to perform two runs of the testsuite, one in daemon
> mode, and one not in daemon mode. This idea is a bit overkill though
> since most tests in the testsuite are not even OVN tests. This also
> doesn't scale well in case there become other ways we want to run
> variants of existing tests.
>
> A better idea is to somehow make existing tests that use ovn-nbctl run
> once in daemon mode and once in non-daemon mode. This is difficult,
> though, because the structure of the tests doesn't allow for an easy way
> to repeat the test. The "easiest" way I can think to do this is to
> modify the existing ovn test files to be .in files that at build time
> generate a daemon version of the test and a non-daemon version of the
> test. Notice "easiest" in quotation marks though. The advantage of doing
> it this way is that it simultaneously clears up problem (2) as well
> since the autogenerated files will have the proper invocations in them.
I agree this is the trickier part.
I was also thinking about introducing a flag that controls if ovn-nbctl
runs in daemon mode or regular mode throughout the tests. If we had
that we could do a second pass only on tests that match on the 'ovn'
keyword.
I haven't thought of generating two variants of tests. That also sounds
interesting. Maybe that would be doable straight from m4, without going
through generating files from templates.
One more thing to consider is that point where we have to start/stop
the daemon differs from test suite to test suite. For tests/ovn.at that
would be ovn_start/OVN_CLEANUP (with one exception [3]). But for
tests/ovn-nbctl.at this would be OVN_NBCTL_TEST_{START,STOP}. That
seems less of problem though.
-Jakub
[1] https://github.com/openvswitch/ovs/blob/master/tests/ovs-macros.at#L135
[2]
https://github.com/jsitnicki/ovs/commit/c34ef03fc11e94e61c57edbfb3e968eacbf07ffb#diff-0c2aae317387234fe3f3e3f28313cf63R32
[3] https://github.com/openvswitch/ovs/blob/master/tests/ovn.at#L6376
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 00/11] Get rid of ctl_fatal() calls in ovn-nbctl (part 2)
This is a continuation of an earlier series that aims to replace calls to ctl_fatal() in command handlers in ovn-nbctl. The motivation is to handle errors gracefully when running commands in daemon mode because as a long-lived process we shouldn't terminate on errors that we can recover from. After this series there are no more ctl_fatal() calls in ovn-nbctl that affect the daemon mode. The only remaining function left to convert is the commands parser in db-ctl-base module (ctl_parse_commands()), which I intend to deal with separately. Either as a part of ovn-nbctl daemon series (already in review [1]), or as a follow-up to it. Thanks, Jakub [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55472 Jakub Sitnicki (11): ovn-nbctl: Don't die in pg_by_name_or_uuid(). ovn-nbctl: Don't die in gc_by_name_or_uuid(). ovn-nbctl: Don't die in lsp_to_ls(). ovn-nbctl: Don't die in lrp_to_lr(). ovn-nbctl: Don't die in parse_priority(). ovn-nbctl: Don't die in parse_direction(). ovn-nbctl: Don't die in dhcp_options_get(). ovn-nbctl: Propagate error thru the context. ovn-nbctl: Use ctl_error() in command handlers. ovn-nbctl: Remove pointless "return;" at ends of functions. ovn-nbctl: Fix mem leak in nbctl_lrp_set_gateway_chassis(). ovn/utilities/ovn-nbctl.c | 360 -- tests/ovn-nbctl.at| 5 + 2 files changed, 260 insertions(+), 105 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 21/21] tests: Add test for ovn-nbctl's command parser error paths.
Preparatory work for getting rid of ctl_fatal() in command parser. Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 76 ++ 1 file changed, 76 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 4e6986a5e..ffbd2a140 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1424,3 +1424,79 @@ AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - commands parser error paths]) +OVN_NBCTL_TEST_START + +dnl FIXME: Duplicate options are allowed when passed with global options. +dnlFor example: ovn-nbctl --if-exists --if-exists list Logical_Switch + +dnl Duplicate option +AT_CHECK([ovn-nbctl -- --if-exists --if-exists list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option specified multiple times' stderr], [0], [ignore]) + +dnl Missing command +AT_CHECK([ovn-nbctl], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +dnl Unknown command +AT_CHECK([ovn-nbctl foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +dnl Unknown option +AT_CHECK([ovn-nbctl --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'unrecognized option' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'command has no .* option' stderr], [0], [ignore]) + +dnl Missing option argument +AT_CHECK([ovn-nbctl --columns], [1], [], [stderr]) +AT_CHECK([grep 'option .* requires an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --columns list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'missing argument to .* option' stderr], [0], [ignore]) + +dnl Unexpected option argument +AT_CHECK([ovn-nbctl --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option .* doesn'\''t allow an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option on .* does not accept an argument' stderr], [0], [ignore]) + +dnl Not enough arguments +AT_CHECK([ovn-nbctl list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +dnl Too many arguments +AT_CHECK([ovn-nbctl show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 20/21] tests: Add test for oneline-formatted output for ovn-nbctl.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 585967568..4e6986a5e 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1403,3 +1403,24 @@ AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - oneline output]) +OVN_NBCTL_TEST_START + +AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1]) + +dnl Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +dnl Expect lines for two commands. +AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +<0> (ls0)\n<1> (ls1) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 19/21] tests: Add test for ovn-nbctl dry run mode.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 73a61a4be..585967568 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1382,3 +1382,24 @@ inactivity_probe: 3 OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - dry run mode]) +OVN_NBCTL_TEST_START + +dnl Check that dry run has no permanent effect. +AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +]) + +dnl Check that dry-run mode is not sticky. +AT_CHECK([ovn-nbctl ls-add ls0]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 18/21] ovn-nbctl: Initial support for daemon mode.
Make ovn-nbctl act as a unixctl server if we were asked to detach. This
turns ovn-nbctl into a long-lived process that acts a proxy for
interacting with NB DB. The main difference to regular mode of ovn-nbctl
is that in the daemon mode, a local copy of database contents has to be
obtained only once.
Just two unixctl commands are supported 'run' and 'exit'. The former can
be used to run any ovn-nbctl command or a batch of them as so:
ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
Running commands that have not yet been converted to not use ctl_fatal()
will result in death of the daemon process. However, --monitor option
can be used to keep the daemon running.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.8.xml | 40 +
ovn/utilities/ovn-nbctl.c | 335 ++
2 files changed, 344 insertions(+), 31 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index abba4ecdb..2cd2fab30 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -913,6 +913,43 @@
+Daemon Mode
+
+
+ If ovn-nbctl is invoked with the --detach
+ option (see Daemon Options, below), it runs in the
+ background as a daemon and accepts commands from ovs-appctl
+ (or another JSON-RPC client) indefinitely. The currently supported
+ commands are described below.
+
+
+
+
+
+
+
+
+run [options] command
+[arg...] [-- [options]
+command [arg...] ...]
+
+
+Instructs the daemon process to run one or more ovn-nbctl
+commands described above and reply with the results of running these
+commands. Accepts the --no-wait, --wait,
+--timeout, --dry-run, --oneline,
+and the options described under Table Formatting Options
+in addition to the the command-specific options.
+
+
+ exit
+ Causes ovn-nbctl to gracefully terminate.
+
+
+
+ Daemon mode is considered experimental.
+
+
Options
@@ -982,6 +1019,9 @@
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
Logging options
http://www.w3.org/2003/XInclude"/>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7a0d6ecd4..920f9b8ca 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -20,6 +20,7 @@
#include
#include "command-line.h"
+#include "daemon.h"
#include "db-ctl-base.h"
#include "dirs.h"
#include "fatal-signal.h"
@@ -38,6 +39,7 @@
#include "table.h"
#include "timeval.h"
#include "timer.h"
+#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status);
/* --leader-only, --no-leader-only: Only accept the leader in a cluster. */
static int leader_only = true;
+/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop"
+ commands. */
+static char *unixctl_path;
+
+static unixctl_cb_func server_cmd_exit;
+static unixctl_cb_func server_cmd_run;
+
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
@@ -98,14 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char
*args,
size_t n_commands,
struct ovsdb_idl *idl,
const struct timer *);
+static void server_loop(struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
{
struct ovsdb_idl *idl;
-struct ctl_command *commands;
struct shash local_options;
-size_t n_commands;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -118,41 +126,59 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-char *error = ctl_parse_commands(argc - optind, argv + optind,
- _options, , _commands);
-if (error) {
-ctl_fatal("%s", error);
-}
-VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
- "Called as %s", args);
-
-if (timeout) {
-time_alarm(timeout);
-}
+argc -= optind;
+argv += optind;
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-error = run_prerequisites(commands, n_commands, idl);
-if (error) {
-ctl_fatal("%s", error);
-}
-error = main_loop(args, commands, n_commands, idl, NULL);
-if (error) {
-ctl_fatal("%s
[ovs-dev] [PATCH v5 17/21] ovn-nbctl: Extract a helper for appending command options.
Will be reused when parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 490577238..7a0d6ecd4 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -288,6 +288,30 @@ build_short_options(const struct option *long_options)
return short_options;
}
+static struct option * OVS_WARN_UNUSED_RESULT
+append_command_options(const struct option *options, int opt_val)
+{
+struct option *o;
+size_t n_allocated;
+size_t n_existing;
+int i;
+
+for (i = 0; options[i].name; i++) {
+;
+}
+n_allocated = i + 1;
+n_existing = i;
+
+/* We want to parse both global and command-specific options here, but
+ * getopt_long() isn't too convenient for the job. We copy our global
+ * options into a dynamic array, then append all of the command-specific
+ * options. */
+o = xmemdup(options, n_allocated * sizeof *options);
+ctl_add_cmd_options(, _existing, _allocated, opt_val);
+
+return o;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -309,22 +333,11 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
char *short_options;
-
struct option *options;
-size_t allocated_options;
-size_t n_options;
size_t i;
short_options = build_short_options(global_long_options);
-
-/* We want to parse both global and command-specific options here, but
- * getopt_long() isn't too convenient for the job. We copy our global
- * options into a dynamic array, then append all of the command-specific
- * options. */
-options = xmemdup(global_long_options, sizeof global_long_options);
-allocated_options = ARRAY_SIZE(global_long_options);
-n_options = n_global_long_options;
-ctl_add_cmd_options(, _options, _options, OPT_LOCAL);
+options = append_command_options(global_long_options, OPT_LOCAL);
for (;;) {
int idx;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 16/21] ovn-nbctl: Extract a helper for building short options string.
Will be reused for parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index ca0f71300..490577238 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -276,6 +276,18 @@ handle_main_loop_option(int opt, const char *arg, bool
*handled)
return NULL;
}
+static char * OVS_WARN_UNUSED_RESULT
+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -296,16 +308,14 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
{NULL, 0, NULL, 0},
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
-char *tmp, *short_options;
+char *short_options;
struct option *options;
size_t allocated_options;
size_t n_options;
size_t i;
-tmp = ovs_cmdl_long_options_to_short_options(global_long_options);
-short_options = xasprintf("+%s", tmp);
-free(tmp);
+short_options = build_short_options(global_long_options);
/* We want to parse both global and command-specific options here, but
* getopt_long() isn't too convenient for the job. We copy our global
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 15/21] ovn-nbctl: Extract handling of options that affect main loop.
Provide a handler for options that change how the main loop behaves.
This will allow code reuse for option parsing in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 136 --
1 file changed, 84 insertions(+), 52 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index e8fd898d3..ca0f71300 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -202,38 +202,93 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
return NULL;
}
+/* All options that affect the main loop and are not external. */
+#define MAIN_LOOP_OPTION_ENUMS \
+OPT_NO_WAIT,\
+OPT_WAIT, \
+OPT_DRY_RUN,\
+OPT_ONELINE
+
+#define MAIN_LOOP_LONG_OPTIONS \
+{"no-wait", no_argument, NULL, OPT_NO_WAIT}, \
+{"wait", required_argument, NULL, OPT_WAIT}, \
+{"dry-run", no_argument, NULL, OPT_DRY_RUN}, \
+{"oneline", no_argument, NULL, OPT_ONELINE}, \
+{"timeout", required_argument, NULL, 't'}
+
+enum {
+OPT_DB = UCHAR_MAX + 1,
+OPT_NO_SYSLOG,
+OPT_LOCAL,
+OPT_COMMANDS,
+OPT_OPTIONS,
+OPT_BOOTSTRAP_CA_CERT,
+MAIN_LOOP_OPTION_ENUMS,
+VLOG_OPTION_ENUMS,
+TABLE_OPTION_ENUMS,
+SSL_OPTION_ENUMS,
+};
+
+static char * OVS_WARN_UNUSED_RESULT
+handle_main_loop_option(int opt, const char *arg, bool *handled)
+{
+ovs_assert(handled);
+*handled = true;
+
+switch (opt) {
+case OPT_ONELINE:
+oneline = true;
+break;
+
+case OPT_NO_WAIT:
+wait_type = NBCTL_WAIT_NONE;
+break;
+
+case OPT_WAIT:
+if (!strcmp(arg, "none")) {
+wait_type = NBCTL_WAIT_NONE;
+} else if (!strcmp(arg, "sb")) {
+wait_type = NBCTL_WAIT_SB;
+} else if (!strcmp(arg, "hv")) {
+wait_type = NBCTL_WAIT_HV;
+} else {
+return xstrdup("argument to --wait must be "
+ "\"none\", \"sb\", or \"hv\"");
+}
+break;
+
+case OPT_DRY_RUN:
+dry_run = true;
+break;
+
+case 't':
+timeout = strtoul(arg, NULL, 10);
+if (timeout < 0) {
+return xasprintf("value %s on -t or --timeout is invalid", arg);
+}
+break;
+
+default:
+*handled = false;
+break;
+}
+
+return NULL;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
-enum {
-OPT_DB = UCHAR_MAX + 1,
-OPT_NO_SYSLOG,
-OPT_NO_WAIT,
-OPT_WAIT,
-OPT_DRY_RUN,
-OPT_ONELINE,
-OPT_LOCAL,
-OPT_COMMANDS,
-OPT_OPTIONS,
-OPT_BOOTSTRAP_CA_CERT,
-VLOG_OPTION_ENUMS,
-TABLE_OPTION_ENUMS,
-SSL_OPTION_ENUMS,
-};
static const struct option global_long_options[] = {
{"db", required_argument, NULL, OPT_DB},
{"no-syslog", no_argument, NULL, OPT_NO_SYSLOG},
-{"no-wait", no_argument, NULL, OPT_NO_WAIT},
-{"wait", required_argument, NULL, OPT_WAIT},
-{"dry-run", no_argument, NULL, OPT_DRY_RUN},
-{"oneline", no_argument, NULL, OPT_ONELINE},
-{"timeout", required_argument, NULL, 't'},
{"help", no_argument, NULL, 'h'},
{"commands", no_argument, NULL, OPT_COMMANDS},
{"options", no_argument, NULL, OPT_OPTIONS},
{"leader-only", no_argument, _only, true},
{"no-leader-only", no_argument, _only, false},
{"version", no_argument, NULL, 'V'},
+MAIN_LOOP_LONG_OPTIONS,
VLOG_LONG_OPTIONS,
STREAM_SSL_LONG_OPTIONS,
{"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -270,40 +325,24 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
break;
}
+bool handled;
+char *error = handle_main_loop_option(c, optarg, );
+if (error) {
+ctl_fatal("%s", error);
+}
+if (handled) {
+continue;
+}
+
switch (c) {
case OPT_DB:
db = optarg;
break;
-case OPT_ONELINE:
-oneline = true;
-break;
-
case OPT_NO_SYSLOG:
vlog_set_levels(_module, VLF_SYSLOG, VLL_WARN);
break;
-case OPT_NO_WAIT:
-wait_type = NBCTL_WAIT_NONE;
-break;
-
-case OPT_WAIT:
-if (!strcmp(optarg, "no
[ovs-dev] [PATCH v5 14/21] ovn-nbctl: Extract helper for printing oneline output.
This will allow us to direct oneline-formatted output to other sinks
than stdout if needed. Preparatory work for daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 53 ++-
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index b84976b26..e8fd898d3 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4164,6 +4164,39 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
return NULL;
}
+static void
+oneline_format(struct ds *lines, struct ds *s)
+{
+size_t j;
+
+ds_chomp(lines, '\n');
+for (j = 0; j < lines->length; j++) {
+int ch = lines->string[j];
+switch (ch) {
+case '\n':
+ds_put_cstr(s, "\\n");
+break;
+
+case '\\':
+ds_put_cstr(s, "");
+break;
+
+default:
+ds_put_char(s, ch);
+}
+}
+ds_put_char(s, '\n');
+}
+
+static void
+oneline_print(struct ds *lines)
+{
+struct ds s = DS_EMPTY_INITIALIZER;
+oneline_format(lines, );
+fputs(ds_cstr(), stdout);
+ds_destroy();
+}
+
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
@@ -4299,25 +4332,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
if (c->table) {
table_print(c->table, _style);
} else if (oneline) {
-size_t j;
-
-ds_chomp(ds, '\n');
-for (j = 0; j < ds->length; j++) {
-int ch = ds->string[j];
-switch (ch) {
-case '\n':
-fputs("\\n", stdout);
-break;
-
-case '\\':
-fputs("", stdout);
-break;
-
-default:
-putchar(ch);
-}
-}
-putchar('\n');
+oneline_print(ds);
} else {
fputs(ds_cstr(ds), stdout);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 13/21] ovn-nbctl: Introduce a poll_timer based wait timeout.
Extend the main loop and the command runner so that the caller can
specify a timeout for poll_block(). This will allow us to break out of
the main loop when waiting on IDL, like in the blocked '--wait=sb/hv
sync' case.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index c7f03cd21..b84976b26 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -37,6 +37,7 @@
#include "svec.h"
#include "table.h"
#include "timeval.h"
+#include "timer.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -87,13 +88,16 @@ static char * OVS_WARN_UNUSED_RESULT
run_prerequisites(struct ctl_command[],
struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+ struct ovsdb_idl *,
+ const struct timer *,
+ bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
struct ctl_command *commands,
size_t n_commands,
- struct ovsdb_idl *idl);
+ struct ovsdb_idl *idl,
+ const struct timer *);
int
main(int argc, char *argv[])
@@ -134,7 +138,7 @@ main(int argc, char *argv[])
ctl_fatal("%s", error);
}
-error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl, NULL);
if (error) {
ctl_fatal("%s", error);
}
@@ -155,7 +159,7 @@ main(int argc, char *argv[])
static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout)
{
unsigned int seqno;
@@ -179,7 +183,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-char *error = do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl,
+ wait_timeout, );
if (error) {
return error;
}
@@ -4161,7 +4166,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl, bool *retry)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4288,8 +4293,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
OVS_NOT_REACHED();
}
-ovsdb_symbol_table_destroy(symtab);
-
for (c = commands; c < [n_commands]; c++) {
struct ds *ds = >output;
@@ -4333,11 +4336,19 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
}
}
ovsdb_idl_wait(idl);
+if (wait_timeout) {
+timer_wait(wait_timeout);
+}
poll_block();
+if (wait_timeout && timer_expired(wait_timeout)) {
+error = xstrdup("timeout expired");
+goto out_error;
+}
}
done: ;
}
+ovsdb_symbol_table_destroy(symtab);
ovsdb_idl_txn_destroy(txn);
the_idl_txn = NULL;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 12/21] ovn-nbctl: Propagate errors from prerequisites runner.
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the prerequisites runner in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 +-
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index fe28e2293..c7f03cd21 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -82,8 +82,9 @@ static int leader_only = true;
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
-static void run_prerequisites(struct ctl_command[], size_t n_commands,
- struct ovsdb_idl *);
+static char * OVS_WARN_UNUSED_RESULT run_prerequisites(struct ctl_command[],
+ size_t n_commands,
+ struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
struct ovsdb_idl *, bool *retry);
@@ -128,7 +129,10 @@ main(int argc, char *argv[])
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-run_prerequisites(commands, n_commands, idl);
+error = run_prerequisites(commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
error = main_loop(args, commands, n_commands, idl);
if (error) {
@@ -4120,7 +4124,7 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
-static void
+static char *
run_prerequisites(struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -4141,7 +4145,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ctl_context_init(, c, idl, NULL, NULL, NULL);
(c->syntax->prerequisites)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+char *error = xstrdup(ctx.error);
+ctl_context_done(, c);
+return error;
}
ctl_context_done(, c);
@@ -4149,6 +4155,8 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ovs_assert(!c->table);
}
}
+
+return NULL;
}
static char *
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 11/21] ovn-nbctl: Propagate errors from the main loop.
Let the caller handle the errors instead of reporting it and
terminating. Prepare for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index e660b2cdf..fe28e2293 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -89,8 +89,10 @@ static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char
*args,
struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
-static void main_loop(const char *args, struct ctl_command *commands,
- size_t n_commands, struct ovsdb_idl *idl);
+static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
+ struct ctl_command *commands,
+ size_t n_commands,
+ struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -128,7 +130,10 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
-main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
@@ -144,7 +149,7 @@ main(int argc, char *argv[])
exit(EXIT_SUCCESS);
}
-static void
+static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -172,10 +177,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
bool retry;
char *error = do_nbctl(args, commands, n_commands, idl, );
if (error) {
-ctl_fatal("%s", error);
+return error;
}
if (!retry) {
-return;
+return NULL;
}
}
@@ -184,6 +189,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
poll_block();
}
}
+
+return NULL;
}
static void
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 10/21] ovn-nbctl: Propagate the error from do_nbctl().
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 46 --
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 465f8f6d1..e660b2cdf 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,9 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static void do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
+ struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -169,7 +170,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl, );
+if (error) {
+ctl_fatal("%s", error);
+}
if (!retry) {
return;
}
@@ -4140,7 +4144,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static void
+static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, bool *retry)
{
@@ -4151,6 +4155,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct ctl_command *c;
struct shash_node *node;
int64_t next_cfg = 0;
+char *error = NULL;
ovs_assert(retry);
@@ -4184,7 +4189,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
(c->syntax->run)();
}
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done_command(, c);
@@ -4198,9 +4205,10 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
SHASH_FOR_EACH (node, >sh) {
struct ovsdb_symbol *symbol = node->data;
if (!symbol->created) {
-ctl_fatal("row id \"%s\" is referenced but never created (e.g. "
- "with \"-- --id=%s create ...\")",
- node->name, node->name);
+error = xasprintf("row id \"%s\" is referenced but never created "
+ "(e.g. with \"-- --id=%s create ...\")",
+ node->name, node->name);
+goto out_error;
}
if (!symbol->strong_ref) {
if (!symbol->weak_ref) {
@@ -4225,7 +4233,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ctl_context_init(, c, idl, txn, symtab, NULL);
(c->syntax->postprocess)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done(, c);
}
@@ -4239,7 +4249,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
case TXN_ABORTED:
/* Should not happen--we never call ovsdb_idl_txn_abort(). */
-ctl_fatal("transaction aborted");
+error = xstrdup("transaction aborted");
+goto out_error;
case TXN_UNCHANGED:
case TXN_SUCCESS:
@@ -4249,11 +4260,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
goto try_again;
case TXN_ERROR:
-ctl_fatal("transaction error: %s", ovsdb_idl_txn_get_error(txn));
+error = xasprintf("transaction error: %s",
+ ovsdb_idl_txn_get_error(txn));
+goto out_error;
case TXN_NOT_LOCKED:
/* Should not happen--we never call ovsdb_idl_set_lock(). */
-ctl_fatal("database not locked");
+error = xstrdup("database not locked");
+goto out_error;
default:
OVS_NOT_REACHED();
@@ -4313
[ovs-dev] [PATCH v5 09/21] db-ctl-base: Propagate errors from the commands parser.
Let the caller decide how to handle the error. Prepare for using the
parser in ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 30 +-
lib/db-ctl-base.h | 6 +++---
ovn/utilities/ovn-nbctl.c | 7 +--
ovn/utilities/ovn-sbctl.c | 7 +--
utilities/ovs-vsctl.c | 7 +--
vtep/vtep-ctl.c | 7 +--
6 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index f92da7c61..4e0eb9c5c 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -2251,22 +2251,24 @@ ctl_add_cmd_options(struct option **options_p, size_t
*n_options_p,
}
/* Parses command-line input for commands. */
-struct ctl_command *
+char *
ctl_parse_commands(int argc, char *argv[], struct shash *local_options,
- size_t *n_commandsp)
+ struct ctl_command **commandsp, size_t *n_commandsp)
{
struct ctl_command *commands;
size_t n_commands, allocated_commands;
int i, start;
+char *error;
commands = NULL;
n_commands = allocated_commands = 0;
+*commandsp = NULL;
+*n_commandsp = 0;
+
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
-char *error;
-
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2277,21 +2279,31 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
}
}
error = parse_command(i - start, [start], local_options,
- [n_commands++]);
+ [n_commands]);
if (error) {
-ctl_fatal("%s", error);
+struct ctl_command *c;
+
+for (c = commands; c < [n_commands]; c++) {
+shash_destroy_free_data(>options);
+}
+free(commands);
+
+return error;
}
+
+n_commands++;
} else if (!shash_is_empty(local_options)) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
start = i + 1;
}
}
if (!n_commands) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
+*commandsp = commands;
*n_commandsp = n_commands;
-return commands;
+return NULL;
}
/* Prints all registered commands. */
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index ba771a180..284b573d0 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -166,9 +166,9 @@ void ctl_print_options(const struct option *);
void ctl_add_cmd_options(struct option **, size_t *n_options_p,
size_t *allocated_options_p, int opt_val);
void ctl_register_commands(const struct ctl_command_syntax *);
-struct ctl_command *ctl_parse_commands(int argc, char *argv[],
- struct shash *local_options,
- size_t *n_commandsp);
+char * OVS_WARN_UNUSED_RESULT ctl_parse_commands(
+int argc, char *argv[], struct shash *local_options,
+struct ctl_command **commandsp, size_t *n_commandsp);
/* Sometimes, it is desirable to print the table with weak reference to
* rows in a 'cmd_show_table' table. In that case, the 'weak_ref_table'
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 25194b2fa..465f8f6d1 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -110,8 +110,11 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if (error) {
+ctl_fatal("%s", error);
+}
VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
"Called as %s", args);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c47cf6df9..7022347ed 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -112,8 +112,11 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
+char *error = ctl_parse_com
[ovs-dev] [PATCH v5 08/21] db-ctl-base: Propagate error from parse_command().
Let the caller handle the error. Needed for ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 63 +--
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab35d6333..f92da7c61 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1880,7 +1880,7 @@ cmd_wait_until(struct ctl_context *ctx)
}
/* Parses one command. */
-static void
+static char * OVS_WARN_UNUSED_RESULT
parse_command(int argc, char *argv[], struct shash *local_options,
struct ctl_command *command)
{
@@ -1888,6 +1888,7 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
struct shash_node *node;
int n_arg;
int i;
+char *error;
shash_init(>options);
shash_swap(local_options, >options);
@@ -1910,17 +1911,23 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
}
if (shash_find(>options, key)) {
-ctl_fatal("'%s' option specified multiple times", argv[i]);
+free(key);
+free(value);
+error = xasprintf("'%s' option specified multiple times", argv[i]);
+goto error;
}
shash_add_nocopy(>options, key, value);
}
if (i == argc) {
-ctl_fatal("missing command name (use --help for help)");
+error = xstrdup("missing command name (use --help for help)");
+goto error;
}
p = shash_find_data(_commands, argv[i]);
if (!p) {
-ctl_fatal("unknown command '%s'; use --help for help", argv[i]);
+error = xasprintf("unknown command '%s'; use --help for help",
+ argv[i]);
+goto error;
}
SHASH_FOR_EACH (node, >options) {
@@ -1928,43 +1935,54 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
int end = s ? s[strlen(node->name)] : EOF;
if (!strchr("=,? ", end)) {
-ctl_fatal("'%s' command has no '%s' option",
- argv[i], node->name);
+error = xasprintf("'%s' command has no '%s' option",
+ argv[i], node->name);
+goto error;
}
if (end != '?' && (end == '=') != (node->data != NULL)) {
if (end == '=') {
-ctl_fatal("missing argument to '%s' option on '%s' "
- "command", node->name, argv[i]);
+error = xasprintf("missing argument to '%s' option on '%s' "
+ "command", node->name, argv[i]);
+goto error;
} else {
-ctl_fatal("'%s' option on '%s' does not accept an "
- "argument", node->name, argv[i]);
+error = xasprintf("'%s' option on '%s' does not accept an "
+ "argument", node->name, argv[i]);
+goto error;
}
}
}
n_arg = argc - i - 1;
if (n_arg < p->min_args) {
-ctl_fatal("'%s' command requires at least %d arguments",
- p->name, p->min_args);
+error = xasprintf("'%s' command requires at least %d arguments",
+ p->name, p->min_args);
+goto error;
} else if (n_arg > p->max_args) {
int j;
for (j = i + 1; j < argc; j++) {
if (argv[j][0] == '-') {
-ctl_fatal("'%s' command takes at most %d arguments "
- "(note that options must precede command "
- "names and follow a \"--\" argument)",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments "
+ "(note that options must precede command "
+ "names and follow a \"--\" argument)",
+ p->name, p->max_args);
+goto error;
}
}
-ctl_fatal("'%s' command takes at most %d arguments",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments",
+ p->name, p->max_args);
+goto error;
}
command->syntax = p;
command->argc = n_arg + 1;
command->argv = [i];
+return NULL;
+
+error:
+shash_destroy_free_data(>options);
+return error;
}
static void
@@ -2247,6 +2265,8 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_o
[ovs-dev] [PATCH v5 07/21] ovn-nbctl: Don't destroy the transaction twice on error.
Reset the global state, if transaction succeeded. Otherwise nbctl_exit() callback will try to clean up on any fatal error. Signed-off-by: Jakub Sitnicki --- ovn/utilities/ovn-nbctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 360b25a89..25194b2fa 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -4307,6 +4307,7 @@ do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands, } ovsdb_idl_txn_destroy(txn); +the_idl_txn = NULL; *retry = false; return; -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 06/21] ovn-nbctl: Signal need to try again via an output param.
Introduce an output parameter for the flag that signals need to retry
running the command. This leaves the return value for error reporting.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a700695fe..360b25a89 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,8 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static bool do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+static void do_nbctl(const char *args, struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -164,7 +164,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
-if (do_nbctl(args, commands, n_commands, idl)) {
+
+bool retry;
+do_nbctl(args, commands, n_commands, idl, );
+if (!retry) {
return;
}
}
@@ -4134,9 +4137,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static bool
+static void
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4146,6 +4149,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct shash_node *node;
int64_t next_cfg = 0;
+ovs_assert(retry);
+
txn = the_idl_txn = ovsdb_idl_txn_create(idl);
if (dry_run) {
ovsdb_idl_txn_set_dry_run(txn);
@@ -4303,7 +4308,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ovsdb_idl_txn_destroy(txn);
-return true;
+*retry = false;
+return;
try_again:
/* Our transaction needs to be rerun, or a prerequisite was not met. Free
@@ -4318,7 +4324,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
table_destroy(c->table);
free(c->table);
}
-return false;
+*retry = true;
}
/* Frees the current transaction and the underlying IDL and then calls
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 05/21] ovn-nbctl: Pull up releasing IDL from do_nbctl().
Destroy IDL resources in the routine where we allocated them.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 51527741b..a700695fe 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -126,6 +126,9 @@ main(int argc, char *argv[])
main_loop(args, commands, n_commands, idl);
+ovsdb_idl_destroy(idl);
+idl = the_idl = NULL;
+
for (struct ctl_command *c = commands; c < [n_commands]; c++) {
ds_destroy(>output);
table_destroy(c->table);
@@ -4299,7 +4302,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
}
ovsdb_idl_txn_destroy(txn);
-ovsdb_idl_destroy(idl);
return true;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 03/21] ovn-nbctl: Extract the main loop.
Split out a routine for the main ovn-nbctl loop.
Preparatory work for introducing daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 47df19b23..a027553b7 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -88,6 +88,8 @@ static bool do_nbctl(const char *args, struct ctl_command *,
size_t n,
struct ovsdb_idl *);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
+static void main_loop(const char *args, struct ctl_command *commands,
+ size_t n_commands, struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -95,7 +97,6 @@ main(int argc, char *argv[])
struct ovsdb_idl *idl;
struct ctl_command *commands;
struct shash local_options;
-unsigned int seqno;
size_t n_commands;
set_program_name(argv[0]);
@@ -123,6 +124,18 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
+main_loop(args, commands, n_commands, idl);
+
+free(args);
+exit(EXIT_SUCCESS);
+}
+
+static void
+main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
+ struct ovsdb_idl *idl)
+{
+unsigned int seqno;
+
/* Execute the commands.
*
* 'seqno' is the database sequence number for which we last tried to
@@ -136,14 +149,13 @@ main(int argc, char *argv[])
if (!ovsdb_idl_is_alive(idl)) {
int retval = ovsdb_idl_get_last_error(idl);
ctl_fatal("%s: database connection failed (%s)",
-db, ovs_retval_to_string(retval));
+ db, ovs_retval_to_string(retval));
}
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
if (do_nbctl(args, commands, n_commands, idl)) {
-free(args);
-exit(EXIT_SUCCESS);
+return;
}
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 01/21] table: Introduce a constant for default table style.
Having a constant in addition to the constant expression for the default
table style allows us to reset 'struct table_style' variables to default
style.
Signed-off-by: Jakub Sitnicki
---
lib/table.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/table.h b/lib/table.h
index 313ac1dd2..76e65bb70 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -20,6 +20,7 @@
#include
#include
#include "compiler.h"
+#include "openvswitch/json.h"
struct ds;
struct table_style;
@@ -83,6 +84,7 @@ struct table_style {
};
#define TABLE_STYLE_DEFAULT { TF_LIST, CF_STRING, true, JSSF_SORT, 0 }
+static const struct table_style table_style_default = TABLE_STYLE_DEFAULT;
#define TABLE_OPTION_ENUMS \
OPT_NO_HEADINGS,\
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 02/21] ovsdb-idl: Allow monitoring columns that are already monitored.
If IDL was created with monitoring and alerts turned on by default for
all columns, then there is no harm in allowing the API users to ask
again for monitoring and alerts to be enabled for any given column.
This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
commands once the IDL has already ran once.
Signed-off-by: Jakub Sitnicki
---
lib/ovsdb-idl.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9ab5d6723..ae0a55c3a 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
return >modes[column - table->class_->columns];
}
+static void
+ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
+ const struct ovsdb_idl_column *column,
+ unsigned char mode)
+{
+const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
+ column);
+size_t column_idx = column - table->class_->columns;
+
+if (table->modes[column_idx] != mode) {
+*ovsdb_idl_db_get_mode(db, column) = mode;
+}
+}
+
static void
add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
{
@@ -1136,7 +1150,7 @@ static void
ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
const struct ovsdb_idl_column *column)
{
-*ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
+ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
add_ref_table(db, >type.key);
add_ref_table(db, >type.value);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v5 00/21] Daemon mode for ovn-nbctl
rn at the end of a void function. Pointed out by Ben Pfaff. - Drop WIP patch for integration with tests from the series until a right approach can be found. Integration with tests that was used during development is available at: https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-11 - Minor style cleanups. Jakub Sitnicki (21): table: Introduce a constant for default table style. ovsdb-idl: Allow monitoring columns that are already monitored. ovn-nbctl: Extract the main loop. ovn-nbctl: Pull up destroying commands from do_nbctl(). ovn-nbctl: Pull up releasing IDL from do_nbctl(). ovn-nbctl: Signal need to try again via an output param. ovn-nbctl: Don't destroy the transaction twice on error. db-ctl-base: Propagate error from parse_command(). db-ctl-base: Propagate errors from the commands parser. ovn-nbctl: Propagate the error from do_nbctl(). ovn-nbctl: Propagate errors from the main loop. ovn-nbctl: Propagate errors from prerequisites runner. ovn-nbctl: Introduce a poll_timer based wait timeout. ovn-nbctl: Extract helper for printing oneline output. ovn-nbctl: Extract handling of options that affect main loop. ovn-nbctl: Extract a helper for building short options string. ovn-nbctl: Extract a helper for appending command options. ovn-nbctl: Initial support for daemon mode. tests: Add test for ovn-nbctl dry run mode. tests: Add test for oneline-formatted output for ovn-nbctl. tests: Add test for ovn-nbctl's command parser error paths. lib/db-ctl-base.c | 85 -- lib/db-ctl-base.h | 6 +- lib/ovsdb-idl.c | 16 +- lib/table.h | 2 + ovn/utilities/ovn-nbctl.8.xml | 40 +++ ovn/utilities/ovn-nbctl.c | 674 ++ ovn/utilities/ovn-sbctl.c | 7 +- tests/ovn-nbctl.at| 118 utilities/ovs-vsctl.c | 7 +- vtep/vtep-ctl.c | 7 +- 10 files changed, 796 insertions(+), 166 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-dev, v4, 11 of 21] ovn-nbctl: Propagate errors from the main loop.
On Thu, 19 Jul 2018 09:15:14 -0400 0-day Robot wrote: (...) > ovn/utilities/ovn-nbctl.c: In function ‘main’: > ovn/utilities/ovn-nbctl.c:133:11: error: redefinition of ‘error’ > char *error = main_loop(args, commands, n_commands, idl); >^ > ovn/utilities/ovn-nbctl.c:116:11: note: previous definition of ‘error’ was > here > char *error = ctl_parse_commands(argc - optind, argv + optind, >^ My fault - rebase gone wrong. This is fixed in the next commit. I will roll out v5 with a fix so that there are no build failures in the middle of the series. -Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 21/21] tests: Add test for ovn-nbctl's command parser error paths.
Preparatory work for getting rid of ctl_fatal() in command parser. Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 76 ++ 1 file changed, 76 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 4e6986a5e..ffbd2a140 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1424,3 +1424,79 @@ AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - commands parser error paths]) +OVN_NBCTL_TEST_START + +dnl FIXME: Duplicate options are allowed when passed with global options. +dnlFor example: ovn-nbctl --if-exists --if-exists list Logical_Switch + +dnl Duplicate option +AT_CHECK([ovn-nbctl -- --if-exists --if-exists list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option specified multiple times' stderr], [0], [ignore]) + +dnl Missing command +AT_CHECK([ovn-nbctl], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl --], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists], [1], [], [stderr]) +AT_CHECK([grep 'missing command name' stderr], [0], [ignore]) + +dnl Unknown command +AT_CHECK([ovn-nbctl foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- foo], [1], [], [stderr]) +AT_CHECK([grep 'unknown command' stderr], [0], [ignore]) + +dnl Unknown option +AT_CHECK([ovn-nbctl --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'unrecognized option' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'command has no .* option' stderr], [0], [ignore]) + +dnl Missing option argument +AT_CHECK([ovn-nbctl --columns], [1], [], [stderr]) +AT_CHECK([grep 'option .* requires an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --columns list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'missing argument to .* option' stderr], [0], [ignore]) + +dnl Unexpected option argument +AT_CHECK([ovn-nbctl --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option .* doesn'\''t allow an argument' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- --if-exists=foo list Logical_Switch], [1], [], [stderr]) +AT_CHECK([grep 'option on .* does not accept an argument' stderr], [0], [ignore]) + +dnl Not enough arguments +AT_CHECK([ovn-nbctl list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- list], [1], [], [stderr]) +AT_CHECK([grep 'command requires at least .* arguments' stderr], [0], [ignore]) + +dnl Too many arguments +AT_CHECK([ovn-nbctl show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl -- show foo --bar], [1], [], [stderr]) +AT_CHECK([grep 'command takes at most .* arguments' stderr], [0], [ignore]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 19/21] tests: Add test for ovn-nbctl dry run mode.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 73a61a4be..585967568 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1382,3 +1382,24 @@ inactivity_probe: 3 OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - dry run mode]) +OVN_NBCTL_TEST_START + +dnl Check that dry run has no permanent effect. +AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +]) + +dnl Check that dry-run mode is not sticky. +AT_CHECK([ovn-nbctl ls-add ls0]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 20/21] tests: Add test for oneline-formatted output for ovn-nbctl.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 585967568..4e6986a5e 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1403,3 +1403,24 @@ AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - oneline output]) +OVN_NBCTL_TEST_START + +AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1]) + +dnl Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +dnl Expect lines for two commands. +AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +<0> (ls0)\n<1> (ls1) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 18/21] ovn-nbctl: Initial support for daemon mode.
Make ovn-nbctl act as a unixctl server if we were asked to detach. This
turns ovn-nbctl into a long-lived process that acts a proxy for
interacting with NB DB. The main difference to regular mode of ovn-nbctl
is that in the daemon mode, a local copy of database contents has to be
obtained only once.
Just two unixctl commands are supported 'run' and 'exit'. The former can
be used to run any ovn-nbctl command or a batch of them as so:
ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
Running commands that have not yet been converted to not use ctl_fatal()
will result in death of the daemon process. However, --monitor option
can be used to keep the daemon running.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.8.xml | 40 +
ovn/utilities/ovn-nbctl.c | 336 ++
2 files changed, 344 insertions(+), 32 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index abba4ecdb..2cd2fab30 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -913,6 +913,43 @@
+Daemon Mode
+
+
+ If ovn-nbctl is invoked with the --detach
+ option (see Daemon Options, below), it runs in the
+ background as a daemon and accepts commands from ovs-appctl
+ (or another JSON-RPC client) indefinitely. The currently supported
+ commands are described below.
+
+
+
+
+
+
+
+
+run [options] command
+[arg...] [-- [options]
+command [arg...] ...]
+
+
+Instructs the daemon process to run one or more ovn-nbctl
+commands described above and reply with the results of running these
+commands. Accepts the --no-wait, --wait,
+--timeout, --dry-run, --oneline,
+and the options described under Table Formatting Options
+in addition to the the command-specific options.
+
+
+ exit
+ Causes ovn-nbctl to gracefully terminate.
+
+
+
+ Daemon mode is considered experimental.
+
+
Options
@@ -982,6 +1019,9 @@
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
Logging options
http://www.w3.org/2003/XInclude"/>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index b6ad78985..920f9b8ca 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -20,6 +20,7 @@
#include
#include "command-line.h"
+#include "daemon.h"
#include "db-ctl-base.h"
#include "dirs.h"
#include "fatal-signal.h"
@@ -38,6 +39,7 @@
#include "table.h"
#include "timeval.h"
#include "timer.h"
+#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status);
/* --leader-only, --no-leader-only: Only accept the leader in a cluster. */
static int leader_only = true;
+/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop"
+ commands. */
+static char *unixctl_path;
+
+static unixctl_cb_func server_cmd_exit;
+static unixctl_cb_func server_cmd_run;
+
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
@@ -98,15 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char
*args,
size_t n_commands,
struct ovsdb_idl *idl,
const struct timer *);
+static void server_loop(struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
{
struct ovsdb_idl *idl;
-struct ctl_command *commands;
struct shash local_options;
-size_t n_commands;
-char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -119,41 +126,59 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-error = ctl_parse_commands(argc - optind, argv + optind, _options,
- , _commands);
-if (error) {
-ctl_fatal("%s", error);
-}
-VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
- "Called as %s", args);
-
-if (timeout) {
-time_alarm(timeout);
-}
+argc -= optind;
+argv += optind;
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-error = run_prerequisites(commands, n_commands, idl);
-if (error) {
-ctl_fatal("%s", error);
-}
-error = main_loop(args, commands, n_commands, idl, NULL);
-if (error) {
-ctl_fatal("%s
[ovs-dev] [PATCH v4 17/21] ovn-nbctl: Extract a helper for appending command options.
Will be reused when parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 339a182e8..b6ad78985 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -289,6 +289,30 @@ build_short_options(const struct option *long_options)
return short_options;
}
+static struct option * OVS_WARN_UNUSED_RESULT
+append_command_options(const struct option *options, int opt_val)
+{
+struct option *o;
+size_t n_allocated;
+size_t n_existing;
+int i;
+
+for (i = 0; options[i].name; i++) {
+;
+}
+n_allocated = i + 1;
+n_existing = i;
+
+/* We want to parse both global and command-specific options here, but
+ * getopt_long() isn't too convenient for the job. We copy our global
+ * options into a dynamic array, then append all of the command-specific
+ * options. */
+o = xmemdup(options, n_allocated * sizeof *options);
+ctl_add_cmd_options(, _existing, _allocated, opt_val);
+
+return o;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -310,22 +334,11 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
char *short_options;
-
struct option *options;
-size_t allocated_options;
-size_t n_options;
size_t i;
short_options = build_short_options(global_long_options);
-
-/* We want to parse both global and command-specific options here, but
- * getopt_long() isn't too convenient for the job. We copy our global
- * options into a dynamic array, then append all of the command-specific
- * options. */
-options = xmemdup(global_long_options, sizeof global_long_options);
-allocated_options = ARRAY_SIZE(global_long_options);
-n_options = n_global_long_options;
-ctl_add_cmd_options(, _options, _options, OPT_LOCAL);
+options = append_command_options(global_long_options, OPT_LOCAL);
for (;;) {
int idx;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 16/21] ovn-nbctl: Extract a helper for building short options string.
Will be reused for parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a9113b9e0..339a182e8 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -277,6 +277,18 @@ handle_main_loop_option(int opt, const char *arg, bool
*handled)
return NULL;
}
+static char * OVS_WARN_UNUSED_RESULT
+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -297,16 +309,14 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
{NULL, 0, NULL, 0},
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
-char *tmp, *short_options;
+char *short_options;
struct option *options;
size_t allocated_options;
size_t n_options;
size_t i;
-tmp = ovs_cmdl_long_options_to_short_options(global_long_options);
-short_options = xasprintf("+%s", tmp);
-free(tmp);
+short_options = build_short_options(global_long_options);
/* We want to parse both global and command-specific options here, but
* getopt_long() isn't too convenient for the job. We copy our global
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 15/21] ovn-nbctl: Extract handling of options that affect main loop.
Provide a handler for options that change how the main loop behaves.
This will allow code reuse for option parsing in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 136 --
1 file changed, 84 insertions(+), 52 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index cce3ae023..a9113b9e0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -203,38 +203,93 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
return NULL;
}
+/* All options that affect the main loop and are not external. */
+#define MAIN_LOOP_OPTION_ENUMS \
+OPT_NO_WAIT,\
+OPT_WAIT, \
+OPT_DRY_RUN,\
+OPT_ONELINE
+
+#define MAIN_LOOP_LONG_OPTIONS \
+{"no-wait", no_argument, NULL, OPT_NO_WAIT}, \
+{"wait", required_argument, NULL, OPT_WAIT}, \
+{"dry-run", no_argument, NULL, OPT_DRY_RUN}, \
+{"oneline", no_argument, NULL, OPT_ONELINE}, \
+{"timeout", required_argument, NULL, 't'}
+
+enum {
+OPT_DB = UCHAR_MAX + 1,
+OPT_NO_SYSLOG,
+OPT_LOCAL,
+OPT_COMMANDS,
+OPT_OPTIONS,
+OPT_BOOTSTRAP_CA_CERT,
+MAIN_LOOP_OPTION_ENUMS,
+VLOG_OPTION_ENUMS,
+TABLE_OPTION_ENUMS,
+SSL_OPTION_ENUMS,
+};
+
+static char * OVS_WARN_UNUSED_RESULT
+handle_main_loop_option(int opt, const char *arg, bool *handled)
+{
+ovs_assert(handled);
+*handled = true;
+
+switch (opt) {
+case OPT_ONELINE:
+oneline = true;
+break;
+
+case OPT_NO_WAIT:
+wait_type = NBCTL_WAIT_NONE;
+break;
+
+case OPT_WAIT:
+if (!strcmp(arg, "none")) {
+wait_type = NBCTL_WAIT_NONE;
+} else if (!strcmp(arg, "sb")) {
+wait_type = NBCTL_WAIT_SB;
+} else if (!strcmp(arg, "hv")) {
+wait_type = NBCTL_WAIT_HV;
+} else {
+return xstrdup("argument to --wait must be "
+ "\"none\", \"sb\", or \"hv\"");
+}
+break;
+
+case OPT_DRY_RUN:
+dry_run = true;
+break;
+
+case 't':
+timeout = strtoul(arg, NULL, 10);
+if (timeout < 0) {
+return xasprintf("value %s on -t or --timeout is invalid", arg);
+}
+break;
+
+default:
+*handled = false;
+break;
+}
+
+return NULL;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
-enum {
-OPT_DB = UCHAR_MAX + 1,
-OPT_NO_SYSLOG,
-OPT_NO_WAIT,
-OPT_WAIT,
-OPT_DRY_RUN,
-OPT_ONELINE,
-OPT_LOCAL,
-OPT_COMMANDS,
-OPT_OPTIONS,
-OPT_BOOTSTRAP_CA_CERT,
-VLOG_OPTION_ENUMS,
-TABLE_OPTION_ENUMS,
-SSL_OPTION_ENUMS,
-};
static const struct option global_long_options[] = {
{"db", required_argument, NULL, OPT_DB},
{"no-syslog", no_argument, NULL, OPT_NO_SYSLOG},
-{"no-wait", no_argument, NULL, OPT_NO_WAIT},
-{"wait", required_argument, NULL, OPT_WAIT},
-{"dry-run", no_argument, NULL, OPT_DRY_RUN},
-{"oneline", no_argument, NULL, OPT_ONELINE},
-{"timeout", required_argument, NULL, 't'},
{"help", no_argument, NULL, 'h'},
{"commands", no_argument, NULL, OPT_COMMANDS},
{"options", no_argument, NULL, OPT_OPTIONS},
{"leader-only", no_argument, _only, true},
{"no-leader-only", no_argument, _only, false},
{"version", no_argument, NULL, 'V'},
+MAIN_LOOP_LONG_OPTIONS,
VLOG_LONG_OPTIONS,
STREAM_SSL_LONG_OPTIONS,
{"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -271,40 +326,24 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
break;
}
+bool handled;
+char *error = handle_main_loop_option(c, optarg, );
+if (error) {
+ctl_fatal("%s", error);
+}
+if (handled) {
+continue;
+}
+
switch (c) {
case OPT_DB:
db = optarg;
break;
-case OPT_ONELINE:
-oneline = true;
-break;
-
case OPT_NO_SYSLOG:
vlog_set_levels(_module, VLF_SYSLOG, VLL_WARN);
break;
-case OPT_NO_WAIT:
-wait_type = NBCTL_WAIT_NONE;
-break;
-
-case OPT_WAIT:
-if (!strcmp(optarg, "no
[ovs-dev] [PATCH v4 14/21] ovn-nbctl: Extract helper for printing oneline output.
This will allow us to direct oneline-formatted output to other sinks
than stdout if needed. Preparatory work for daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 53 ++-
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 00b75aa6c..cce3ae023 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4165,6 +4165,39 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
return NULL;
}
+static void
+oneline_format(struct ds *lines, struct ds *s)
+{
+size_t j;
+
+ds_chomp(lines, '\n');
+for (j = 0; j < lines->length; j++) {
+int ch = lines->string[j];
+switch (ch) {
+case '\n':
+ds_put_cstr(s, "\\n");
+break;
+
+case '\\':
+ds_put_cstr(s, "");
+break;
+
+default:
+ds_put_char(s, ch);
+}
+}
+ds_put_char(s, '\n');
+}
+
+static void
+oneline_print(struct ds *lines)
+{
+struct ds s = DS_EMPTY_INITIALIZER;
+oneline_format(lines, );
+fputs(ds_cstr(), stdout);
+ds_destroy();
+}
+
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
@@ -4300,25 +4333,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
if (c->table) {
table_print(c->table, _style);
} else if (oneline) {
-size_t j;
-
-ds_chomp(ds, '\n');
-for (j = 0; j < ds->length; j++) {
-int ch = ds->string[j];
-switch (ch) {
-case '\n':
-fputs("\\n", stdout);
-break;
-
-case '\\':
-fputs("", stdout);
-break;
-
-default:
-putchar(ch);
-}
-}
-putchar('\n');
+oneline_print(ds);
} else {
fputs(ds_cstr(ds), stdout);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 12/21] ovn-nbctl: Propagate errors from prerequisites runner.
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the prerequisites runner in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index ee423577e..ecb351a80 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -82,8 +82,9 @@ static int leader_only = true;
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
-static void run_prerequisites(struct ctl_command[], size_t n_commands,
- struct ovsdb_idl *);
+static char * OVS_WARN_UNUSED_RESULT run_prerequisites(struct ctl_command[],
+ size_t n_commands,
+ struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
struct ovsdb_idl *, bool *retry);
@@ -101,6 +102,7 @@ main(int argc, char *argv[])
struct ctl_command *commands;
struct shash local_options;
size_t n_commands;
+char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -113,8 +115,8 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-char *error = ctl_parse_commands(argc - optind, argv + optind,
- _options, , _commands);
+error = ctl_parse_commands(argc - optind, argv + optind, _options,
+ , _commands);
if (error) {
ctl_fatal("%s", error);
}
@@ -128,9 +130,12 @@ main(int argc, char *argv[])
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-run_prerequisites(commands, n_commands, idl);
+error = run_prerequisites(commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
-char *error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl);
if (error) {
ctl_fatal("%s", error);
}
@@ -4120,7 +4125,7 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
-static void
+static char *
run_prerequisites(struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -4141,7 +4146,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ctl_context_init(, c, idl, NULL, NULL, NULL);
(c->syntax->prerequisites)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+char *error = xstrdup(ctx.error);
+ctl_context_done(, c);
+return error;
}
ctl_context_done(, c);
@@ -4149,6 +4156,8 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ovs_assert(!c->table);
}
}
+
+return NULL;
}
static char *
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 13/21] ovn-nbctl: Introduce a poll_timer based wait timeout.
Extend the main loop and the command runner so that the caller can
specify a timeout for poll_block(). This will allow us to break out of
the main loop when waiting on IDL, like in the blocked '--wait=sb/hv
sync' case.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index ecb351a80..00b75aa6c 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -37,6 +37,7 @@
#include "svec.h"
#include "table.h"
#include "timeval.h"
+#include "timer.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -87,13 +88,16 @@ static char * OVS_WARN_UNUSED_RESULT
run_prerequisites(struct ctl_command[],
struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+ struct ovsdb_idl *,
+ const struct timer *,
+ bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
struct ctl_command *commands,
size_t n_commands,
- struct ovsdb_idl *idl);
+ struct ovsdb_idl *idl,
+ const struct timer *);
int
main(int argc, char *argv[])
@@ -135,7 +139,7 @@ main(int argc, char *argv[])
ctl_fatal("%s", error);
}
-error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl, NULL);
if (error) {
ctl_fatal("%s", error);
}
@@ -156,7 +160,7 @@ main(int argc, char *argv[])
static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout)
{
unsigned int seqno;
@@ -180,7 +184,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-char *error = do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl,
+ wait_timeout, );
if (error) {
return error;
}
@@ -4162,7 +4167,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl, bool *retry)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4289,8 +4294,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
OVS_NOT_REACHED();
}
-ovsdb_symbol_table_destroy(symtab);
-
for (c = commands; c < [n_commands]; c++) {
struct ds *ds = >output;
@@ -4334,11 +4337,19 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
}
}
ovsdb_idl_wait(idl);
+if (wait_timeout) {
+timer_wait(wait_timeout);
+}
poll_block();
+if (wait_timeout && timer_expired(wait_timeout)) {
+error = xstrdup("timeout expired");
+goto out_error;
+}
}
done: ;
}
+ovsdb_symbol_table_destroy(symtab);
ovsdb_idl_txn_destroy(txn);
the_idl_txn = NULL;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 11/21] ovn-nbctl: Propagate errors from the main loop.
Let the caller handle the errors instead of reporting it and
terminating. Prepare for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index e660b2cdf..ee423577e 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -89,8 +89,10 @@ static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char
*args,
struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
-static void main_loop(const char *args, struct ctl_command *commands,
- size_t n_commands, struct ovsdb_idl *idl);
+static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
+ struct ctl_command *commands,
+ size_t n_commands,
+ struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -128,7 +130,10 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
-main_loop(args, commands, n_commands, idl);
+char *error = main_loop(args, commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
@@ -144,7 +149,7 @@ main(int argc, char *argv[])
exit(EXIT_SUCCESS);
}
-static void
+static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -172,10 +177,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
bool retry;
char *error = do_nbctl(args, commands, n_commands, idl, );
if (error) {
-ctl_fatal("%s", error);
+return error;
}
if (!retry) {
-return;
+return NULL;
}
}
@@ -184,6 +189,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
poll_block();
}
}
+
+return NULL;
}
static void
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 10/21] ovn-nbctl: Propagate the error from do_nbctl().
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 46 --
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 465f8f6d1..e660b2cdf 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,9 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static void do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
+ struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -169,7 +170,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl, );
+if (error) {
+ctl_fatal("%s", error);
+}
if (!retry) {
return;
}
@@ -4140,7 +4144,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static void
+static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, bool *retry)
{
@@ -4151,6 +4155,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct ctl_command *c;
struct shash_node *node;
int64_t next_cfg = 0;
+char *error = NULL;
ovs_assert(retry);
@@ -4184,7 +4189,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
(c->syntax->run)();
}
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done_command(, c);
@@ -4198,9 +4205,10 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
SHASH_FOR_EACH (node, >sh) {
struct ovsdb_symbol *symbol = node->data;
if (!symbol->created) {
-ctl_fatal("row id \"%s\" is referenced but never created (e.g. "
- "with \"-- --id=%s create ...\")",
- node->name, node->name);
+error = xasprintf("row id \"%s\" is referenced but never created "
+ "(e.g. with \"-- --id=%s create ...\")",
+ node->name, node->name);
+goto out_error;
}
if (!symbol->strong_ref) {
if (!symbol->weak_ref) {
@@ -4225,7 +4233,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ctl_context_init(, c, idl, txn, symtab, NULL);
(c->syntax->postprocess)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done(, c);
}
@@ -4239,7 +4249,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
case TXN_ABORTED:
/* Should not happen--we never call ovsdb_idl_txn_abort(). */
-ctl_fatal("transaction aborted");
+error = xstrdup("transaction aborted");
+goto out_error;
case TXN_UNCHANGED:
case TXN_SUCCESS:
@@ -4249,11 +4260,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
goto try_again;
case TXN_ERROR:
-ctl_fatal("transaction error: %s", ovsdb_idl_txn_get_error(txn));
+error = xasprintf("transaction error: %s",
+ ovsdb_idl_txn_get_error(txn));
+goto out_error;
case TXN_NOT_LOCKED:
/* Should not happen--we never call ovsdb_idl_set_lock(). */
-ctl_fatal("database not locked");
+error = xstrdup("database not locked");
+goto out_error;
default:
OVS_NOT_REACHED();
@@ -4313
[ovs-dev] [PATCH v4 09/21] db-ctl-base: Propagate errors from the commands parser.
Let the caller decide how to handle the error. Prepare for using the
parser in ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 30 +-
lib/db-ctl-base.h | 6 +++---
ovn/utilities/ovn-nbctl.c | 7 +--
ovn/utilities/ovn-sbctl.c | 7 +--
utilities/ovs-vsctl.c | 7 +--
vtep/vtep-ctl.c | 7 +--
6 files changed, 44 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index f92da7c61..4e0eb9c5c 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -2251,22 +2251,24 @@ ctl_add_cmd_options(struct option **options_p, size_t
*n_options_p,
}
/* Parses command-line input for commands. */
-struct ctl_command *
+char *
ctl_parse_commands(int argc, char *argv[], struct shash *local_options,
- size_t *n_commandsp)
+ struct ctl_command **commandsp, size_t *n_commandsp)
{
struct ctl_command *commands;
size_t n_commands, allocated_commands;
int i, start;
+char *error;
commands = NULL;
n_commands = allocated_commands = 0;
+*commandsp = NULL;
+*n_commandsp = 0;
+
for (start = i = 0; i <= argc; i++) {
if (i == argc || !strcmp(argv[i], "--")) {
if (i > start) {
-char *error;
-
if (n_commands >= allocated_commands) {
struct ctl_command *c;
@@ -2277,21 +2279,31 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_options,
}
}
error = parse_command(i - start, [start], local_options,
- [n_commands++]);
+ [n_commands]);
if (error) {
-ctl_fatal("%s", error);
+struct ctl_command *c;
+
+for (c = commands; c < [n_commands]; c++) {
+shash_destroy_free_data(>options);
+}
+free(commands);
+
+return error;
}
+
+n_commands++;
} else if (!shash_is_empty(local_options)) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
start = i + 1;
}
}
if (!n_commands) {
-ctl_fatal("missing command name (use --help for help)");
+return xstrdup("missing command name (use --help for help)");
}
+*commandsp = commands;
*n_commandsp = n_commands;
-return commands;
+return NULL;
}
/* Prints all registered commands. */
diff --git a/lib/db-ctl-base.h b/lib/db-ctl-base.h
index ba771a180..284b573d0 100644
--- a/lib/db-ctl-base.h
+++ b/lib/db-ctl-base.h
@@ -166,9 +166,9 @@ void ctl_print_options(const struct option *);
void ctl_add_cmd_options(struct option **, size_t *n_options_p,
size_t *allocated_options_p, int opt_val);
void ctl_register_commands(const struct ctl_command_syntax *);
-struct ctl_command *ctl_parse_commands(int argc, char *argv[],
- struct shash *local_options,
- size_t *n_commandsp);
+char * OVS_WARN_UNUSED_RESULT ctl_parse_commands(
+int argc, char *argv[], struct shash *local_options,
+struct ctl_command **commandsp, size_t *n_commandsp);
/* Sometimes, it is desirable to print the table with weak reference to
* rows in a 'cmd_show_table' table. In that case, the 'weak_ref_table'
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 25194b2fa..465f8f6d1 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -110,8 +110,11 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
+char *error = ctl_parse_commands(argc - optind, argv + optind,
+ _options, , _commands);
+if (error) {
+ctl_fatal("%s", error);
+}
VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
"Called as %s", args);
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index c47cf6df9..7022347ed 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -112,8 +112,11 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
+char *error = ctl_parse_com
[ovs-dev] [PATCH v4 08/21] db-ctl-base: Propagate error from parse_command().
Let the caller handle the error. Needed for ovn-nbctl daemon mode.
Signed-off-by: Jakub Sitnicki
---
lib/db-ctl-base.c | 63 +--
1 file changed, 43 insertions(+), 20 deletions(-)
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
index ab35d6333..f92da7c61 100644
--- a/lib/db-ctl-base.c
+++ b/lib/db-ctl-base.c
@@ -1880,7 +1880,7 @@ cmd_wait_until(struct ctl_context *ctx)
}
/* Parses one command. */
-static void
+static char * OVS_WARN_UNUSED_RESULT
parse_command(int argc, char *argv[], struct shash *local_options,
struct ctl_command *command)
{
@@ -1888,6 +1888,7 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
struct shash_node *node;
int n_arg;
int i;
+char *error;
shash_init(>options);
shash_swap(local_options, >options);
@@ -1910,17 +1911,23 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
}
if (shash_find(>options, key)) {
-ctl_fatal("'%s' option specified multiple times", argv[i]);
+free(key);
+free(value);
+error = xasprintf("'%s' option specified multiple times", argv[i]);
+goto error;
}
shash_add_nocopy(>options, key, value);
}
if (i == argc) {
-ctl_fatal("missing command name (use --help for help)");
+error = xstrdup("missing command name (use --help for help)");
+goto error;
}
p = shash_find_data(_commands, argv[i]);
if (!p) {
-ctl_fatal("unknown command '%s'; use --help for help", argv[i]);
+error = xasprintf("unknown command '%s'; use --help for help",
+ argv[i]);
+goto error;
}
SHASH_FOR_EACH (node, >options) {
@@ -1928,43 +1935,54 @@ parse_command(int argc, char *argv[], struct shash
*local_options,
int end = s ? s[strlen(node->name)] : EOF;
if (!strchr("=,? ", end)) {
-ctl_fatal("'%s' command has no '%s' option",
- argv[i], node->name);
+error = xasprintf("'%s' command has no '%s' option",
+ argv[i], node->name);
+goto error;
}
if (end != '?' && (end == '=') != (node->data != NULL)) {
if (end == '=') {
-ctl_fatal("missing argument to '%s' option on '%s' "
- "command", node->name, argv[i]);
+error = xasprintf("missing argument to '%s' option on '%s' "
+ "command", node->name, argv[i]);
+goto error;
} else {
-ctl_fatal("'%s' option on '%s' does not accept an "
- "argument", node->name, argv[i]);
+error = xasprintf("'%s' option on '%s' does not accept an "
+ "argument", node->name, argv[i]);
+goto error;
}
}
}
n_arg = argc - i - 1;
if (n_arg < p->min_args) {
-ctl_fatal("'%s' command requires at least %d arguments",
- p->name, p->min_args);
+error = xasprintf("'%s' command requires at least %d arguments",
+ p->name, p->min_args);
+goto error;
} else if (n_arg > p->max_args) {
int j;
for (j = i + 1; j < argc; j++) {
if (argv[j][0] == '-') {
-ctl_fatal("'%s' command takes at most %d arguments "
- "(note that options must precede command "
- "names and follow a \"--\" argument)",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments "
+ "(note that options must precede command "
+ "names and follow a \"--\" argument)",
+ p->name, p->max_args);
+goto error;
}
}
-ctl_fatal("'%s' command takes at most %d arguments",
- p->name, p->max_args);
+error = xasprintf("'%s' command takes at most %d arguments",
+ p->name, p->max_args);
+goto error;
}
command->syntax = p;
command->argc = n_arg + 1;
command->argv = [i];
+return NULL;
+
+error:
+shash_destroy_free_data(>options);
+return error;
}
static void
@@ -2247,6 +2265,8 @@ ctl_parse_commands(int argc, char *argv[], struct shash
*local_o
[ovs-dev] [PATCH v4 07/21] ovn-nbctl: Don't destroy the transaction twice on error.
If transaction succeeded reset the state. Otherwise nbctl_exit() callback will try to clean up on any fatal error. Signed-off-by: Jakub Sitnicki --- ovn/utilities/ovn-nbctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 360b25a89..25194b2fa 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -4307,6 +4307,7 @@ do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands, } ovsdb_idl_txn_destroy(txn); +the_idl_txn = NULL; *retry = false; return; -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 06/21] ovn-nbctl: Signal need to try again via an output param.
Introduce an output parameter for the flag that signals need to retry
running the command. This leaves the return value for error reporting.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a700695fe..360b25a89 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,8 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static bool do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+static void do_nbctl(const char *args, struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -164,7 +164,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
-if (do_nbctl(args, commands, n_commands, idl)) {
+
+bool retry;
+do_nbctl(args, commands, n_commands, idl, );
+if (!retry) {
return;
}
}
@@ -4134,9 +4137,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static bool
+static void
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4146,6 +4149,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct shash_node *node;
int64_t next_cfg = 0;
+ovs_assert(retry);
+
txn = the_idl_txn = ovsdb_idl_txn_create(idl);
if (dry_run) {
ovsdb_idl_txn_set_dry_run(txn);
@@ -4303,7 +4308,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ovsdb_idl_txn_destroy(txn);
-return true;
+*retry = false;
+return;
try_again:
/* Our transaction needs to be rerun, or a prerequisite was not met. Free
@@ -4318,7 +4324,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
table_destroy(c->table);
free(c->table);
}
-return false;
+*retry = true;
}
/* Frees the current transaction and the underlying IDL and then calls
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 03/21] ovn-nbctl: Extract the main loop.
Split out a routine for the main ovn-nbctl loop.
Preparatory work for introducing daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 47df19b23..a027553b7 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -88,6 +88,8 @@ static bool do_nbctl(const char *args, struct ctl_command *,
size_t n,
struct ovsdb_idl *);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
+static void main_loop(const char *args, struct ctl_command *commands,
+ size_t n_commands, struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -95,7 +97,6 @@ main(int argc, char *argv[])
struct ovsdb_idl *idl;
struct ctl_command *commands;
struct shash local_options;
-unsigned int seqno;
size_t n_commands;
set_program_name(argv[0]);
@@ -123,6 +124,18 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
+main_loop(args, commands, n_commands, idl);
+
+free(args);
+exit(EXIT_SUCCESS);
+}
+
+static void
+main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
+ struct ovsdb_idl *idl)
+{
+unsigned int seqno;
+
/* Execute the commands.
*
* 'seqno' is the database sequence number for which we last tried to
@@ -136,14 +149,13 @@ main(int argc, char *argv[])
if (!ovsdb_idl_is_alive(idl)) {
int retval = ovsdb_idl_get_last_error(idl);
ctl_fatal("%s: database connection failed (%s)",
-db, ovs_retval_to_string(retval));
+ db, ovs_retval_to_string(retval));
}
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
if (do_nbctl(args, commands, n_commands, idl)) {
-free(args);
-exit(EXIT_SUCCESS);
+return;
}
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 04/21] ovn-nbctl: Pull up destroying commands from do_nbctl().
Destroy commands in the same routine where they were allocated.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a027553b7..51527741b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -126,6 +126,13 @@ main(int argc, char *argv[])
main_loop(args, commands, n_commands, idl);
+for (struct ctl_command *c = commands; c < [n_commands]; c++) {
+ds_destroy(>output);
+table_destroy(c->table);
+free(c->table);
+shash_destroy_free_data(>options);
+}
+free(commands);
free(args);
exit(EXIT_SUCCESS);
}
@@ -4271,13 +4278,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
} else {
fputs(ds_cstr(ds), stdout);
}
-ds_destroy(>output);
-table_destroy(c->table);
-free(c->table);
-
-shash_destroy_free_data(>options);
}
-free(commands);
if (wait_type != NBCTL_WAIT_NONE && status != TXN_UNCHANGED) {
ovsdb_idl_enable_reconnect(idl);
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 02/21] ovsdb-idl: Allow monitoring columns that are already monitored.
If IDL was created with monitoring and alerts turned on by default for
all columns, then there is no harm in allowing the API users to ask
again for monitoring and alerts to be enabled for any given column.
This allows us to run prerequisites handlers for db-ctl and ovn-nbctl
commands once the IDL has already ran once.
Signed-off-by: Jakub Sitnicki
---
lib/ovsdb-idl.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9ab5d6723..ae0a55c3a 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1116,6 +1116,20 @@ ovsdb_idl_db_get_mode(struct ovsdb_idl_db *db,
return >modes[column - table->class_->columns];
}
+static void
+ovsdb_idl_db_set_mode(struct ovsdb_idl_db *db,
+ const struct ovsdb_idl_column *column,
+ unsigned char mode)
+{
+const struct ovsdb_idl_table *table = ovsdb_idl_table_from_column(db,
+ column);
+size_t column_idx = column - table->class_->columns;
+
+if (table->modes[column_idx] != mode) {
+*ovsdb_idl_db_get_mode(db, column) = mode;
+}
+}
+
static void
add_ref_table(struct ovsdb_idl_db *db, const struct ovsdb_base_type *base)
{
@@ -1136,7 +1150,7 @@ static void
ovsdb_idl_db_add_column(struct ovsdb_idl_db *db,
const struct ovsdb_idl_column *column)
{
-*ovsdb_idl_db_get_mode(db, column) = OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT;
+ovsdb_idl_db_set_mode(db, column, OVSDB_IDL_MONITOR | OVSDB_IDL_ALERT);
add_ref_table(db, >type.key);
add_ref_table(db, >type.value);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 01/21] table: Introduce a constant for default table style.
Having a constant in addition to the constant expression for the default
table style allows us to reset 'struct table_style' variables to default
style.
Signed-off-by: Jakub Sitnicki
---
lib/table.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/table.h b/lib/table.h
index 313ac1dd2..76e65bb70 100644
--- a/lib/table.h
+++ b/lib/table.h
@@ -20,6 +20,7 @@
#include
#include
#include "compiler.h"
+#include "openvswitch/json.h"
struct ds;
struct table_style;
@@ -83,6 +84,7 @@ struct table_style {
};
#define TABLE_STYLE_DEFAULT { TF_LIST, CF_STRING, true, JSSF_SORT, 0 }
+static const struct table_style table_style_default = TABLE_STYLE_DEFAULT;
#define TABLE_OPTION_ENUMS \
OPT_NO_HEADINGS,\
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v4 00/21] Daemon mode for ovn-nbctl
was used during development is available at: https://github.com/jsitnicki/ovs/commits/wip-nbctl-daemon-2018-07-11 - Minor style cleanups. Jakub Sitnicki (21): table: Introduce a constant for default table style. ovsdb-idl: Allow monitoring columns that are already monitored. ovn-nbctl: Extract the main loop. ovn-nbctl: Pull up destroying commands from do_nbctl(). ovn-nbctl: Pull up releasing IDL from do_nbctl(). ovn-nbctl: Signal need to try again via an output param. ovn-nbctl: Don't destroy the transaction twice on error. db-ctl-base: Propagate error from parse_command(). db-ctl-base: Propagate errors from the commands parser. ovn-nbctl: Propagate the error from do_nbctl(). ovn-nbctl: Propagate errors from the main loop. ovn-nbctl: Propagate errors from prerequisites runner. ovn-nbctl: Introduce a poll_timer based wait timeout. ovn-nbctl: Extract helper for printing oneline output. ovn-nbctl: Extract handling of options that affect main loop. ovn-nbctl: Extract a helper for building short options string. ovn-nbctl: Extract a helper for appending command options. ovn-nbctl: Initial support for daemon mode. tests: Add test for ovn-nbctl dry run mode. tests: Add test for oneline-formatted output for ovn-nbctl. tests: Add test for ovn-nbctl's command parser error paths. lib/db-ctl-base.c | 85 -- lib/db-ctl-base.h | 6 +- lib/ovsdb-idl.c | 16 +- lib/table.h | 2 + ovn/utilities/ovn-nbctl.8.xml | 40 +++ ovn/utilities/ovn-nbctl.c | 674 ++ ovn/utilities/ovn-sbctl.c | 7 +- tests/ovn-nbctl.at| 118 utilities/ovs-vsctl.c | 7 +- vtep/vtep-ctl.c | 7 +- 10 files changed, 796 insertions(+), 166 deletions(-) -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH 01/11] ovn-nbctl: Don't die in pg_by_name_or_uuid().
(Cover-letter awaits moderator approval. Reposting here.) This is a continuation of an earlier series that aims to replace calls to ctl_fatal() in command handlers in ovn-nbctl. The motivation is to handle errors gracefully when running commands in daemon mode because as a long-lived process we shouldn't terminate on errors that we can recover from. After this series there are no more ctl_fatal() calls in ovn-nbctl that affect the daemon mode. The only remaining function left to convert is the commands parser in db-ctl-base module (ctl_parse_commands()), which I intend to deal with separately. Either as a part of ovn-nbctl daemon series (already in review [1]), or as a follow-up to it. Thanks, Jakub [1] https://patchwork.ozlabs.org/project/openvswitch/list/?series=55472 Jakub Sitnicki (11): ovn-nbctl: Don't die in pg_by_name_or_uuid(). ovn-nbctl: Don't die in gc_by_name_or_uuid(). ovn-nbctl: Don't die in lsp_to_ls(). ovn-nbctl: Don't die in lrp_to_lr(). ovn-nbctl: Don't die in parse_priority(). ovn-nbctl: Don't die in parse_direction(). ovn-nbctl: Don't die in dhcp_options_get(). ovn-nbctl: Propagate error thru the context. ovn-nbctl: Use ctl_error() in command handlers. ovn-nbctl: Remove pointless "return;" at ends of functions. ovn-nbctl: Fix mem leak in nbctl_lrp_set_gateway_chassis(). ovn/utilities/ovn-nbctl.c | 360 -- tests/ovn-nbctl.at| 5 + 2 files changed, 260 insertions(+), 105 deletions(-) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 11/11] ovn-nbctl: Fix mem leak in nbctl_lrp_set_gateway_chassis().
Fix fall-out from applying a semantic patch to propagate the error.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 27e3151a8..12845a035 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -3473,6 +3473,7 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
error = gc_by_name_or_uuid(ctx, gc_name, false, _gc);
if (error) {
ctx->error = error;
+free(gc_name);
return;
}
if (existing_gc) {
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 10/11] ovn-nbctl: Remove pointless "return; " at ends of functions.
Fix fall-out from applying a semantic patch that converts ctl_fatal() calls to use ctl_error(). Signed-off-by: Jakub Sitnicki --- ovn/utilities/ovn-nbctl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 8301e74cc..27e3151a8 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -1149,7 +1149,6 @@ nbctl_lsp_del(struct ctl_context *ctx) /* Can't happen because of the database schema. */ ctl_error(ctx, "logical port %s is not part of any logical switch", ctx->argv[1]); -return; } static void @@ -3787,7 +3786,6 @@ nbctl_lrp_del(struct ctl_context *ctx) /* Can't happen because of the database schema. */ ctl_error(ctx, "logical port %s is not part of any logical router", ctx->argv[1]); -return; } /* Print a list of logical router ports. */ -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 09/11] ovn-nbctl: Use ctl_error() in command handlers.
Instead of dying with ctl_fatal(), propagate the error thru the context.
This will allow us to report errors when running in daemon mode.
This patch is a result of applying the following semantic patch:
@@
identifier F, C;
expression S;
@@
static void F(struct ctl_context *C) {
<...
- ctl_fatal(S);
+ ctl_error(C, S);
+ return;
...>
}
@@
identifier F, C;
expression S, A;
@@
static void F(struct ctl_context *C) {
<...
- ctl_fatal(S, A);
+ ctl_error(C, S, A);
+ return;
...>
}
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 36
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index cf2b8eced..8301e74cc 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1147,8 +1147,9 @@ nbctl_lsp_del(struct ctl_context *ctx)
}
/* Can't happen because of the database schema. */
-ctl_fatal("logical port %s is not part of any logical switch",
+ctl_error(ctx, "logical port %s is not part of any logical switch",
ctx->argv[1]);
+return;
}
static void
@@ -1230,10 +1231,11 @@ nbctl_lsp_set_addresses(struct ctl_context *ctx)
&& strcmp(ctx->argv[i], "router")
&& !ovs_scan(ctx->argv[i], ETH_ADDR_SCAN_FMT,
ETH_ADDR_SCAN_ARGS(ea))) {
-ctl_fatal("%s: Invalid address format. See ovn-nb(5). "
+ctl_error(ctx, "%s: Invalid address format. See ovn-nb(5). "
"Hint: An Ethernet address must be "
"listed before an IP address, together as a single "
"argument.", ctx->argv[i]);
+return;
}
}
@@ -1480,7 +1482,9 @@ nbctl_lsp_set_dhcpv4_options(struct ctl_context *ctx)
error = ip_parse_cidr(dhcp_opt->cidr, , );
if (error){
free(error);
-ctl_fatal("DHCP options cidr '%s' is not IPv4", dhcp_opt->cidr);
+ctl_error(ctx, "DHCP options cidr '%s' is not IPv4",
+ dhcp_opt->cidr);
+return;
}
}
nbrec_logical_switch_port_set_dhcpv4_options(lsp, dhcp_opt);
@@ -1512,7 +1516,9 @@ nbctl_lsp_set_dhcpv6_options(struct ctl_context *ctx)
error = ipv6_parse_cidr(dhcp_opt->cidr, , );
if (error) {
free(error);
-ctl_fatal("DHCP options cidr '%s' is not IPv6", dhcp_opt->cidr);
+ctl_error(ctx, "DHCP options cidr '%s' is not IPv6",
+ dhcp_opt->cidr);
+return;
}
}
nbrec_logical_switch_port_set_dhcpv6_options(lsp, dhcp_opt);
@@ -1883,7 +1889,8 @@ nbctl_acl_del(struct ctl_context *ctx)
}
if (ctx->argc == 4) {
-ctl_fatal("cannot specify priority without match");
+ctl_error(ctx, "cannot specify priority without match");
+return;
}
/* Remove the matching rule. */
@@ -2120,7 +2127,8 @@ nbctl_qos_del(struct ctl_context *ctx)
}
if (ctx->argc == 4) {
-ctl_fatal("cannot specify priority without match");
+ctl_error(ctx, "cannot specify priority without match");
+return;
}
/* Remove the matching rule. */
@@ -2501,8 +2509,9 @@ nbctl_lr_lb_del(struct ctl_context *ctx)
bool must_exist = !shash_find(>options, "--if-exists");
if (must_exist) {
-ctl_fatal("load balancer %s is not part of any logical router.",
-del_lb->name);
+ctl_error(ctx, "load balancer %s is not part of any logical router.",
+ del_lb->name);
+return;
}
}
@@ -2625,8 +2634,9 @@ nbctl_ls_lb_del(struct ctl_context *ctx)
bool must_exist = !shash_find(>options, "--if-exists");
if (must_exist) {
-ctl_fatal("load balancer %s is not part of any logical switch.",
-del_lb->name);
+ctl_error(ctx, "load balancer %s is not part of any logical switch.",
+ del_lb->name);
+return;
}
}
@@ -2770,7 +2780,8 @@ nbctl_dhcp_options_create(struct ctl_context *ctx)
error = ipv6_parse_cidr(ctx->argv[1], , );
if (error) {
free(error);
-ctl_fatal("Invalid cidr format '%s'", ctx->argv[1]);
+ctl_error(ctx, "Invalid cidr format '%s'", ctx->argv[1]);
+return;
}
}
@@ -3774,8 +3785,9 @@ nbctl_lrp_del(struct ctl_context *ctx)
}
/* Can't happen because of the database schema. */
-ctl_fatal("logical port %s is not part of any logical router",
+ctl_error(ctx, "logical port %s is not part of any logical router",
ctx->argv[1]);
+return;
}
/* Print a list of logical router ports. */
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 08/11] ovn-nbctl: Propagate error thru the context.
Instead of dying let the main loop handle the error.
This will allow us to report errors when running in daemon mode.
This is a result of applying the following semantic patch:
@@
identifier F;
identifier C;
identifier E;
@@
static void F(struct ctl_context *C) {
<...
if (E) {
- ctl_fatal("%s", E);
+ C->error = E;
+ return;
}
...>
}
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 135 ++
1 file changed, 90 insertions(+), 45 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index cf49a6b11..cf2b8eced 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -813,7 +813,8 @@ nbctl_show(struct ctl_context *ctx)
if (ctx->argc == 2) {
char *error = ls_by_name_or_uuid(ctx, ctx->argv[1], false, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
if (ls) {
print_ls(ls, >output);
@@ -828,7 +829,8 @@ nbctl_show(struct ctl_context *ctx)
if (ctx->argc == 2) {
char *error = lr_by_name_or_uuid(ctx, ctx->argv[1], false, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
if (lr) {
print_lr(lr, >output);
@@ -1126,7 +1128,8 @@ nbctl_lsp_del(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, ctx->argv[1], must_exist, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
if (!lsp) {
return;
@@ -1158,7 +1161,8 @@ nbctl_lsp_list(struct ctl_context *ctx)
char *error = ls_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
smap_init();
@@ -1183,7 +1187,8 @@ nbctl_lsp_get_parent(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, ctx->argv[1], true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
if (lsp->parent_name) {
ds_put_format(>output, "%s\n", lsp->parent_name);
@@ -1197,7 +1202,8 @@ nbctl_lsp_get_tag(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, ctx->argv[1], true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
if (lsp->n_tag > 0) {
ds_put_format(>output, "%"PRId64"\n", lsp->tag[0]);
@@ -1212,7 +1218,8 @@ nbctl_lsp_set_addresses(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
int i;
@@ -1245,7 +1252,8 @@ nbctl_lsp_get_addresses(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
svec_init();
@@ -1267,7 +1275,8 @@ nbctl_lsp_set_port_security(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
nbrec_logical_switch_port_set_port_security(lsp,
(const char **) ctx->argv + 2, ctx->argc - 2);
@@ -1284,7 +1293,8 @@ nbctl_lsp_get_port_security(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
svec_init();
for (i = 0; i < lsp->n_port_security; i++) {
@@ -1305,7 +1315,8 @@ nbctl_lsp_get_up(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
ds_put_format(>output,
"%s\n", (lsp->up && *lsp->up) ? "up" : "down");
@@ -1336,12 +1347,14 @@ nbctl_lsp_set_enabled(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
bool enabled;
error = parse_enabled(state, );
if (error) {
-ctl_fatal("%s", error);
+ctx->error = error;
+return;
}
nbrec_logical_switch_port_set_enabled(lsp, , 1);
}
@@ -1354,7 +1367,8 @@ nbctl_lsp_get_enabled(struct ctl_context *ctx)
char *error = lsp_by_name_or_uuid(ctx, id, true, );
if (error) {
-ctl_fatal("%s", erro
[ovs-dev] [PATCH 07/11] ovn-nbctl: Don't die in dhcp_options_get().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 44 +++-
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index c18fa2825..cf49a6b11 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -86,8 +86,9 @@ static void run_prerequisites(struct ctl_command[], size_t
n_commands,
struct ovsdb_idl *);
static bool do_nbctl(const char *args, struct ctl_command *, size_t n,
struct ovsdb_idl *);
-static const struct nbrec_dhcp_options *dhcp_options_get(
-struct ctl_context *ctx, const char *id, bool must_exist);
+static char * OVS_WARN_UNUSED_RESULT dhcp_options_get(
+struct ctl_context *ctx, const char *id, bool must_exist,
+const struct nbrec_dhcp_options **);
int
main(int argc, char *argv[])
@@ -1448,7 +1449,10 @@ nbctl_lsp_set_dhcpv4_options(struct ctl_context *ctx)
}
const struct nbrec_dhcp_options *dhcp_opt = NULL;
if (ctx->argc == 3 ) {
-dhcp_opt = dhcp_options_get(ctx, ctx->argv[2], true);
+error = dhcp_options_get(ctx, ctx->argv[2], true, _opt);
+if (error) {
+ctl_fatal("%s", error);
+}
}
if (dhcp_opt) {
@@ -1475,7 +1479,10 @@ nbctl_lsp_set_dhcpv6_options(struct ctl_context *ctx)
}
const struct nbrec_dhcp_options *dhcp_opt = NULL;
if (ctx->argc == 3) {
-dhcp_opt = dhcp_options_get(ctx, ctx->argv[2], true);
+error = dhcp_options_get(ctx, ctx->argv[2], true, _opt);
+if (error) {
+ctl_fatal("%s", error);
+}
}
if (dhcp_opt) {
@@ -2696,8 +2703,9 @@ nbctl_lr_list(struct ctl_context *ctx)
free(nodes);
}
-static const struct nbrec_dhcp_options *
-dhcp_options_get(struct ctl_context *ctx, const char *id, bool must_exist)
+static char *
+dhcp_options_get(struct ctl_context *ctx, const char *id, bool must_exist,
+ const struct nbrec_dhcp_options **dhcp_opts_p)
{
struct uuid dhcp_opts_uuid;
const struct nbrec_dhcp_options *dhcp_opts = NULL;
@@ -2706,9 +2714,10 @@ dhcp_options_get(struct ctl_context *ctx, const char
*id, bool must_exist)
}
if (!dhcp_opts && must_exist) {
-ctl_fatal("%s: dhcp options UUID not found", id);
+return xasprintf("%s: dhcp options UUID not found", id);
}
-return dhcp_opts;
+*dhcp_opts_p = dhcp_opts;
+return NULL;
}
static void
@@ -2750,8 +2759,11 @@ nbctl_dhcp_options_create(struct ctl_context *ctx)
static void
nbctl_dhcp_options_set_options(struct ctl_context *ctx)
{
-const struct nbrec_dhcp_options *dhcp_opts = dhcp_options_get(
-ctx, ctx->argv[1], true);
+const struct nbrec_dhcp_options *dhcp_opts;
+char *error = dhcp_options_get(ctx, ctx->argv[1], true, _opts);
+if (error) {
+ctl_fatal("%s", error);
+}
struct smap dhcp_options = SMAP_INITIALIZER(_options);
for (size_t i = 2; i < ctx->argc; i++) {
@@ -2771,8 +2783,11 @@ nbctl_dhcp_options_set_options(struct ctl_context *ctx)
static void
nbctl_dhcp_options_get_options(struct ctl_context *ctx)
{
-const struct nbrec_dhcp_options *dhcp_opts = dhcp_options_get(
-ctx, ctx->argv[1], true);
+const struct nbrec_dhcp_options *dhcp_opts;
+char *error = dhcp_options_get(ctx, ctx->argv[1], true, _opts);
+if (error) {
+ctl_fatal("%s", error);
+}
struct smap_node *node;
SMAP_FOR_EACH(node, _opts->options) {
@@ -2787,7 +2802,10 @@ nbctl_dhcp_options_del(struct ctl_context *ctx)
const char *id = ctx->argv[1];
const struct nbrec_dhcp_options *dhcp_opts;
-dhcp_opts = dhcp_options_get(ctx, id, must_exist);
+char *error = dhcp_options_get(ctx, id, must_exist, _opts);
+if (error) {
+ctl_fatal("%s", error);
+}
if (!dhcp_opts) {
return;
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 06/11] ovn-nbctl: Don't die in parse_direction().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 39 ++-
1 file changed, 30 insertions(+), 9 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 1cee059f2..c18fa2825 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1671,17 +1671,19 @@ qos_cmp(const void *qos1_, const void *qos2_)
}
}
-static const char *
-parse_direction(const char *arg)
+static char * OVS_WARN_UNUSED_RESULT
+parse_direction(const char *arg, const char **direction_p)
{
/* Validate direction. Only require the first letter. */
if (arg[0] == 't') {
-return "to-lport";
+*direction_p = "to-lport";
} else if (arg[0] == 'f') {
-return "from-lport";
+*direction_p = "from-lport";
} else {
-ctl_fatal("%s: direction must be \"to-lport\" or \"from-lport\"", arg);
+return xasprintf("%s: direction must be \"to-lport\" or "
+ "\"from-lport\"", arg);
}
+return NULL;
}
static char * OVS_WARN_UNUSED_RESULT
@@ -1710,7 +1712,12 @@ nbctl_acl_add(struct ctl_context *ctx)
return;
}
-const char *direction = parse_direction(ctx->argv[2]);
+const char *direction;
+error = parse_direction(ctx->argv[2], );
+if (error) {
+ctx->error = error;
+return;
+}
int64_t priority;
error = parse_priority(ctx->argv[3], );
if (error) {
@@ -1804,7 +1811,11 @@ nbctl_acl_del(struct ctl_context *ctx)
return;
}
-const char *direction = parse_direction(ctx->argv[2]);
+const char *direction;
+error = parse_direction(ctx->argv[2], );
+if (error) {
+ctl_fatal("%s", error);
+}
size_t n_acls = pg ? pg->n_acls : ls->n_acls;
struct nbrec_acl **acls = pg ? pg->acls : ls->acls;
@@ -1917,13 +1928,18 @@ static void
nbctl_qos_add(struct ctl_context *ctx)
{
const struct nbrec_logical_switch *ls;
-const char *direction = parse_direction(ctx->argv[2]);
+const char *direction;
int64_t priority;
int64_t dscp = -1;
int64_t rate = 0;
int64_t burst = 0;
char *error;
+error = parse_direction(ctx->argv[2], );
+if (error) {
+ctx->error = error;
+return;
+}
error = parse_priority(ctx->argv[3], );
if (error) {
ctx->error = error;
@@ -2034,7 +2050,12 @@ nbctl_qos_del(struct ctl_context *ctx)
return;
}
-const char *direction = parse_direction(ctx->argv[2]);
+const char *direction;
+error = parse_direction(ctx->argv[2], );
+if (error) {
+ctx->error = error;
+return;
+}
/* If priority and match are not specified, delete all qos_rules with the
* specified direction. */
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 05/11] ovn-nbctl: Don't die in parse_priority().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 44 ++--
1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index f89ddbc0a..1cee059f2 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -1684,16 +1684,17 @@ parse_direction(const char *arg)
}
}
-static int
-parse_priority(const char *arg)
+static char * OVS_WARN_UNUSED_RESULT
+parse_priority(const char *arg, int64_t *priority_p)
{
/* Validate priority. */
int64_t priority;
if (!ovs_scan(arg, "%"SCNd64, )
|| priority < 0 || priority > 32767) {
-ctl_fatal("%s: priority must in range 0...32767", arg);
+return xasprintf("%s: priority must in range 0...32767", arg);
}
-return priority;
+*priority_p = priority;
+return NULL;
}
static void
@@ -1710,7 +1711,12 @@ nbctl_acl_add(struct ctl_context *ctx)
}
const char *direction = parse_direction(ctx->argv[2]);
-int64_t priority = parse_priority(ctx->argv[3]);
+int64_t priority;
+error = parse_priority(ctx->argv[3], );
+if (error) {
+ctx->error = error;
+return;
+}
/* Validate action. */
if (strcmp(action, "allow") && strcmp(action, "allow-related")
@@ -1825,7 +1831,11 @@ nbctl_acl_del(struct ctl_context *ctx)
return;
}
-int64_t priority = parse_priority(ctx->argv[3]);
+int64_t priority;
+error = parse_priority(ctx->argv[3], );
+if (error) {
+ctl_fatal("%s", error);
+}
if (ctx->argc == 4) {
ctl_fatal("cannot specify priority without match");
@@ -1908,12 +1918,18 @@ nbctl_qos_add(struct ctl_context *ctx)
{
const struct nbrec_logical_switch *ls;
const char *direction = parse_direction(ctx->argv[2]);
-int64_t priority = parse_priority(ctx->argv[3]);
+int64_t priority;
int64_t dscp = -1;
int64_t rate = 0;
int64_t burst = 0;
+char *error;
-char *error = ls_by_name_or_uuid(ctx, ctx->argv[1], true, );
+error = parse_priority(ctx->argv[3], );
+if (error) {
+ctx->error = error;
+return;
+}
+error = ls_by_name_or_uuid(ctx, ctx->argv[1], true, );
if (error) {
ctx->error = error;
return;
@@ -2039,7 +2055,11 @@ nbctl_qos_del(struct ctl_context *ctx)
return;
}
-int64_t priority = parse_priority(ctx->argv[3]);
+int64_t priority;
+error = parse_priority(ctx->argv[3], );
+if (error) {
+ctl_fatal("%s", error);
+}
if (ctx->argc == 4) {
ctl_fatal("cannot specify priority without match");
@@ -3352,7 +3372,11 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
const char *chassis_name = ctx->argv[2];
if (ctx->argv[3]) {
-priority = parse_priority(ctx->argv[3]);
+error = parse_priority(ctx->argv[3], );
+if (error) {
+ctx->error = error;
+return;
+}
}
gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 04/11] ovn-nbctl: Don't die in lrp_to_lr().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 19e99670b..f89ddbc0a 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -3270,22 +3270,26 @@ lrp_by_name_or_uuid(struct ctl_context *ctx, const char
*id, bool must_exist,
}
/* Returns the logical router that contains 'lrp'. */
-static const struct nbrec_logical_router *
+static char * OVS_WARN_UNUSED_RESULT
lrp_to_lr(const struct ovsdb_idl *idl,
- const struct nbrec_logical_router_port *lrp)
+ const struct nbrec_logical_router_port *lrp,
+ const struct nbrec_logical_router **lr_p)
{
const struct nbrec_logical_router *lr;
+*lr_p = NULL;
+
NBREC_LOGICAL_ROUTER_FOR_EACH (lr, idl) {
for (size_t i = 0; i < lr->n_ports; i++) {
if (lr->ports[i] == lrp) {
-return lr;
+*lr_p = lr;
+return NULL;
}
}
}
/* Can't happen because of the database schema */
-ctl_fatal("port %s is not part of any logical router",
- lrp->name);
+return xasprintf("port %s is not part of any logical router",
+ lrp->name);
}
static const char *
@@ -3513,7 +3517,11 @@ nbctl_lrp_add(struct ctl_context *ctx)
}
const struct nbrec_logical_router *bound_lr;
-bound_lr = lrp_to_lr(ctx->idl, lrp);
+error = lrp_to_lr(ctx->idl, lrp, _lr);
+if (error) {
+ctx->error = error;
+return;
+}
if (bound_lr != lr) {
char uuid_s[UUID_LEN + 1];
ctl_error(ctx, "%s: port already exists but in router %s",
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 03/11] ovn-nbctl: Don't die in lsp_to_ls().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 ++--
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index df35d11d6..19e99670b 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -952,22 +952,26 @@ lsp_by_name_or_uuid(struct ctl_context *ctx, const char
*id,
}
/* Returns the logical switch that contains 'lsp'. */
-static const struct nbrec_logical_switch *
+static char * OVS_WARN_UNUSED_RESULT
lsp_to_ls(const struct ovsdb_idl *idl,
- const struct nbrec_logical_switch_port *lsp)
+ const struct nbrec_logical_switch_port *lsp,
+ const struct nbrec_logical_switch **ls_p)
{
const struct nbrec_logical_switch *ls;
+*ls_p = NULL;
+
NBREC_LOGICAL_SWITCH_FOR_EACH (ls, idl) {
for (size_t i = 0; i < ls->n_ports; i++) {
if (ls->ports[i] == lsp) {
-return ls;
+*ls_p = ls;
+return NULL;
}
}
}
/* Can't happen because of the database schema */
-ctl_fatal("logical port %s is not part of any logical switch",
- lsp->name);
+return xasprintf("logical port %s is not part of any logical switch",
+ lsp->name);
}
static const char *
@@ -1027,7 +1031,11 @@ nbctl_lsp_add(struct ctl_context *ctx)
}
const struct nbrec_logical_switch *lsw;
-lsw = lsp_to_ls(ctx->idl, lsp);
+error = lsp_to_ls(ctx->idl, lsp, );
+if (error) {
+ctx->error = error;
+return;
+}
if (lsw != ls) {
char uuid_s[UUID_LEN + 1];
ctl_error(ctx, "%s: port already exists but in switch %s",
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 02/11] ovn-nbctl: Don't die in gc_by_name_or_uuid().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 0cd11e845..df35d11d6 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -3291,10 +3291,12 @@ lr_get_name(const struct nbrec_logical_router *lr, char
uuid_s[UUID_LEN + 1],
return uuid_s;
}
-static const struct nbrec_gateway_chassis *
-gc_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist)
+static char * OVS_WARN_UNUSED_RESULT
+gc_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+ const struct nbrec_gateway_chassis **gc_p)
{
const struct nbrec_gateway_chassis *gc = NULL;
+*gc_p = NULL;
struct uuid gc_uuid;
bool is_uuid = uuid_from_string(_uuid, id);
@@ -3311,11 +3313,12 @@ gc_by_name_or_uuid(struct ctl_context *ctx, const char
*id, bool must_exist)
}
if (!gc && must_exist) {
-ctl_fatal("%s: gateway chassis %s not found", id,
- is_uuid ? "UUID" : "name");
+return xasprintf("%s: gateway chassis %s not found", id,
+ is_uuid ? "UUID" : "name");
}
-return gc;
+*gc_p = gc;
+return NULL;
}
static void
@@ -3342,7 +3345,10 @@ nbctl_lrp_set_gateway_chassis(struct ctl_context *ctx)
gc_name = xasprintf("%s-%s", lrp_name, chassis_name);
const struct nbrec_gateway_chassis *existing_gc;
-existing_gc = gc_by_name_or_uuid(ctx, gc_name, false);
+error = gc_by_name_or_uuid(ctx, gc_name, false, _gc);
+if (error) {
+ctl_fatal("%s", error);
+}
if (existing_gc) {
nbrec_gateway_chassis_set_priority(existing_gc, priority);
free(gc_name);
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH 01/11] ovn-nbctl: Don't die in pg_by_name_or_uuid().
Let the caller handle the error. This prepares us for reporting errors
in daemon mode.
Also, extend the tests to cover this error path.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 23 ---
tests/ovn-nbctl.at| 5 +
2 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index d07524697..0cd11e845 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -609,10 +609,12 @@ lb_by_name_or_uuid(struct ctl_context *ctx, const char
*id, bool must_exist,
return NULL;
}
-static const struct nbrec_port_group *
-pg_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist)
+static char * OVS_WARN_UNUSED_RESULT
+pg_by_name_or_uuid(struct ctl_context *ctx, const char *id, bool must_exist,
+ const struct nbrec_port_group **pg_p)
{
const struct nbrec_port_group *pg = NULL;
+*pg_p = NULL;
struct uuid pg_uuid;
bool is_uuid = uuid_from_string(_uuid, id);
@@ -632,11 +634,12 @@ pg_by_name_or_uuid(struct ctl_context *ctx, const char
*id, bool must_exist)
}
if (!pg && must_exist) {
-ctl_fatal("%s: port group %s not found", id,
- is_uuid ? "UUID" : "name");
+return xasprintf("%s: port group %s not found", id,
+ is_uuid ? "UUID" : "name");
}
-return pg;
+*pg_p = pg;
+return NULL;
}
static void
@@ -1559,7 +1562,10 @@ acl_cmd_get_pg_or_ls(struct ctl_context *ctx,
char *error;
if (!opt_type) {
-*pg = pg_by_name_or_uuid(ctx, ctx->argv[1], false);
+error = pg_by_name_or_uuid(ctx, ctx->argv[1], false, pg);
+if (error) {
+return error;
+}
error = ls_by_name_or_uuid(ctx, ctx->argv[1], false, ls);
if (error) {
return error;
@@ -1574,7 +1580,10 @@ acl_cmd_get_pg_or_ls(struct ctl_context *ctx,
ctx->argv[1]);
}
} else if (!strcmp(opt_type, "port-group")) {
-*pg = pg_by_name_or_uuid(ctx, ctx->argv[1], true);
+error = pg_by_name_or_uuid(ctx, ctx->argv[1], true, pg);
+if (error) {
+return error;
+}
*ls = NULL;
} else if (!strcmp(opt_type, "switch")) {
error = ls_by_name_or_uuid(ctx, ctx->argv[1], true, ls);
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 73a61a4be..9f6a2783b 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -234,6 +234,11 @@ OVN_NBCTL_TEST_ACL([ls1], [--type=switch])
AT_CHECK([ovn-nbctl create port_group name=pg0], [0], [ignore])
OVN_NBCTL_TEST_ACL([pg0], [--type=port-group])
+dnl Test when port group doesn't exist
+AT_CHECK([ovn-nbctl --type=port-group acl-add pg1 to-lport 100 ip drop], [1],
[], [dnl
+ovn-nbctl: pg1: port group name not found
+])
+
dnl Test when same name exists in logical switches and portgroups
AT_CHECK([ovn-nbctl create port_group name=ls0], [0], [ignore])
AT_CHECK([ovn-nbctl acl-add ls0 to-lport 100 ip drop], [1], [], [stderr])
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v3 00/17] Daemon mode for ovn-nbctl
Hi Mark, On Mon, 16 Jul 2018 14:11:19 -0400 Mark Michelson wrote: > I've had a look through again and this series addresses the findings I > previously had, so in short: > > Acked-by: Mark Michelson > > I decided to do some testing to see what sort of performance gain we see > with this patch. I'm attaching four scripts that I used for testing. > > In all of the tests, I create a logical router and 159 logical switches > connected to the router. I then create 92 logical switch ports on each > logical switch (a total of 14628 ports). On alternating logical switch > port additions, I create an address set. Every set of two logical switch > ports gets added to the most recently created address set. For each > logical switch port, I also create two ACLs. Note that the script does > not perform any synchronization or port binding. The goal of the script > is to isolate the performance of ovn-nbctl at scale rather than as an > overall view of the performance of OVN. > > The scale.sh script is a straightforward version that uses individual > calls to ovn-nbctl at each step. The scale_batch.sh script batches > multiple operations into a single ovn-nbctl call. This way, the switch > port, address set, and ACLs are created in one ovn-nbctl invocation. I > ran these scripts about a month ago in a sandbox against OVS master. The > scale.sh script took ~13 hours to complete. The scale_batch.sh script > took ~3 hours to complete. > > I then modified each of these scripts to work with ovn-nbctl in daemon > mode. scale.sh was modified into scale-daemon.sh, and scale_batch.sh was > modified into scale_batch-daemon.sh. I applied the patch series to the > current OVS master and ran the daemon versions of the scripts in a > sandbox. scale-daemon.sh completed in 8 minutes 54 seconds. > scale_batch-daemon.sh completed in 3 minutes 52 seconds. In both cases, > this is a 99.9% decrease in the amount of time to complete. Thanks a lot for the review feedback and for doing proper benchmarking of these changes. Great to have a confirmation that there is the expected speed-up. Thanks, Jakub ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 17/17] tests: Add test for oneline-formatted output for ovn-nbctl.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 89daf631a..60b4d0c9c 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1403,3 +1403,24 @@ AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - oneline output]) +OVN_NBCTL_TEST_START + +AT_CHECK([ovn-nbctl ls-add ls0 -- ls-add ls1]) + +# Expect one line for one command. +AT_CHECK([ovn-nbctl --oneline ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +]) + +# Expect lines for two commands. +AT_CHECK([ovn-nbctl --oneline ls-list -- ls-list | uuidfilt], [0], [dnl +<0> (ls0)\n<1> (ls1) +<0> (ls0)\n<1> (ls1) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 16/17] tests: Add test for ovn-nbctl dry run mode.
Signed-off-by: Jakub Sitnicki --- tests/ovn-nbctl.at | 21 + 1 file changed, 21 insertions(+) diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 73a61a4be..89daf631a 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1382,3 +1382,24 @@ inactivity_probe: 3 OVN_NBCTL_TEST_STOP AT_CLEANUP + +dnl - + +AT_SETUP([ovn-nbctl - dry run mode]) +OVN_NBCTL_TEST_START + +# Check that dry run has no permanent effect. +AT_CHECK([ovn-nbctl --dry-run ls-add ls0 -- ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +]) + +# Check that dry-run mode is not sticky. +AT_CHECK([ovn-nbctl ls-add ls0]) +AT_CHECK([ovn-nbctl ls-list | uuidfilt], [0], [dnl +<0> (ls0) +]) + +OVN_NBCTL_TEST_STOP +AT_CLEANUP -- 2.14.4 ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 15/17] ovn-nbctl: Initial support for daemon mode.
Make ovn-nbctl act as a unixctl server if we were asked to detach. This
turns ovn-nbctl into a long-lived process that acts a proxy for
interacting with NB DB. The main difference to regular mode of ovn-nbctl
is that in the daemon mode, a local copy of database contents has to be
obtained only once.
Just two unixctl commands are supported 'run' and 'exit'. The former can
be used to run any ovn-nbctl command or a batch of them as so:
ovs-appctl -t ovn-nbctl run [OPTIONS] COMMAND [-- [OPTIONS] COMMAND] ...
Running commands that have not yet been converted to not use ctl_fatal()
will result in death of the daemon process. However, --monitor option
can be used to keep the daemon running.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.8.xml | 40 ++
ovn/utilities/ovn-nbctl.c | 308 ++
2 files changed, 322 insertions(+), 26 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index abba4ecdb..2cd2fab30 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -913,6 +913,43 @@
+Daemon Mode
+
+
+ If ovn-nbctl is invoked with the --detach
+ option (see Daemon Options, below), it runs in the
+ background as a daemon and accepts commands from ovs-appctl
+ (or another JSON-RPC client) indefinitely. The currently supported
+ commands are described below.
+
+
+
+
+
+
+
+
+run [options] command
+[arg...] [-- [options]
+command [arg...] ...]
+
+
+Instructs the daemon process to run one or more ovn-nbctl
+commands described above and reply with the results of running these
+commands. Accepts the --no-wait, --wait,
+--timeout, --dry-run, --oneline,
+and the options described under Table Formatting Options
+in addition to the the command-specific options.
+
+
+ exit
+ Causes ovn-nbctl to gracefully terminate.
+
+
+
+ Daemon mode is considered experimental.
+
+
Options
@@ -982,6 +1019,9 @@
+Daemon Options
+http://www.w3.org/2003/XInclude"/>
+
Logging options
http://www.w3.org/2003/XInclude"/>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index c83d03218..46e923cb9 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -20,6 +20,7 @@
#include
#include "command-line.h"
+#include "daemon.h"
#include "db-ctl-base.h"
#include "dirs.h"
#include "fatal-signal.h"
@@ -38,6 +39,7 @@
#include "table.h"
#include "timeval.h"
#include "timer.h"
+#include "unixctl.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -80,6 +82,13 @@ OVS_NO_RETURN static void nbctl_exit(int status);
/* --leader-only, --no-leader-only: Only accept the leader in a cluster. */
static int leader_only = true;
+/* --unixctl-path: Path to use for unixctl server, for "monitor" and "snoop"
+ commands. */
+static char *unixctl_path;
+
+static unixctl_cb_func server_cmd_exit;
+static unixctl_cb_func server_cmd_run;
+
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
@@ -98,15 +107,13 @@ static char * OVS_WARN_UNUSED_RESULT main_loop(const char
*args,
size_t n_commands,
struct ovsdb_idl *idl,
const struct timer *);
+static void server_loop(struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
{
struct ovsdb_idl *idl;
-struct ctl_command *commands;
struct shash local_options;
-size_t n_commands;
-char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -119,38 +126,55 @@ main(int argc, char *argv[])
char *args = process_escape_args(argv);
shash_init(_options);
parse_options(argc, argv, _options);
-commands = ctl_parse_commands(argc - optind, argv + optind, _options,
- _commands);
-VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG,
- "Called as %s", args);
-
-if (timeout) {
-time_alarm(timeout);
-}
+argc -= optind;
+argv += optind;
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-error = run_prerequisites(commands, n_commands, idl);
-if (error) {
-ctl_fatal("%s", error);
-}
-error = main_loop(args, commands, n_commands, idl, NULL);
-if (error) {
-ctl_fatal("%s", error);
+if (get_detach()) {
+if (argc != 0) {
+ct
[ovs-dev] [PATCH v3 14/17] ovn-nbctl: Extract a helper for appending command options.
Will be reused when parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 37 +
1 file changed, 25 insertions(+), 12 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index bac1c001d..c83d03218 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -286,6 +286,30 @@ build_short_options(const struct option *long_options)
return short_options;
}
+static struct option * OVS_WARN_UNUSED_RESULT
+append_command_options(const struct option *options, int opt_val)
+{
+struct option *o;
+size_t n_allocated;
+size_t n_existing;
+int i;
+
+for (i = 0; options[i].name; i++) {
+;
+}
+n_allocated = i + 1;
+n_existing = i;
+
+/* We want to parse both global and command-specific options here, but
+ * getopt_long() isn't too convenient for the job. We copy our global
+ * options into a dynamic array, then append all of the command-specific
+ * options. */
+o = xmemdup(options, n_allocated * sizeof *options);
+ctl_add_cmd_options(, _existing, _allocated, opt_val);
+
+return o;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -307,22 +331,11 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
char *short_options;
-
struct option *options;
-size_t allocated_options;
-size_t n_options;
size_t i;
short_options = build_short_options(global_long_options);
-
-/* We want to parse both global and command-specific options here, but
- * getopt_long() isn't too convenient for the job. We copy our global
- * options into a dynamic array, then append all of the command-specific
- * options. */
-options = xmemdup(global_long_options, sizeof global_long_options);
-allocated_options = ARRAY_SIZE(global_long_options);
-n_options = n_global_long_options;
-ctl_add_cmd_options(, _options, _options, OPT_LOCAL);
+options = append_command_options(global_long_options, OPT_LOCAL);
for (;;) {
int idx;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 13/17] ovn-nbctl: Extract a helper for building short options string.
Will be reused for parsing options in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 72c78795c..bac1c001d 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -274,6 +274,18 @@ handle_main_loop_option(int opt, const char *arg, bool
*handled)
return NULL;
}
+static char * OVS_WARN_UNUSED_RESULT
+build_short_options(const struct option *long_options)
+{
+char *tmp, *short_options;
+
+tmp = ovs_cmdl_long_options_to_short_options(long_options);
+short_options = xasprintf("+%s", tmp);
+free(tmp);
+
+return short_options;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
@@ -294,16 +306,14 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
{NULL, 0, NULL, 0},
};
const int n_global_long_options = ARRAY_SIZE(global_long_options) - 1;
-char *tmp, *short_options;
+char *short_options;
struct option *options;
size_t allocated_options;
size_t n_options;
size_t i;
-tmp = ovs_cmdl_long_options_to_short_options(global_long_options);
-short_options = xasprintf("+%s", tmp);
-free(tmp);
+short_options = build_short_options(global_long_options);
/* We want to parse both global and command-specific options here, but
* getopt_long() isn't too convenient for the job. We copy our global
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 12/17] ovn-nbctl: Extract handling of options that affect main loop.
Provide a handler for options that change how the main loop behaves.
This will allow code reuse for option parsing in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 136 --
1 file changed, 84 insertions(+), 52 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 3dd24d193..72c78795c 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -200,38 +200,93 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
return NULL;
}
+/* All options that affect the main loop and are not external. */
+#define MAIN_LOOP_OPTION_ENUMS \
+OPT_NO_WAIT,\
+OPT_WAIT, \
+OPT_DRY_RUN,\
+OPT_ONELINE
+
+#define MAIN_LOOP_LONG_OPTIONS \
+{"no-wait", no_argument, NULL, OPT_NO_WAIT}, \
+{"wait", required_argument, NULL, OPT_WAIT}, \
+{"dry-run", no_argument, NULL, OPT_DRY_RUN}, \
+{"oneline", no_argument, NULL, OPT_ONELINE}, \
+{"timeout", required_argument, NULL, 't'}
+
+enum {
+OPT_DB = UCHAR_MAX + 1,
+OPT_NO_SYSLOG,
+OPT_LOCAL,
+OPT_COMMANDS,
+OPT_OPTIONS,
+OPT_BOOTSTRAP_CA_CERT,
+MAIN_LOOP_OPTION_ENUMS,
+VLOG_OPTION_ENUMS,
+TABLE_OPTION_ENUMS,
+SSL_OPTION_ENUMS,
+};
+
+static char * OVS_WARN_UNUSED_RESULT
+handle_main_loop_option(int opt, const char *arg, bool *handled)
+{
+ovs_assert(handled);
+*handled = true;
+
+switch (opt) {
+case OPT_ONELINE:
+oneline = true;
+break;
+
+case OPT_NO_WAIT:
+wait_type = NBCTL_WAIT_NONE;
+break;
+
+case OPT_WAIT:
+if (!strcmp(arg, "none")) {
+wait_type = NBCTL_WAIT_NONE;
+} else if (!strcmp(arg, "sb")) {
+wait_type = NBCTL_WAIT_SB;
+} else if (!strcmp(arg, "hv")) {
+wait_type = NBCTL_WAIT_HV;
+} else {
+return xstrdup("argument to --wait must be "
+ "\"none\", \"sb\", or \"hv\"");
+}
+break;
+
+case OPT_DRY_RUN:
+dry_run = true;
+break;
+
+case 't':
+timeout = strtoul(arg, NULL, 10);
+if (timeout < 0) {
+return xasprintf("value %s on -t or --timeout is invalid", arg);
+}
+break;
+
+default:
+*handled = false;
+break;
+}
+
+return NULL;
+}
+
static void
parse_options(int argc, char *argv[], struct shash *local_options)
{
-enum {
-OPT_DB = UCHAR_MAX + 1,
-OPT_NO_SYSLOG,
-OPT_NO_WAIT,
-OPT_WAIT,
-OPT_DRY_RUN,
-OPT_ONELINE,
-OPT_LOCAL,
-OPT_COMMANDS,
-OPT_OPTIONS,
-OPT_BOOTSTRAP_CA_CERT,
-VLOG_OPTION_ENUMS,
-TABLE_OPTION_ENUMS,
-SSL_OPTION_ENUMS,
-};
static const struct option global_long_options[] = {
{"db", required_argument, NULL, OPT_DB},
{"no-syslog", no_argument, NULL, OPT_NO_SYSLOG},
-{"no-wait", no_argument, NULL, OPT_NO_WAIT},
-{"wait", required_argument, NULL, OPT_WAIT},
-{"dry-run", no_argument, NULL, OPT_DRY_RUN},
-{"oneline", no_argument, NULL, OPT_ONELINE},
-{"timeout", required_argument, NULL, 't'},
{"help", no_argument, NULL, 'h'},
{"commands", no_argument, NULL, OPT_COMMANDS},
{"options", no_argument, NULL, OPT_OPTIONS},
{"leader-only", no_argument, _only, true},
{"no-leader-only", no_argument, _only, false},
{"version", no_argument, NULL, 'V'},
+MAIN_LOOP_LONG_OPTIONS,
VLOG_LONG_OPTIONS,
STREAM_SSL_LONG_OPTIONS,
{"bootstrap-ca-cert", required_argument, NULL, OPT_BOOTSTRAP_CA_CERT},
@@ -268,40 +323,24 @@ parse_options(int argc, char *argv[], struct shash
*local_options)
break;
}
+bool handled;
+char *error = handle_main_loop_option(c, optarg, );
+if (error) {
+ctl_fatal("%s", error);
+}
+if (handled) {
+continue;
+}
+
switch (c) {
case OPT_DB:
db = optarg;
break;
-case OPT_ONELINE:
-oneline = true;
-break;
-
case OPT_NO_SYSLOG:
vlog_set_levels(_module, VLF_SYSLOG, VLL_WARN);
break;
-case OPT_NO_WAIT:
-wait_type = NBCTL_WAIT_NONE;
-break;
-
-case OPT_WAIT:
-if (!strcmp(optarg, "no
[ovs-dev] [PATCH v3 11/17] ovn-nbctl: Extract helper for printing oneline output.
This will allow us to direct oneline-formatted output to other sinks
than stdout if needed. Preparatory work for daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 53 ++-
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 7f83abc40..3dd24d193 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -4162,6 +4162,39 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
return NULL;
}
+static void
+oneline_format(struct ds *lines, struct ds *s)
+{
+size_t j;
+
+ds_chomp(lines, '\n');
+for (j = 0; j < lines->length; j++) {
+int ch = lines->string[j];
+switch (ch) {
+case '\n':
+ds_put_cstr(s, "\\n");
+break;
+
+case '\\':
+ds_put_cstr(s, "");
+break;
+
+default:
+ds_put_char(s, ch);
+}
+}
+ds_put_char(s, '\n');
+}
+
+static void
+oneline_print(struct ds *lines)
+{
+struct ds s = DS_EMPTY_INITIALIZER;
+oneline_format(lines, );
+fputs(ds_cstr(), stdout);
+ds_destroy();
+}
+
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
@@ -4297,25 +4330,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
if (c->table) {
table_print(c->table, _style);
} else if (oneline) {
-size_t j;
-
-ds_chomp(ds, '\n');
-for (j = 0; j < ds->length; j++) {
-int ch = ds->string[j];
-switch (ch) {
-case '\n':
-fputs("\\n", stdout);
-break;
-
-case '\\':
-fputs("", stdout);
-break;
-
-default:
-putchar(ch);
-}
-}
-putchar('\n');
+oneline_print(ds);
} else {
fputs(ds_cstr(ds), stdout);
}
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 10/17] ovn-nbctl: Introduce a poll_timer based wait timeout.
Extend the main loop and the command runner so that the caller can
specify a timeout for poll_block(). This will allow us to break out of
the main loop when waiting on IDL, like in the blocked '--wait=sb/hv
sync' case.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 154e7799a..7f83abc40 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -37,6 +37,7 @@
#include "svec.h"
#include "table.h"
#include "timeval.h"
+#include "timer.h"
#include "util.h"
#include "openvswitch/vlog.h"
@@ -87,13 +88,16 @@ static char * OVS_WARN_UNUSED_RESULT
run_prerequisites(struct ctl_command[],
struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+ struct ovsdb_idl *,
+ const struct timer *,
+ bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
struct ctl_command *commands,
size_t n_commands,
- struct ovsdb_idl *idl);
+ struct ovsdb_idl *idl,
+ const struct timer *);
int
main(int argc, char *argv[])
@@ -132,7 +136,7 @@ main(int argc, char *argv[])
ctl_fatal("%s", error);
}
-error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl, NULL);
if (error) {
ctl_fatal("%s", error);
}
@@ -153,7 +157,7 @@ main(int argc, char *argv[])
static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout)
{
unsigned int seqno;
@@ -177,7 +181,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-char *error = do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl,
+ wait_timeout, );
if (error) {
return error;
}
@@ -4159,7 +4164,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl, bool *retry)
+ struct ovsdb_idl *idl, const struct timer *wait_timeout, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4286,8 +4291,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
OVS_NOT_REACHED();
}
-ovsdb_symbol_table_destroy(symtab);
-
for (c = commands; c < [n_commands]; c++) {
struct ds *ds = >output;
@@ -4331,11 +4334,19 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
}
}
ovsdb_idl_wait(idl);
+if (wait_timeout) {
+timer_wait(wait_timeout);
+}
poll_block();
+if (wait_timeout && timer_expired(wait_timeout)) {
+error = xstrdup("timeout expired");
+goto out_error;
+}
}
done: ;
}
+ovsdb_symbol_table_destroy(symtab);
ovsdb_idl_txn_destroy(txn);
*retry = false;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 09/17] ovn-nbctl: Propagate errors from prerequisites runner.
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the prerequisites runner in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 21 +++--
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 747aa63b6..154e7799a 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -82,8 +82,9 @@ static int leader_only = true;
static void nbctl_cmd_init(void);
OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
-static void run_prerequisites(struct ctl_command[], size_t n_commands,
- struct ovsdb_idl *);
+static char * OVS_WARN_UNUSED_RESULT run_prerequisites(struct ctl_command[],
+ size_t n_commands,
+ struct ovsdb_idl *);
static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
struct ctl_command *, size_t n,
struct ovsdb_idl *, bool *retry);
@@ -101,6 +102,7 @@ main(int argc, char *argv[])
struct ctl_command *commands;
struct shash local_options;
size_t n_commands;
+char *error;
set_program_name(argv[0]);
fatal_ignore_sigpipe();
@@ -125,9 +127,12 @@ main(int argc, char *argv[])
/* Initialize IDL. */
idl = the_idl = ovsdb_idl_create(db, _idl_class, true, false);
ovsdb_idl_set_leader_only(idl, leader_only);
-run_prerequisites(commands, n_commands, idl);
+error = run_prerequisites(commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
-char *error = main_loop(args, commands, n_commands, idl);
+error = main_loop(args, commands, n_commands, idl);
if (error) {
ctl_fatal("%s", error);
}
@@ -4117,7 +4122,7 @@ static const struct ctl_table_class
tables[NBREC_N_TABLES] = {
[NBREC_TABLE_ACL].row_ids[0] = {_acl_col_name, NULL, NULL},
};
-static void
+static char *
run_prerequisites(struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -4138,7 +4143,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ctl_context_init(, c, idl, NULL, NULL, NULL);
(c->syntax->prerequisites)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+char *error = xstrdup(ctx.error);
+ctl_context_done(, c);
+return error;
}
ctl_context_done(, c);
@@ -4146,6 +4153,8 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
ovs_assert(!c->table);
}
}
+
+return NULL;
}
static char *
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 08/17] ovn-nbctl: Propagate errors from the main loop.
Let the caller handle the errors instead of reporting it and
terminating. Prepare for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 6e136e1d0..747aa63b6 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -89,8 +89,10 @@ static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char
*args,
struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
-static void main_loop(const char *args, struct ctl_command *commands,
- size_t n_commands, struct ovsdb_idl *idl);
+static char * OVS_WARN_UNUSED_RESULT main_loop(const char *args,
+ struct ctl_command *commands,
+ size_t n_commands,
+ struct ovsdb_idl *idl);
int
main(int argc, char *argv[])
@@ -125,7 +127,10 @@ main(int argc, char *argv[])
ovsdb_idl_set_leader_only(idl, leader_only);
run_prerequisites(commands, n_commands, idl);
-main_loop(args, commands, n_commands, idl);
+char *error = main_loop(args, commands, n_commands, idl);
+if (error) {
+ctl_fatal("%s", error);
+}
ovsdb_idl_destroy(idl);
idl = the_idl = NULL;
@@ -141,7 +146,7 @@ main(int argc, char *argv[])
exit(EXIT_SUCCESS);
}
-static void
+static char *
main_loop(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl)
{
@@ -169,10 +174,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
bool retry;
char *error = do_nbctl(args, commands, n_commands, idl, );
if (error) {
-ctl_fatal("%s", error);
+return error;
}
if (!retry) {
-return;
+return NULL;
}
}
@@ -181,6 +186,8 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
poll_block();
}
}
+
+return NULL;
}
static void
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 07/17] ovn-nbctl: Propagate the error from do_nbctl().
Instead of terminating the process, return the error to the caller.
This will allow us to reuse the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 46 --
1 file changed, 32 insertions(+), 14 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 360b25a89..6e136e1d0 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,9 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static void do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *, bool *retry);
+static char * OVS_WARN_UNUSED_RESULT do_nbctl(const char *args,
+ struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -166,7 +167,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
seqno = ovsdb_idl_get_seqno(idl);
bool retry;
-do_nbctl(args, commands, n_commands, idl, );
+char *error = do_nbctl(args, commands, n_commands, idl, );
+if (error) {
+ctl_fatal("%s", error);
+}
if (!retry) {
return;
}
@@ -4137,7 +4141,7 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static void
+static char *
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
struct ovsdb_idl *idl, bool *retry)
{
@@ -4148,6 +4152,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct ctl_command *c;
struct shash_node *node;
int64_t next_cfg = 0;
+char *error = NULL;
ovs_assert(retry);
@@ -4181,7 +4186,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
(c->syntax->run)();
}
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done_command(, c);
@@ -4195,9 +4202,10 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
SHASH_FOR_EACH (node, >sh) {
struct ovsdb_symbol *symbol = node->data;
if (!symbol->created) {
-ctl_fatal("row id \"%s\" is referenced but never created (e.g. "
- "with \"-- --id=%s create ...\")",
- node->name, node->name);
+error = xasprintf("row id \"%s\" is referenced but never created "
+ "(e.g. with \"-- --id=%s create ...\")",
+ node->name, node->name);
+goto out_error;
}
if (!symbol->strong_ref) {
if (!symbol->weak_ref) {
@@ -4222,7 +4230,9 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ctl_context_init(, c, idl, txn, symtab, NULL);
(c->syntax->postprocess)();
if (ctx.error) {
-ctl_fatal("%s", ctx.error);
+error = xstrdup(ctx.error);
+ctl_context_done(, c);
+goto out_error;
}
ctl_context_done(, c);
}
@@ -4236,7 +4246,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
case TXN_ABORTED:
/* Should not happen--we never call ovsdb_idl_txn_abort(). */
-ctl_fatal("transaction aborted");
+error = xstrdup("transaction aborted");
+goto out_error;
case TXN_UNCHANGED:
case TXN_SUCCESS:
@@ -4246,11 +4257,14 @@ do_nbctl(const char *args, struct ctl_command
*commands, size_t n_commands,
goto try_again;
case TXN_ERROR:
-ctl_fatal("transaction error: %s", ovsdb_idl_txn_get_error(txn));
+error = xasprintf("transaction error: %s",
+ ovsdb_idl_txn_get_error(txn));
+goto out_error;
case TXN_NOT_LOCKED:
/* Should not happen--we never call ovsdb_idl_set_lock(). */
-ctl_fatal("database not locked");
+error = xstrdup("database not locked");
+goto out_error;
default:
OVS_NOT_REACHED();
@@ -
[ovs-dev] [PATCH v3 06/17] ovn-nbctl: Signal need to try again via an output param.
Introduce an output parameter for the flag that signals need to retry
running the command. This leaves the return value for error reporting.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index a700695fe..360b25a89 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -84,8 +84,8 @@ OVS_NO_RETURN static void usage(void);
static void parse_options(int argc, char *argv[], struct shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
-static bool do_nbctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+static void do_nbctl(const char *args, struct ctl_command *, size_t n,
+ struct ovsdb_idl *, bool *retry);
static const struct nbrec_dhcp_options *dhcp_options_get(
struct ctl_context *ctx, const char *id, bool must_exist);
static void main_loop(const char *args, struct ctl_command *commands,
@@ -164,7 +164,10 @@ main_loop(const char *args, struct ctl_command *commands,
size_t n_commands,
if (seqno != ovsdb_idl_get_seqno(idl)) {
seqno = ovsdb_idl_get_seqno(idl);
-if (do_nbctl(args, commands, n_commands, idl)) {
+
+bool retry;
+do_nbctl(args, commands, n_commands, idl, );
+if (!retry) {
return;
}
}
@@ -4134,9 +4137,9 @@ run_prerequisites(struct ctl_command *commands, size_t
n_commands,
}
}
-static bool
+static void
do_nbctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, bool *retry)
{
struct ovsdb_idl_txn *txn;
enum ovsdb_idl_txn_status status;
@@ -4146,6 +4149,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
struct shash_node *node;
int64_t next_cfg = 0;
+ovs_assert(retry);
+
txn = the_idl_txn = ovsdb_idl_txn_create(idl);
if (dry_run) {
ovsdb_idl_txn_set_dry_run(txn);
@@ -4303,7 +4308,8 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ovsdb_idl_txn_destroy(txn);
-return true;
+*retry = false;
+return;
try_again:
/* Our transaction needs to be rerun, or a prerequisite was not met. Free
@@ -4318,7 +4324,7 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
table_destroy(c->table);
free(c->table);
}
-return false;
+*retry = true;
}
/* Frees the current transaction and the underlying IDL and then calls
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH v3 05/17] ovn-nbctl: Pull up releasing IDL from do_nbctl().
Destroy IDL resources in the routine where we allocated them.
Preparatory work for reusing the main loop in daemon mode.
Signed-off-by: Jakub Sitnicki
---
ovn/utilities/ovn-nbctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 51527741b..a700695fe 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -126,6 +126,9 @@ main(int argc, char *argv[])
main_loop(args, commands, n_commands, idl);
+ovsdb_idl_destroy(idl);
+idl = the_idl = NULL;
+
for (struct ctl_command *c = commands; c < [n_commands]; c++) {
ds_destroy(>output);
table_destroy(c->table);
@@ -4299,7 +4302,6 @@ do_nbctl(const char *args, struct ctl_command *commands,
size_t n_commands,
}
ovsdb_idl_txn_destroy(txn);
-ovsdb_idl_destroy(idl);
return true;
--
2.14.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
