[ovs-dev] [PATCH 1/2] jsonrpc: Don't swallow errors in jsonrpc_transact_block().

[Ethan Jackson]

If a server returned an error in response to a request,
jsonrpc_transact_block() would ignore it.  This patch changes the
behavior and updates its callers to gracefully handle the
possibility.

Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 lib/jsonrpc.c        |    8 +++++---
 ovsdb/ovsdb-client.c |   43 +++++++++++++++++++------------------------
 tests/test-ovsdb.c   |    2 +-
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 705cef7..4bcd4af 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -353,9 +353,11 @@ jsonrpc_transact_block(struct jsonrpc *rpc, struct jsonrpc_msg *request,
     if (!error) {
         for (;;) {
             error = jsonrpc_recv_block(rpc, &reply);
-            if (error
-                || (reply->type == JSONRPC_REPLY
-                    && json_equal(id, reply->id))) {
+            if (error) {
+                break;
+            }
+            if ((reply->type == JSONRPC_REPLY || reply->type == JSONRPC_ERROR)
+                && json_equal(id, reply->id)) {
                 break;
             }
             jsonrpc_msg_destroy(reply);
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index b1c660a..8562fc9 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -275,6 +275,21 @@ usage(void)
     exit(EXIT_SUCCESS);
 }
 
+static void
+check_txn(int error, struct jsonrpc_msg **reply_)
+{
+    struct jsonrpc_msg *reply = *reply_;
+
+    if (error) {
+        ovs_fatal(error, "transaction failed");
+    }
+
+    if (reply->error) {
+        ovs_fatal(error, "transaction returned error: %s",
+                  json_to_string(reply->error, table_style.json_flags));
+    }
+}
+
 static struct json *
 parse_json(const char *s)
 {
@@ -342,16 +357,12 @@ fetch_schema(struct jsonrpc *rpc, const char *database)
 {
     struct jsonrpc_msg *request, *reply;
     struct ovsdb_schema *schema;
-    int error;
 
     request = jsonrpc_create_request("get_schema",
                                      json_array_create_1(
                                          json_string_create(database)),
                                      NULL);
-    error = jsonrpc_transact_block(rpc, request, &reply);
-    if (error) {
-        ovs_fatal(error, "transaction failed");
-    }
+    check_txn(jsonrpc_transact_block(rpc, request, &reply), &reply);
     check_ovsdb_error(ovsdb_schema_from_json(reply->result, &schema));
     jsonrpc_msg_destroy(reply);
 
@@ -362,16 +373,12 @@ static void
 fetch_dbs(struct jsonrpc *rpc, struct sset *dbs)
 {
     struct jsonrpc_msg *request, *reply;
-    int error;
     size_t i;
 
     request = jsonrpc_create_request("list_dbs", json_array_create_empty(),
                                      NULL);
-    error = jsonrpc_transact_block(rpc, request, &reply);
-    if (error) {
-        ovs_fatal(error, "transaction failed");
-    }
 
+    check_txn(jsonrpc_transact_block(rpc, request, &reply), &reply);
     if (reply->result->type != JSON_ARRAY) {
         ovs_fatal(0, "list_dbs response is not array");
     }
@@ -485,19 +492,11 @@ do_transact(struct jsonrpc *rpc, const char *database OVS_UNUSED,
 {
     struct jsonrpc_msg *request, *reply;
     struct json *transaction;
-    int error;
 
     transaction = parse_json(argv[0]);
 
     request = jsonrpc_create_request("transact", transaction, NULL);
-    error = jsonrpc_transact_block(rpc, request, &reply);
-    if (error) {
-        ovs_fatal(error, "transaction failed");
-    }
-    if (reply->error) {
-        ovs_fatal(error, "transaction returned error: %s",
-                  json_to_string(reply->error, table_style.json_flags));
-    }
+    check_txn(jsonrpc_transact_block(rpc, request, &reply), &reply);
     print_json(reply->result);
     putchar('\n');
     jsonrpc_msg_destroy(reply);
@@ -898,7 +897,6 @@ do_dump(struct jsonrpc *rpc, const char *database,
     struct jsonrpc_msg *request, *reply;
     struct ovsdb_schema *schema;
     struct json *transaction;
-    int error;
 
     const struct shash_node **tables;
     size_t n_tables;
@@ -935,10 +933,7 @@ do_dump(struct jsonrpc *rpc, const char *database,
 
     /* Send request, get reply. */
     request = jsonrpc_create_request("transact", transaction, NULL);
-    error = jsonrpc_transact_block(rpc, request, &reply);
-    if (error) {
-        ovs_fatal(error, "transaction failed");
-    }
+    check_txn(jsonrpc_transact_block(rpc, request, &reply), &reply);
 
     /* Print database contents. */
     if (reply->result->type != JSON_ARRAY
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 1b9ea20..893532c 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1916,7 +1916,7 @@ do_idl(int argc, char *argv[])
             substitute_uuids(json, symtab);
             request = jsonrpc_create_request("transact", json, NULL);
             error = jsonrpc_transact_block(rpc, request, &reply);
-            if (error) {
+            if (error || reply->error) {
                 ovs_fatal(error, "jsonrpc transaction failed");
             }
             printf("%03d: ", step++);
-- 
1.7.9.1