[ovs-dev] [PATCH 2/3] vswitchd: Add error column to Interface table to store error condition
Thomas Graf
tgraf at redhat.com
Thu Apr 10 10:50:10 UTC 2014
Store the error condition of a failed port configuration in a new
column 'error' in the Interface table.
Example:
$ ovs-vsctl add-port br0 test -- \
set Interface test type=vxlan options:unknown=1
ovs-vsctl: Error detected while setting up 'test'. [...]
$ ovs-vsctl list Interface test | grep error
error : "test: could not set configuration (Invalid argument)"
Fixing the error will clear the error column:
$ ovs-vsctl set Interface test options:remote_ip=1.1.1.1
$ ovs-vsctl list Interface test | grep error
error : []
$
For now, the high level error messages when opening and configuring
the netdev are used. Further patches can extend passing the error
pointer into the individual netdev implementations to allow for more
fine grained error messages to be stored.
Signed-off-by: Thomas Graf <tgraf at redhat.com>
---
lib/netdev.c | 10 +++++-----
lib/netdev.h | 2 +-
utilities/ovs-dpctl.c | 4 ++--
vswitchd/bridge.c | 29 ++++++++++++++++++-----------
vswitchd/vswitch.ovsschema | 8 +++++---
vswitchd/vswitch.xml | 5 +++++
6 files changed, 36 insertions(+), 22 deletions(-)
diff --git a/lib/netdev.c b/lib/netdev.c
index 4ec1d7d..7e9c712 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -406,7 +406,7 @@ netdev_ref(const struct netdev *netdev_)
/* Reconfigures the device 'netdev' with 'args'. 'args' may be empty
* or NULL if none are needed. */
int
-netdev_set_config(struct netdev *netdev, const struct smap *args)
+netdev_set_config(struct netdev *netdev, const struct smap *args, char **errp)
OVS_EXCLUDED(netdev_mutex)
{
if (netdev->netdev_class->set_config) {
@@ -416,13 +416,13 @@ netdev_set_config(struct netdev *netdev, const struct smap *args)
error = netdev->netdev_class->set_config(netdev,
args ? args : &no_args);
if (error) {
- VLOG_WARN("%s: could not set configuration (%s)",
- netdev_get_name(netdev), ovs_strerror(error));
+ VLOG_WARN_BUF(errp, "%s: could not set configuration (%s)",
+ netdev_get_name(netdev), ovs_strerror(error));
}
return error;
} else if (args && !smap_is_empty(args)) {
- VLOG_WARN("%s: arguments provided to device that is not configurable",
- netdev_get_name(netdev));
+ VLOG_WARN_BUF(errp, "%s: arguments provided to device that is not configurable",
+ netdev_get_name(netdev));
}
return 0;
}
diff --git a/lib/netdev.h b/lib/netdev.h
index 432f327..532ee63 100644
--- a/lib/netdev.h
+++ b/lib/netdev.h
@@ -146,7 +146,7 @@ void netdev_close(struct netdev *);
void netdev_parse_name(const char *netdev_name, char **name, char **type);
/* Options. */
-int netdev_set_config(struct netdev *, const struct smap *args);
+int netdev_set_config(struct netdev *, const struct smap *args, char **errp);
int netdev_get_config(const struct netdev *, struct smap *);
const struct netdev_tunnel_config *
netdev_get_tunnel_config(const struct netdev *);
diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c
index ccc55b5..9e879f8 100644
--- a/utilities/ovs-dpctl.c
+++ b/utilities/ovs-dpctl.c
@@ -349,7 +349,7 @@ dpctl_add_if(int argc OVS_UNUSED, char *argv[])
goto next;
}
- error = netdev_set_config(netdev, &args);
+ error = netdev_set_config(netdev, &args, NULL);
if (error) {
goto next;
}
@@ -456,7 +456,7 @@ dpctl_set_if(int argc, char *argv[])
}
/* Update configuration. */
- error = netdev_set_config(netdev, &args);
+ error = netdev_set_config(netdev, &args, NULL);
smap_destroy(&args);
if (error) {
goto next;
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f46d002..fdb7ab6 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -258,7 +258,7 @@ static struct iface *iface_from_ofp_port(const struct bridge *,
ofp_port_t ofp_port);
static void iface_set_mac(const struct bridge *, const struct port *, struct iface *);
static void iface_set_ofport(const struct ovsrec_interface *, ofp_port_t ofport);
-static void iface_clear_db_record(const struct ovsrec_interface *if_cfg);
+static void iface_clear_db_record(const struct ovsrec_interface *if_cfg, char *errp);
static void iface_configure_qos(struct iface *, const struct ovsrec_qos *);
static void iface_configure_cfm(struct iface *);
static void iface_refresh_cfm_stats(struct iface *);
@@ -379,6 +379,7 @@ bridge_init(const char *remote)
ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_cfm_remote_opstate);
ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_bfd_status);
ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_lacp_current);
+ ovsdb_idl_omit_alert(idl, &ovsrec_interface_col_error);
ovsdb_idl_omit(idl, &ovsrec_interface_col_external_ids);
ovsdb_idl_omit_alert(idl, &ovsrec_controller_col_is_connected);
@@ -587,6 +588,8 @@ bridge_reconfigure(const struct ovsrec_open_vswitch *ovs_cfg)
LIST_FOR_EACH (iface, port_elem, &port->ifaces) {
iface_set_ofport(iface->cfg, iface->ofp_port);
+ /* Clear eventual previous errors */
+ ovsrec_interface_set_error(iface->cfg, NULL);
iface_configure_cfm(iface);
iface_configure_qos(iface, port->cfg->qos);
iface_set_mac(br, port, iface);
@@ -696,7 +699,7 @@ bridge_delete_or_reconfigure_ports(struct bridge *br)
}
if (strcmp(ofproto_port.type, iface->type)
- || netdev_set_config(iface->netdev, &iface->cfg->options)) {
+ || netdev_set_config(iface->netdev, &iface->cfg->options, NULL)) {
/* The interface is the wrong type or can't be configured.
* Delete it. */
goto delete;
@@ -1399,9 +1402,9 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
* Returns 0 if successful, otherwise a positive errno value. */
static int
iface_set_netdev_config(const struct ovsrec_interface *iface_cfg,
- struct netdev *netdev)
+ struct netdev *netdev, char **errp)
{
- return netdev_set_config(netdev, &iface_cfg->options);
+ return netdev_set_config(netdev, &iface_cfg->options, errp);
}
/* Opens a network device for 'if_cfg' and configures it. Adds the network
@@ -1413,7 +1416,8 @@ static int
iface_do_create(const struct bridge *br,
const struct ovsrec_interface *iface_cfg,
const struct ovsrec_port *port_cfg,
- ofp_port_t *ofp_portp, struct netdev **netdevp)
+ ofp_port_t *ofp_portp, struct netdev **netdevp,
+ char **errp)
{
struct netdev *netdev = NULL;
int error;
@@ -1428,12 +1432,12 @@ iface_do_create(const struct bridge *br,
error = netdev_open(iface_cfg->name,
iface_get_type(iface_cfg, br->cfg), &netdev);
if (error) {
- VLOG_WARN("could not open network device %s (%s)",
- iface_cfg->name, ovs_strerror(error));
+ VLOG_WARN_BUF(errp, "could not open network device %s (%s)",
+ iface_cfg->name, ovs_strerror(error));
goto error;
}
- error = iface_set_netdev_config(iface_cfg, netdev);
+ error = iface_set_netdev_config(iface_cfg, netdev, errp);
if (error) {
goto error;
}
@@ -1474,13 +1478,15 @@ iface_create(struct bridge *br, const struct ovsrec_interface *iface_cfg,
struct iface *iface;
ofp_port_t ofp_port;
struct port *port;
+ char *errp = NULL;
int error;
/* Do the bits that can fail up front. */
ovs_assert(!iface_lookup(br, iface_cfg->name));
- error = iface_do_create(br, iface_cfg, port_cfg, &ofp_port, &netdev);
+ error = iface_do_create(br, iface_cfg, port_cfg, &ofp_port, &netdev, &errp);
if (error) {
- iface_clear_db_record(iface_cfg);
+ iface_clear_db_record(iface_cfg, errp);
+ free(errp);
return false;
}
@@ -3559,10 +3565,11 @@ iface_set_ofport(const struct ovsrec_interface *if_cfg, ofp_port_t ofport)
* This is appropriate when 'if_cfg''s interface cannot be created or is
* otherwise invalid. */
static void
-iface_clear_db_record(const struct ovsrec_interface *if_cfg)
+iface_clear_db_record(const struct ovsrec_interface *if_cfg, char *errp)
{
if (!ovsdb_idl_row_is_synthetic(&if_cfg->header_)) {
iface_set_ofport(if_cfg, OFPP_NONE);
+ ovsrec_interface_set_error(if_cfg, errp);
ovsrec_interface_set_status(if_cfg, NULL);
ovsrec_interface_set_admin_state(if_cfg, NULL);
ovsrec_interface_set_duplex(if_cfg, NULL);
diff --git a/vswitchd/vswitch.ovsschema b/vswitchd/vswitch.ovsschema
index 3fb45d1..4c46b20 100644
--- a/vswitchd/vswitch.ovsschema
+++ b/vswitchd/vswitch.ovsschema
@@ -1,6 +1,6 @@
{"name": "Open_vSwitch",
- "version": "7.5.0",
- "cksum": "1448369194 20560",
+ "version": "7.6.0",
+ "cksum": "19534524 20635",
"tables": {
"Open_vSwitch": {
"columns": {
@@ -287,7 +287,9 @@
"ephemeral": true},
"mtu": {
"type": {"key": "integer", "min": 0, "max": 1},
- "ephemeral": true}},
+ "ephemeral": true},
+ "error": {
+ "type": {"key": "string", "min": 0, "max": 1}}},
"indexes": [["name"]]},
"Flow_Table": {
"columns": {
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 78594e7..33f9904 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -1279,6 +1279,11 @@
address.</p>
</column>
+ <column name="error">
+ Error description in human readable form if the configuration of the
+ port failed. Empty if port has been configured successfully.
+ </column>
+
<group title="OpenFlow Port Number">
<p>
When a client adds a new interface, Open vSwitch chooses an OpenFlow
--
1.8.3.1
More information about the dev
mailing list