I realized that this bug existed only a few weeks ago, but it has been
there since the earliest ovsdb-server code.
Signed-off-by: Ben Pfaff <b...@nicira.com>
---
ovsdb/jsonrpc-server.c | 26 ++++++++++++++------------
ovsdb/ovsdb-server.1.in | 29 ++++++++++++++++++++++++++++-
tests/ovsdb-server.at | 2 +-
3 files changed, 43 insertions(+), 14 deletions(-)
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 2ba5ddd..95b4995 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -78,8 +78,9 @@ static void ovsdb_jsonrpc_trigger_complete_done(
struct ovsdb_jsonrpc_session *);
/* Monitors. */
-static struct json *ovsdb_jsonrpc_monitor_create(
- struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params);
+static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_create(
+ struct ovsdb_jsonrpc_session *, struct ovsdb *, struct json *params,
+ const struct json *request_id);
static struct jsonrpc_msg *ovsdb_jsonrpc_monitor_cancel(
struct ovsdb_jsonrpc_session *,
struct json_array *params,
@@ -674,7 +675,7 @@ ovsdb_jsonrpc_lookup_db(const struct ovsdb_jsonrpc_session
*s,
return db;
error:
- *replyp = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+ *replyp = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return NULL;
}
@@ -752,7 +753,7 @@ ovsdb_jsonrpc_session_lock(struct ovsdb_jsonrpc_session *s,
return jsonrpc_create_reply(result, request->id);
error:
- reply = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+ reply = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return reply;
}
@@ -812,7 +813,7 @@ ovsdb_jsonrpc_session_unlock(struct ovsdb_jsonrpc_session
*s,
return jsonrpc_create_reply(json_object_create(), request->id);
error:
- reply = jsonrpc_create_reply(ovsdb_error_to_json(error), request->id);
+ reply = jsonrpc_create_error(ovsdb_error_to_json(error), request->id);
ovsdb_error_destroy(error);
return reply;
}
@@ -842,9 +843,8 @@ ovsdb_jsonrpc_session_got_request(struct
ovsdb_jsonrpc_session *s,
} else if (!strcmp(request->method, "monitor")) {
struct ovsdb *db = ovsdb_jsonrpc_lookup_db(s, request, &reply);
if (!reply) {
- reply = jsonrpc_create_reply(
- ovsdb_jsonrpc_monitor_create(s, db, request->params),
- request->id);
+ reply = ovsdb_jsonrpc_monitor_create(s, db, request->params,
+ request->id);
}
} else if (!strcmp(request->method, "monitor_cancel")) {
reply = ovsdb_jsonrpc_monitor_cancel(s, json_array(request->params),
@@ -1221,9 +1221,10 @@ ovsdb_jsonrpc_parse_monitor_request(struct
ovsdb_jsonrpc_monitor_table *mt,
return NULL;
}
-static struct json *
+static struct jsonrpc_msg *
ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session *s, struct ovsdb *db,
- struct json *params)
+ struct json *params,
+ const struct json *request_id)
{
struct ovsdb_jsonrpc_monitor *m = NULL;
struct json *monitor_id, *monitor_requests;
@@ -1310,7 +1311,8 @@ ovsdb_jsonrpc_monitor_create(struct ovsdb_jsonrpc_session
*s, struct ovsdb *db,
}
}
- return ovsdb_jsonrpc_monitor_get_initial(m);
+ return jsonrpc_create_reply(ovsdb_jsonrpc_monitor_get_initial(m),
+ request_id);
error:
if (m) {
@@ -1319,7 +1321,7 @@ error:
json = ovsdb_error_to_json(error);
ovsdb_error_destroy(error);
- return json;
+ return jsonrpc_create_error(json, request_id);
}
static struct jsonrpc_msg *
diff --git a/ovsdb/ovsdb-server.1.in b/ovsdb/ovsdb-server.1.in
index 0fafc49..e33d718 100644
--- a/ovsdb/ovsdb-server.1.in
+++ b/ovsdb/ovsdb-server.1.in
@@ -251,7 +251,34 @@ vSwitch 2.4 and later extend <condition> to allow the use
of \fB<\fR,
of 0 or 1 integer'' and ``set of 0 or 1 real''. These conditions
evaluate to false when the column is empty, and otherwise as described
in RFC 7047 for integer and real types.
-
+.
+.SH "BUGS"
+.
+In Open vSwitch before version 2.4, when \fBovsdb\-server\fR sent
+JSON-RPC error responses to some requests, it incorrectly formulated
+them with the \fBresult\fR and \fBerror\fR swapped, so that the
+response appeared to indicate success (with a nonsensical result)
+rather than an error. The requests that suffered from this problem
+were:
+.
+.IP \fBtransact\fR
+.IQ \fBget_schema\fR
+Only if the request names a nonexistent database.
+.IP \fBmonitor\fR
+.IQ \fBlock\fR
+.IQ \fBunlock\fR
+In all error cases.
+.
+.PP
+Of these cases, the only error that a well-written application is
+likely to encounter in practice is \fBmonitor\fR of tables or columns
+that do not exist, in an situation where the application has been
+upgraded but the old database schema is still temporarily in use. To
+handle this situation gracefully, we recommend that clients should
+treat a \fBmonitor\fR response with a \fBresult\fR that contains an
+\fBerror\fR key-value pair as an error (assuming that the database
+being monitored does not contain a table named \fBerror\fR).
+.
.SH "SEE ALSO"
.
.BR ovsdb\-tool (1).
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index 35f0d11..c9ce4b1 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -163,7 +163,7 @@ ordinals
], [ignore], [test ! -e pid || kill `cat pid`])
AT_CHECK(
[[ovstest test-jsonrpc request unix:socket get_schema [\"nonexistent\"]]],
[0],
- [[{"error":null,"id":0,"result":{"details":"get_schema request specifies
unknown database nonexistent","error":"unknown
database","syntax":"[\"nonexistent\"]"}}
+ [[{"error":{"details":"get_schema request specifies unknown database
nonexistent","error":"unknown
database","syntax":"[\"nonexistent\"]"},"id":0,"result":null}
]], [], [test ! -e pid || kill `cat pid`])
OVSDB_SERVER_SHUTDOWN
AT_CLEANUP
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
