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 <karol.latecki@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/461884
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>

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 *
+int
-spdk_bdev_rbd_create(const char *name, const char *user_id, const char *pool_name,
+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 = spdk_bdev_rbd_create(&bdev, NULL, NULL, pool_name, NULL, rbd_name, block_size);
-			rc = -1;
+		if (rc) {
 			goto end;
 		}
 	}

lib/bdev/rbd/bdev_rbd.h

@@ -43,7 +43,8 @@ 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,
+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);
 /**

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,
+	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 (bdev == NULL) {
+	if (rc) {
-		goto invalid;
+		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:
+cleanup:
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
 	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;
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
-		goto invalid;
+						 "spdk_json_decode_object failed");
+		goto cleanup;
 	}
 
 	bdev = spdk_bdev_get_by_name(req.name);
 	if (bdev == NULL) {
-		rc = -ENODEV;
+		spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV));
-		goto invalid;
+		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)