From 5d0f262073248d5a8e349fbfb48fad2fe00387d9 Mon Sep 17 00:00:00 2001 From: Pawel Wodkowski Date: Fri, 12 Oct 2018 21:19:14 +0200 Subject: [PATCH] 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 Reviewed-on: https://review.gerrithub.io/429262 Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Shuhei Matsumoto Reviewed-by: Darek Stojaczyk Reviewed-by: Jim Harris Chandler-Test-Pool: SPDK Automated Test System --- include/spdk/jsonrpc.h | 52 ++++++++++++----- lib/jsonrpc/jsonrpc_client.c | 94 +++++++++++++++++-------------- lib/jsonrpc/jsonrpc_client_tcp.c | 82 ++++++++++++++++++--------- lib/jsonrpc/jsonrpc_internal.h | 29 ++++++---- test/rpc_client/rpc_client_test.c | 27 +++++++-- 5 files changed, 185 insertions(+), 99 deletions(-) diff --git a/include/spdk/jsonrpc.h b/include/spdk/jsonrpc.h index ff9438a7d..0fa5fb421 100644 --- a/include/spdk/jsonrpc.h +++ b/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 - * parameter is necessary. + * \return JSON write context or NULL in case of error. */ 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( - struct spdk_jsonrpc_client_request *req); +void spdk_jsonrpc_client_free_request(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, - spdk_jsonrpc_client_response_parser parser_fn, - void *parser_ctx); +int spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client); + +/** + * 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 } diff --git a/lib/jsonrpc/jsonrpc_client.c b/lib/jsonrpc/jsonrpc_client.c index 2426f4e3b..3ee8d2520 100644 --- a/lib/jsonrpc/jsonrpc_client.c +++ b/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}, - {"id", offsetof(struct jsonrpc_response, id), capture_id}, - {"result", offsetof(struct jsonrpc_response, result), capture_any}, + {"jsonrpc", offsetof(struct spdk_jsonrpc_client_response, version), capture_version}, + {"id", offsetof(struct spdk_jsonrpc_client_response, id), capture_id, true}, + {"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) { - SPDK_ERRLOG("JSON parse error on second pass\n"); - return SPDK_JSON_PARSE_INVALID; + if (rc != (ssize_t)values_cnt) { + SPDK_ERRLOG("JSON parse error on second pass (rc: %zd, expected: %zu)\n", rc, values_cnt); + 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 diff --git a/lib/jsonrpc/jsonrpc_client_tcp.c b/lib/jsonrpc/jsonrpc_client_tcp.c index a7696c844..8324a9319 100644 --- a/lib/jsonrpc/jsonrpc_client_tcp.c +++ b/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_response_parser parser_fn, - void *parser_ctx) +spdk_jsonrpc_client_recv_response(struct spdk_jsonrpc_client *client) { ssize_t rc = 0; size_t recv_avail; - size_t recv_offset = 0; - client->parser_fn = parser_fn; - client->parser_ctx = parser_ctx; - - recv_avail = client->recv_buf_size; + if (client->recv_buf == NULL) { + /* memory malloc for recv-buf */ + client->recv_buf = malloc(SPDK_JSONRPC_SEND_BUF_SIZE_INIT); + 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); +} diff --git a/lib/jsonrpc/jsonrpc_internal.h b/lib/jsonrpc/jsonrpc_internal.h index 87355fdb4..1518145b4 100644 --- a/lib/jsonrpc/jsonrpc_internal.h +++ b/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]; - size_t recv_buf_size; - uint8_t *recv_buf; + /* Parsed response */ + struct spdk_jsonrpc_client_response_internal *resp; - spdk_jsonrpc_client_response_parser parser_fn; - void *parser_ctx; + size_t recv_buf_size; + 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 - * SPDK_JSON_PARSE_INCOMPLETE If the provided data is not a complete JSON value - * SPDK_JSON_PARSE_INVALID if the provided data has invalid JSON syntax. + * \return 0 On success. Negative error code in error + * -EAGAIN - If the provided data is not a complete JSON value (SPDK_JSON_PARSE_INCOMPLETE) + * -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, - size_t size); +int spdk_jsonrpc_parse_response(struct spdk_jsonrpc_client *client); #endif diff --git a/test/rpc_client/rpc_client_test.c b/test/rpc_client/rpc_client_test.c index 68f847132..62b3cd924 100644 --- a/test/rpc_client/rpc_client_test.c +++ b/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; }