From ac8b48898211cb77b6e8c75b688d3c9f2f3c6044 Mon Sep 17 00:00:00 2001 From: Karol Latecki Date: Mon, 15 Jul 2019 14:08:29 +0200 Subject: [PATCH] bdev/rbd: add more descriptive rpc error messages Improve error messages where possible. Modify spdk_bdev_rbd_create() to return instead instead of pointer for better return code handling. Change-Id: I5fcf90794f5fe44296422c654c5f8d404f3d5eef Signed-off-by: Karol Latecki Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/461884 Tested-by: SPDK CI Jenkins Reviewed-by: Darek Stojaczyk Reviewed-by: Changpeng Liu --- lib/bdev/rbd/bdev_rbd.c | 32 ++++++++++++++++------------- lib/bdev/rbd/bdev_rbd.h | 7 ++++--- lib/bdev/rbd/bdev_rbd_rpc.c | 41 +++++++++++++++++-------------------- 3 files changed, 41 insertions(+), 39 deletions(-) diff --git a/lib/bdev/rbd/bdev_rbd.c b/lib/bdev/rbd/bdev_rbd.c index 3d4ba330d..4f4ceea49 100644 --- a/lib/bdev/rbd/bdev_rbd.c +++ b/lib/bdev/rbd/bdev_rbd.c @@ -681,8 +681,9 @@ static const struct spdk_bdev_fn_table rbd_fn_table = { .write_config_json = bdev_rbd_write_config_json, }; -struct spdk_bdev * -spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_name, +int +spdk_bdev_rbd_create(struct spdk_bdev **bdev, const char *name, const char *user_id, + const char *pool_name, const char *const *config, const char *rbd_name, uint32_t block_size) @@ -691,38 +692,38 @@ spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_nam int ret; if ((pool_name == NULL) || (rbd_name == NULL)) { - return NULL; + return -EINVAL; } rbd = calloc(1, sizeof(struct bdev_rbd)); if (rbd == NULL) { SPDK_ERRLOG("Failed to allocate bdev_rbd struct\n"); - return NULL; + return -ENOMEM; } rbd->rbd_name = strdup(rbd_name); if (!rbd->rbd_name) { bdev_rbd_free(rbd); - return NULL; + return -ENOMEM; } if (user_id) { rbd->user_id = strdup(user_id); if (!rbd->user_id) { bdev_rbd_free(rbd); - return NULL; + return -ENOMEM; } } rbd->pool_name = strdup(pool_name); if (!rbd->pool_name) { bdev_rbd_free(rbd); - return NULL; + return -ENOMEM; } if (config && !(rbd->config = spdk_bdev_rbd_dup_config(config))) { bdev_rbd_free(rbd); - return NULL; + return -ENOMEM; } ret = bdev_rbd_init(rbd->user_id, rbd->pool_name, @@ -731,7 +732,7 @@ spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_nam if (ret < 0) { bdev_rbd_free(rbd); SPDK_ERRLOG("Failed to init rbd device\n"); - return NULL; + return ret; } if (name) { @@ -741,7 +742,7 @@ spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_nam } if (!rbd->disk.name) { bdev_rbd_free(rbd); - return NULL; + return -ENOMEM; } rbd->disk.product_name = "Ceph Rbd Disk"; bdev_rbd_count++; @@ -763,10 +764,12 @@ spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_nam if (ret) { spdk_io_device_unregister(rbd, NULL); bdev_rbd_free(rbd); - return NULL; + return ret; } - return &rbd->disk; + *bdev = &(rbd->disk); + + return ret; } void @@ -787,6 +790,7 @@ bdev_rbd_library_init(void) const char *val; const char *pool_name; const char *rbd_name; + struct spdk_bdev *bdev; uint32_t block_size; long int tmp; @@ -841,8 +845,8 @@ bdev_rbd_library_init(void) } /* TODO(?): user_id and rbd config values */ - if (spdk_bdev_rbd_create(NULL, NULL, pool_name, NULL, rbd_name, block_size) == NULL) { - rc = -1; + rc = spdk_bdev_rbd_create(&bdev, NULL, NULL, pool_name, NULL, rbd_name, block_size); + if (rc) { goto end; } } diff --git a/lib/bdev/rbd/bdev_rbd.h b/lib/bdev/rbd/bdev_rbd.h index c275ba464..d62df027f 100644 --- a/lib/bdev/rbd/bdev_rbd.h +++ b/lib/bdev/rbd/bdev_rbd.h @@ -43,9 +43,10 @@ char **spdk_bdev_rbd_dup_config(const char *const *config); typedef void (*spdk_delete_rbd_complete)(void *cb_arg, int bdeverrno); -struct spdk_bdev *spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_name, - const char *const *config, - const char *rbd_name, uint32_t block_size); +int spdk_bdev_rbd_create(struct spdk_bdev **bdev, const char *name, const char *user_id, + const char *pool_name, + const char *const *config, + const char *rbd_name, uint32_t block_size); /** * Delete rbd bdev. * diff --git a/lib/bdev/rbd/bdev_rbd_rpc.c b/lib/bdev/rbd/bdev_rbd_rpc.c index 31ff8d989..8ed4ff6b0 100644 --- a/lib/bdev/rbd/bdev_rbd_rpc.c +++ b/lib/bdev/rbd/bdev_rbd_rpc.c @@ -114,35 +114,35 @@ spdk_rpc_construct_rbd_bdev(struct spdk_jsonrpc_request *request, struct rpc_construct_rbd req = {}; struct spdk_json_write_ctx *w; struct spdk_bdev *bdev; + int rc = 0; if (spdk_json_decode_object(params, rpc_construct_rbd_decoders, SPDK_COUNTOF(rpc_construct_rbd_decoders), &req)) { SPDK_DEBUGLOG(SPDK_LOG_BDEV_RBD, "spdk_json_decode_object failed\n"); - goto invalid; + spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "spdk_json_decode_object failed"); + goto cleanup; } - bdev = spdk_bdev_rbd_create(req.name, req.user_id, req.pool_name, - (const char *const *)req.config, - req.rbd_name, - req.block_size); - if (bdev == NULL) { - goto invalid; + rc = spdk_bdev_rbd_create(&bdev, req.name, req.user_id, req.pool_name, + (const char *const *)req.config, + req.rbd_name, + req.block_size); + if (rc) { + spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); + goto cleanup; } - free_rpc_construct_rbd(&req); - w = spdk_jsonrpc_begin_result(request); if (w == NULL) { - return; + goto cleanup; } spdk_json_write_string(w, spdk_bdev_get_name(bdev)); spdk_jsonrpc_end_result(request, w); - return; -invalid: - spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters"); +cleanup: free_rpc_construct_rbd(&req); } SPDK_RPC_REGISTER("construct_rbd_bdev", spdk_rpc_construct_rbd_bdev, SPDK_RPC_RUNTIME) @@ -182,27 +182,24 @@ spdk_rpc_delete_rbd_bdev(struct spdk_jsonrpc_request *request, { struct rpc_delete_rbd req = {NULL}; struct spdk_bdev *bdev; - int rc; if (spdk_json_decode_object(params, rpc_delete_rbd_decoders, SPDK_COUNTOF(rpc_delete_rbd_decoders), &req)) { - rc = -EINVAL; - goto invalid; + spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "spdk_json_decode_object failed"); + goto cleanup; } bdev = spdk_bdev_get_by_name(req.name); if (bdev == NULL) { - rc = -ENODEV; - goto invalid; + spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV)); + goto cleanup; } spdk_bdev_rbd_delete(bdev, _spdk_rpc_delete_rbd_bdev_cb, request); - free_rpc_delete_rbd(&req); - return; -invalid: +cleanup: free_rpc_delete_rbd(&req); - spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, spdk_strerror(-rc)); } SPDK_RPC_REGISTER("delete_rbd_bdev", spdk_rpc_delete_rbd_bdev, SPDK_RPC_RUNTIME)