jsonrpc: introduce spdk_jsonrpc_client_response

As a step toward non-blocking JSON RPC client change the the way we
retrive the response.

Now we retrive full response by calling
spdk_jsonrpc_client_recv_response(). If response is ready it will be
parsed to spdk_jsonrpc_client_response object then user can issue
spdk_jsonrpc_client_get_response() to get the response object. When
processing response object is done the user calls
spdk_jsonrpc_client_free_response() to free it.

This logic will simplify both non-blocking mode and multiple response
handling.

Change-Id: I737d8a34283f4a68795d6bc6ba0c8646b7ae3319
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-on: https://review.gerrithub.io/429262
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>

include/spdk/jsonrpc.h

@@ -67,6 +67,13 @@ struct spdk_jsonrpc_request;
 struct spdk_jsonrpc_client;
 struct spdk_jsonrpc_client_request;
 
+struct spdk_jsonrpc_client_response {
+	struct spdk_json_val *version;
+	struct spdk_json_val *id;
+	struct spdk_json_val *result;
+	struct spdk_json_val *error;
+};
+
 /**
  * User callback to handle a single JSON-RPC request.
  *
@@ -185,8 +192,7 @@ void spdk_jsonrpc_send_error_response_fmt(struct spdk_jsonrpc_request *request,
  * \param id ID index for the request. If < 0 skip ID.
  * \param method Name of the RPC method. If NULL caller will have to create "method" key.
  *
- * \return JSON write context to write the parameter object to, or NULL if no
+ * \return JSON write context or NULL in case of error.
- * parameter is necessary.
  */
 struct spdk_json_write_ctx *
 spdk_jsonrpc_begin_request(struct spdk_jsonrpc_client_request *request, int32_t id,
@@ -220,27 +226,29 @@ struct spdk_jsonrpc_client *spdk_jsonrpc_client_connect(const char *rpc_sock_add
 void spdk_jsonrpc_client_close(struct spdk_jsonrpc_client *client);
 
 /**
- * Create one JSON-RPC request
+ * Create one JSON-RPC request. Returned request must be passed to
+ * \c spdk_jsonrpc_client_send_request when done or to \c spdk_jsonrpc_client_free_request
+ * if discaded.
  *
- * \return Created JSON-RPC request.
+ * \return pointer to JSON-RPC request object.
  */
 struct spdk_jsonrpc_client_request *spdk_jsonrpc_client_create_request(void);
 
 /**
- * free one JSON-RPC request
+ * Free one JSON-RPC request.
  *
- * \param req Created JSON-RPC request.
+ * \param req pointer to JSON-RPC request object.
  */
-void spdk_jsonrpc_client_free_request(
+void spdk_jsonrpc_client_free_request(struct spdk_jsonrpc_client_request *req);
-	struct spdk_jsonrpc_client_request *req);
 
 /**
- * Send the JSON-RPC request in JSON-RPC client.
+ * Send the JSON-RPC request in JSON-RPC client. Library takes ownership of the
+ * request object and will free it when done.
  *
  * \param client JSON-RPC client.
  * \param req JSON-RPC request.
  *
- * \return 0 on success.
+ * \return 0 on success or negative error code.
  */
 int spdk_jsonrpc_client_send_request(struct spdk_jsonrpc_client *client,
 				     struct spdk_jsonrpc_client_request *req);
@@ -249,14 +257,28 @@ int spdk_jsonrpc_client_send_request(struct spdk_jsonrpc_client *client,
  * Receive the JSON-RPC response in JSON-RPC client.
  *
  * \param client JSON-RPC client.
- * \param parser_fn Specific function used to parse the result inside response.
- * \param parser_ctx Parameter for parser_fn.
  *
  * \return 0 on success.
  */
-int spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client,
+int spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client);
-				      spdk_jsonrpc_client_response_parser parser_fn,
+
-				      void *parser_ctx);
+/**
+ * Return JSON RPC response object representing next available response from client connection.
+ * Returned pointer must be freed using \c spdk_jsonrpc_client_free_response
+ *
+ * \param client
+ * \return pointer to JSON RPC response object or NULL if no response available.
+ */
+struct spdk_jsonrpc_client_response *spdk_jsonrpc_client_get_response(struct spdk_jsonrpc_client
+		*client);
+
+/**
+ * Free response object obtained from \c spdk_jsonrpc_client_get_response
+ *
+ * \param resp pointer to JSON RPC response object. If NULL no operation is performed.
+ */
+void spdk_jsonrpc_client_free_response(struct spdk_jsonrpc_client_response *resp);
+
 
 #ifdef __cplusplus
 }

lib/jsonrpc/jsonrpc_client.c

@@ -34,14 +34,8 @@
 #include "spdk/util.h"
 #include "jsonrpc_internal.h"
 
-struct jsonrpc_response {
-	const struct spdk_json_val *version;
-	const struct spdk_json_val *id;
-	const struct spdk_json_val *result;
-};
-
 static int
-capture_string(const struct spdk_json_val *val, void *out)
+capture_version(const struct spdk_json_val *val, void *out)
 {
 	const struct spdk_json_val **vptr = out;
 
@@ -59,7 +53,7 @@ capture_id(const struct spdk_json_val *val, void *out)
 	const struct spdk_json_val **vptr = out;
 
 	if (val->type != SPDK_JSON_VAL_STRING && val->type != SPDK_JSON_VAL_NUMBER) {
-		return SPDK_JSON_PARSE_INVALID;
+		return -EINVAL;
 	}
 
 	*vptr = val;
@@ -76,67 +70,85 @@ capture_any(const struct spdk_json_val *val, void *out)
 }
 
 static const struct spdk_json_object_decoder jsonrpc_response_decoders[] = {
-	{"jsonrpc", offsetof(struct jsonrpc_response, version), capture_string},
+	{"jsonrpc", offsetof(struct spdk_jsonrpc_client_response, version), capture_version},
-	{"id", offsetof(struct jsonrpc_response, id), capture_id},
+	{"id", offsetof(struct spdk_jsonrpc_client_response, id), capture_id, true},
-	{"result", offsetof(struct jsonrpc_response, result), capture_any},
+	{"result", offsetof(struct spdk_jsonrpc_client_response, result), capture_any, true},
+	{"error", offsetof(struct spdk_jsonrpc_client_response, error), capture_any, true},
 };
 
-static int
-parse_single_response(struct spdk_json_val *values,
-		      spdk_jsonrpc_client_response_parser parser_fn,
-		      void *parser_ctx)
-{
-	struct jsonrpc_response resp = {};
-
-	if (spdk_json_decode_object(values, jsonrpc_response_decoders,
-				    SPDK_COUNTOF(jsonrpc_response_decoders),
-				    &resp)) {
-		return SPDK_JSON_PARSE_INVALID;
-	}
-
-	return parser_fn(parser_ctx, resp.result);
-}
-
 int
-spdk_jsonrpc_parse_response(struct spdk_jsonrpc_client *client, void *json, size_t size)
+spdk_jsonrpc_parse_response(struct spdk_jsonrpc_client *client)
 {
+	struct spdk_jsonrpc_client_response_internal *r;
 	ssize_t rc;
+	size_t buf_len;
+	size_t values_cnt;
 	void *end = NULL;
 
+
 	/* Check to see if we have received a full JSON value. */
-	rc = spdk_json_parse(json, size, NULL, 0, &end, 0);
+	rc = spdk_json_parse(client->recv_buf, client->recv_offset, NULL, 0, &end, 0);
 	if (rc == SPDK_JSON_PARSE_INCOMPLETE) {
-		return rc;
+		return -EAGAIN;
 	}
 
-	SPDK_DEBUGLOG(SPDK_LOG_RPC_CLIENT, "Json string is :\n%s\n", (char *)json);
+	SPDK_DEBUGLOG(SPDK_LOG_RPC_CLIENT, "JSON string is :\n%s\n", client->recv_buf);
 	if (rc < 0 || rc > SPDK_JSONRPC_MAX_VALUES) {
-		SPDK_ERRLOG("JSON parse error\n");
+		SPDK_ERRLOG("JSON parse error (rc: %zd)\n", rc);
 		/*
 		 * Can't recover from parse error (no guaranteed resync point in streaming JSON).
 		 * Return an error to indicate that the connection should be closed.
 		 */
-		return SPDK_JSON_PARSE_INVALID;
+		return -EINVAL;
 	}
 
+	values_cnt = rc;
+
+	r = calloc(1, sizeof(*r) + sizeof(struct spdk_json_val) * (values_cnt + 1));
+	if (!r) {
+		return -errno;
+	}
+
+	if (client->resp) {
+		free(r);
+		return -ENOSPC;
+	}
+
+	client->resp = r;
+
+	r->buf = client->recv_buf;
+	buf_len = client->recv_offset;
+	r->values_cnt = values_cnt;
+
+	client->recv_buf_size = 0;
+	client->recv_offset = 0;
+	client->recv_buf = NULL;
+
 	/* Decode a second time now that there is a full JSON value available. */
-	rc = spdk_json_parse(json, size, client->values, SPDK_JSONRPC_MAX_VALUES, &end,
+	rc = spdk_json_parse(r->buf, buf_len, r->values, values_cnt, &end,
 			     SPDK_JSON_PARSE_FLAG_DECODE_IN_PLACE);
-	if (rc < 0 || rc > SPDK_JSONRPC_MAX_VALUES) {
+	if (rc != (ssize_t)values_cnt) {
-		SPDK_ERRLOG("JSON parse error on second pass\n");
+		SPDK_ERRLOG("JSON parse error on second pass (rc: %zd, expected: %zu)\n", rc, values_cnt);
-		return SPDK_JSON_PARSE_INVALID;
+		goto err;
 	}
 
 	assert(end != NULL);
 
-	if (client->values[0].type != SPDK_JSON_VAL_OBJECT_BEGIN) {
+	if (r->values[0].type != SPDK_JSON_VAL_OBJECT_BEGIN) {
 		SPDK_ERRLOG("top-level JSON value was not object\n");
-		return SPDK_JSON_PARSE_INVALID;
+		goto err;
 	}
 
-	rc = parse_single_response(client->values, client->parser_fn, client->parser_ctx);
+	if (spdk_json_decode_object(r->values, jsonrpc_response_decoders,
+				    SPDK_COUNTOF(jsonrpc_response_decoders), &r->jsonrpc)) {
+		goto err;
+	}
 
-	return rc;
+	return 0;
+
+err:
+	spdk_jsonrpc_client_free_response(&r->jsonrpc);
+	return -EINVAL;
 }
 
 static int

lib/jsonrpc/jsonrpc_client_tcp.c

@@ -32,6 +32,7 @@
  */
 #include "spdk/string.h"
 #include "jsonrpc_internal.h"
+#include "spdk/util.h"
 
 #define RPC_DEFAULT_PORT	"5260"
 
@@ -62,16 +63,6 @@ _spdk_jsonrpc_client_connect(int domain, int protocol,
 		return NULL;
 	}
 
-	/* memory malloc for recv-buf */
-	client->recv_buf = malloc(SPDK_JSONRPC_SEND_BUF_SIZE_INIT);
-	if (!client->recv_buf) {
-		SPDK_ERRLOG("memory malloc for recv-buf failed\n");
-		close(client->sockfd);
-		free(client);
-		return NULL;
-	}
-	client->recv_buf_size = SPDK_JSONRPC_SEND_BUF_SIZE_INIT;
-
 	return client;
 }
 
@@ -146,8 +137,11 @@ spdk_jsonrpc_client_close(struct spdk_jsonrpc_client *client)
 {
 	if (client->sockfd >= 0) {
 		close(client->sockfd);
-		free(client->recv_buf);
+	}
-		client->sockfd = -1;
+
+	free(client->recv_buf);
+	if (client->resp) {
+		spdk_jsonrpc_client_free_response(&client->resp->jsonrpc);
 	}
 
 	free(client);
@@ -232,21 +226,26 @@ recv_buf_expand(struct spdk_jsonrpc_client *client)
 }
 
 int
-spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client,
+spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client)
-				  spdk_jsonrpc_client_response_parser parser_fn,
-				  void *parser_ctx)
 {
 	ssize_t rc = 0;
 	size_t recv_avail;
-	size_t recv_offset = 0;
 
-	client->parser_fn = parser_fn;
+	if (client->recv_buf == NULL) {
-	client->parser_ctx = parser_ctx;
+		/* memory malloc for recv-buf */
-
+		client->recv_buf = malloc(SPDK_JSONRPC_SEND_BUF_SIZE_INIT);
-	recv_avail = client->recv_buf_size;
+		if (!client->recv_buf) {
+			rc = errno;
+			SPDK_ERRLOG("malloc() failed (%d): %s\n", (int)rc, spdk_strerror(rc));
+			return -rc;
+		}
+		client->recv_buf_size = SPDK_JSONRPC_SEND_BUF_SIZE_INIT;
+		client->recv_offset = 0;
+	}
 
+	recv_avail = client->recv_buf_size - client->recv_offset;
 	while (recv_avail > 0) {
-		rc = recv(client->sockfd, client->recv_buf + recv_offset, recv_avail, 0);
+		rc = recv(client->sockfd, client->recv_buf + client->recv_offset, recv_avail - 1, 0);
 		if (rc < 0) {
 			if (errno == EINTR) {
 				continue;
@@ -257,28 +256,57 @@ spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client,
 			return -EIO;
 		}
 
-		recv_offset += rc;
+		client->recv_offset += rc;
 		recv_avail -= rc;
 
+		client->recv_buf[client->recv_offset] = '\0';
+
 		/* Check to see if we have received a full JSON value. */
-		rc = spdk_jsonrpc_parse_response(client, client->recv_buf, recv_offset);
+		rc = spdk_jsonrpc_parse_response(client);
 		if (rc == 0) {
 			/* Successfully parsed response */
 			return 0;
-		} else if (rc != SPDK_JSON_PARSE_INCOMPLETE) {
+		} else if (rc && rc != -EAGAIN) {
 			SPDK_ERRLOG("jsonrpc parse request failed\n");
-			return -EINVAL;
+			return rc;
 		}
 
 		/* Expand receive buffer if larger one is needed */
-		if (recv_avail == 0) {
+		if (recv_avail == 1) {
 			rc = recv_buf_expand(client);
 			if (rc != 0) {
 				return rc;
 			}
-			recv_avail = client->recv_buf_size - recv_offset;
+			recv_avail = client->recv_buf_size - client->recv_offset;
 		}
 	}
 
 	return 0;
 }
+
+struct spdk_jsonrpc_client_response *
+spdk_jsonrpc_client_get_response(struct spdk_jsonrpc_client *client)
+{
+	struct spdk_jsonrpc_client_response_internal *r = client->resp;
+
+	if (r == NULL) {
+		return NULL;
+	}
+
+	client->resp = NULL;
+	return &r->jsonrpc;
+}
+
+void
+spdk_jsonrpc_client_free_response(struct spdk_jsonrpc_client_response *resp)
+{
+	struct spdk_jsonrpc_client_response_internal *r;
+
+	if (!resp) {
+		return;
+	}
+
+	r = SPDK_CONTAINEROF(resp, struct spdk_jsonrpc_client_response_internal, jsonrpc);
+	free(r->buf);
+	free(r);
+}

lib/jsonrpc/jsonrpc_internal.h

@@ -106,15 +106,22 @@ struct spdk_jsonrpc_client_request {
 	uint8_t *send_buf;
 };
 
+struct spdk_jsonrpc_client_response_internal {
+	struct spdk_jsonrpc_client_response jsonrpc;
+	uint8_t *buf;
+	size_t values_cnt;
+	struct spdk_json_val values[];
+};
+
 struct spdk_jsonrpc_client {
 	int sockfd;
 
-	struct spdk_json_val values[SPDK_JSONRPC_MAX_VALUES];
+	/* Parsed response */
-	size_t recv_buf_size;
+	struct spdk_jsonrpc_client_response_internal *resp;
-	uint8_t *recv_buf;
 
-	spdk_jsonrpc_client_response_parser parser_fn;
+	size_t recv_buf_size;
-	void *parser_ctx;
+	size_t recv_offset;
+	char *recv_buf;
 };
 
 /* jsonrpc_server_tcp */
@@ -136,14 +143,12 @@ void spdk_jsonrpc_free_request(struct spdk_jsonrpc_request *request);
  * Parse JSON data as RPC command response.
  *
  * \param client structure pointer of jsonrpc client
- * \param json Raw JSON data; must be encoded in UTF-8.
- * \param size Size of data in bytes.
  *
- * \return 0 On success
+ * \return 0 On success. Negative error code in error
- *         SPDK_JSON_PARSE_INCOMPLETE If the provided data is not a complete JSON value
+ * -EAGAIN - If the provided data is not a complete JSON value (SPDK_JSON_PARSE_INCOMPLETE)
- *         SPDK_JSON_PARSE_INVALID if the provided data has invalid JSON syntax.
+ * -EINVAL - If the provided data has invalid JSON syntax and can't be parsed (SPDK_JSON_PARSE_INVALID).
+ * -ENOSPC - No space left to store parsed response.
  */
-int spdk_jsonrpc_parse_response(struct spdk_jsonrpc_client *client, void *json,
+int spdk_jsonrpc_parse_response(struct spdk_jsonrpc_client *client);
-				size_t size);
 
 #endif

test/rpc_client/rpc_client_test.c

@@ -48,11 +48,9 @@ struct get_jsonrpc_methods_resp {
 };
 
 static int
-get_jsonrpc_method_json_parser(void *parser_ctx,
+get_jsonrpc_method_json_parser(struct get_jsonrpc_methods_resp *resp,
 			       const struct spdk_json_val *result)
 {
-	struct get_jsonrpc_methods_resp *resp = parser_ctx;
-
 	return spdk_json_decode_array(result, spdk_json_decode_string, resp->method_names,
 				      RPC_MAX_METHODS, &resp->method_num, sizeof(char *));
 }
@@ -61,6 +59,7 @@ static int
 spdk_jsonrpc_client_check_rpc_method(struct spdk_jsonrpc_client *client, char *method_name)
 {
 	int rc, i;
+	struct spdk_jsonrpc_client_response *json_resp = NULL;
 	struct get_jsonrpc_methods_resp resp = {};
 	struct spdk_json_write_ctx *w;
 	struct spdk_jsonrpc_client_request *request;
@@ -75,8 +74,27 @@ spdk_jsonrpc_client_check_rpc_method(struct spdk_jsonrpc_client *client, char *m
 	spdk_jsonrpc_client_send_request(client, request);
 	spdk_jsonrpc_client_free_request(request);
 
-	rc = spdk_jsonrpc_client_recv_response(client, get_jsonrpc_method_json_parser, &resp);
+	rc = spdk_jsonrpc_client_recv_response(client);
+	if (rc) {
+		goto out;
+	}
 
+	json_resp = spdk_jsonrpc_client_get_response(client);
+	if (json_resp == NULL) {
+		rc = -errno;
+		goto out;
+
+	}
+
+	/* Check for error response */
+	if (json_resp->error != NULL) {
+		rc = -1;
+		goto out;
+	}
+
+	assert(json_resp->result);
+
+	rc = get_jsonrpc_method_json_parser(&resp, json_resp->result);
 	if (rc) {
 		goto out;
 	}
@@ -96,6 +114,7 @@ out:
 		free(resp.method_names[i]);
 	}
 
+	spdk_jsonrpc_client_free_response(json_resp);
 	return rc;
 }