bdev/pmem: add more descriptive rpc error messages

Improve error messages where possible.
Also change return code values to negative in bdev_pmem.c so that
it follows the same pattern as other bdev modules.

Change-Id: I81b205a41104e15457154d1e31540210636ef9d3
Signed-off-by: Karol Latecki <karol.latecki@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/461548
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Vitaliy Mysak <vitaliy.mysak@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>

lib/bdev/pmem/bdev_pmem.c

@@ -321,17 +321,17 @@ spdk_create_pmem_disk(const char *pmem_file, const char *name, struct spdk_bdev
 
 	if (name == NULL) {
 		SPDK_ERRLOG("Missing name parameter for spdk_create_pmem_disk()\n");
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	if (pmemblk_check(pmem_file, 0) != 1) {
 		SPDK_ERRLOG("Pool '%s' check failed: %s\n", pmem_file, pmemblk_errormsg());
-		return EIO;
+		return -EIO;
 	}
 
 	pdisk = calloc(1, sizeof(*pdisk));
 	if (!pdisk) {
-		return ENOMEM;
+		return -ENOMEM;
 	}
 
 	snprintf(pdisk->pmem_file, sizeof(pdisk->pmem_file), "%s", pmem_file);
@@ -339,7 +339,7 @@ spdk_create_pmem_disk(const char *pmem_file, const char *name, struct spdk_bdev
 	if (!pdisk->pool) {
 		SPDK_ERRLOG("Opening pmem pool '%s' failed: %d\n", pmem_file, errno);
 		free(pdisk);
-		return errno;
+		return -errno;
 	}
 
 	block_size = pmemblk_bsize(pdisk->pool);
@@ -349,21 +349,21 @@ spdk_create_pmem_disk(const char *pmem_file, const char *name, struct spdk_bdev
 		SPDK_ERRLOG("Block size must be more than 0 bytes\n");
 		pmemblk_close(pdisk->pool);
 		free(pdisk);
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	if (num_blocks == 0) {
 		SPDK_ERRLOG("Disk must be more than 0 blocks\n");
 		pmemblk_close(pdisk->pool);
 		free(pdisk);
-		return EINVAL;
+		return -EINVAL;
 	}
 
 	pdisk->disk.name = strdup(name);
 	if (!pdisk->disk.name) {
 		pmemblk_close(pdisk->pool);
 		free(pdisk);
-		return ENOMEM;
+		return -ENOMEM;
 	}
 
 	pdisk->disk.product_name = "pmemblk disk";

lib/bdev/pmem/bdev_pmem_rpc.c

@@ -69,26 +69,21 @@ spdk_rpc_construct_pmem_bdev(struct spdk_jsonrpc_request *request,
 				    SPDK_COUNTOF(rpc_construct_pmem_decoders),
 				    &req)) {
 		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "spdk_json_decode_object failed\n");
-		rc = EINVAL;
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
-		goto invalid;
+						 "spdk_json_decode_object failed");
+		goto cleanup;
 	}
 	rc = spdk_create_pmem_disk(req.pmem_file, req.name, &bdev);
 	if (rc != 0) {
-		goto invalid;
+		spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc));
-	}
+		goto cleanup;
-	if (bdev == NULL) {
-		goto invalid;
 	}
 
 	w = spdk_jsonrpc_begin_result(request);
 	spdk_json_write_string(w, spdk_bdev_get_name(bdev));
 	spdk_jsonrpc_end_result(request, w);
 
-	free_rpc_construct_pmem_bdev(&req);
+cleanup:
-	return;
-
-invalid:
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, spdk_strerror(rc));
 	free_rpc_construct_pmem_bdev(&req);
 }
 SPDK_RPC_REGISTER("construct_pmem_bdev", spdk_rpc_construct_pmem_bdev, SPDK_RPC_RUNTIME)
@@ -123,28 +118,26 @@ spdk_rpc_delete_pmem_bdev(struct spdk_jsonrpc_request *request,
 {
 	struct rpc_delete_pmem req = {NULL};
 	struct spdk_bdev *bdev;
-	int rc;
 
 	if (spdk_json_decode_object(params, rpc_delete_pmem_decoders,
 				    SPDK_COUNTOF(rpc_delete_pmem_decoders),
 				    &req)) {
-		rc = -EINVAL;
+		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "spdk_json_decode_object failed\n");
-		goto invalid;
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
+						 "spdk_json_decode_object failed");
 	}
 
 	bdev = spdk_bdev_get_by_name(req.name);
 	if (bdev == NULL) {
-		rc = -ENODEV;
+		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "bdev '%s' does not exist\n", req.name);
-		goto invalid;
+		spdk_jsonrpc_send_error_response(request, -ENODEV, spdk_strerror(ENODEV));
+		goto cleanup;
 	}
 
 	spdk_delete_pmem_disk(bdev, _spdk_rpc_delete_pmem_bdev_cb, request);
-	free_rpc_delete_pmem(&req);
-	return;
 
-invalid:
+cleanup:
 	free_rpc_delete_pmem(&req);
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, spdk_strerror(-rc));
 }
 SPDK_RPC_REGISTER("delete_pmem_bdev", spdk_rpc_delete_pmem_bdev, SPDK_RPC_RUNTIME)
 
@@ -179,25 +172,33 @@ spdk_rpc_create_pmem_pool(struct spdk_jsonrpc_request *request,
 				    SPDK_COUNTOF(rpc_create_pmem_pool_decoders),
 				    &req)) {
 		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "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;
 	}
 
 	/* libpmemblk pool has to contain at least 256 blocks */
 	if (req.num_blocks < 256) {
-		goto invalid;
+		spdk_jsonrpc_send_error_response(request, -EINVAL,
+						 "Pmem pool num_blocks must be at least 256");
+		goto cleanup;
 	}
 
 	pool_size = req.num_blocks * req.block_size;
 	if (pool_size < PMEMBLK_MIN_POOL) {
-		goto invalid;
+		spdk_jsonrpc_send_error_response_fmt(request, -EINVAL,
+						     "Pmem pool size must be at least %ld", PMEMBLK_MIN_POOL);
+		goto cleanup;
 	}
 
 	pbp = pmemblk_create(req.pmem_file, req.block_size, pool_size, 0666);
 	if (pbp == NULL) {
 		const char *msg = pmemblk_errormsg();
 
-		SPDK_ERRLOG("pmemblk_create() failed: %s\n", msg ? msg : "(logs disabled)");
+		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "pmemblk_create() failed: %s\n", msg ? msg : "(logs disabled)");
-		goto invalid;
+		spdk_jsonrpc_send_error_response_fmt(request, -SPDK_JSONRPC_ERROR_INVALID_PARAMS,
+						     "pmemblk_create failed: %s", msg ? msg : "(logs disabled)");
+		goto cleanup;
 	}
 
 	pmemblk_close(pbp);
@@ -205,11 +206,8 @@ spdk_rpc_create_pmem_pool(struct spdk_jsonrpc_request *request,
 	w = spdk_jsonrpc_begin_result(request);
 	spdk_json_write_bool(w, true);
 	spdk_jsonrpc_end_result(request, w);
-	free_rpc_create_pmem_pool(&req);
-	return;
 
-invalid:
+cleanup:
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
 	free_rpc_create_pmem_pool(&req);
 }
 SPDK_RPC_REGISTER("create_pmem_pool", spdk_rpc_create_pmem_pool, SPDK_RPC_RUNTIME)
@@ -240,24 +238,32 @@ spdk_rpc_pmem_pool_info(struct spdk_jsonrpc_request *request,
 	if (spdk_json_decode_object(params, rpc_pmem_pool_info_decoders,
 				    SPDK_COUNTOF(rpc_pmem_pool_info_decoders),
 				    &req)) {
-		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "spdk_json_decode_object failed\n");
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
-		goto invalid;
+						 "spdk_json_decode_object failed");
+		goto cleanup;
 	}
 
 	pbp = pmemblk_open(req.pmem_file, 0);
 	if (pbp == NULL) {
-		goto invalid;
+		const char *msg = pmemblk_errormsg();
+
+		spdk_jsonrpc_send_error_response_fmt(request, -SPDK_JSONRPC_ERROR_INVALID_PARAMS,
+						     "pmemblk_open failed: %s", msg ? msg : "(logs disabled)");
+		goto cleanup;
 	}
 
 	block_size = pmemblk_bsize(pbp);
 	num_blocks = pmemblk_nblock(pbp);
 
-
 	pmemblk_close(pbp);
 
 	/* Check pmem pool consistency */
 	if (pmemblk_check(req.pmem_file, block_size) != 1) {
-		goto invalid;
+		const char *msg = pmemblk_errormsg();
+
+		spdk_jsonrpc_send_error_response_fmt(request, -SPDK_JSONRPC_ERROR_INVALID_PARAMS,
+						     "pmemblk_check failed: %s", msg ? msg : "(logs disabled)");
+		goto cleanup;
 	}
 
 	w = spdk_jsonrpc_begin_result(request);
@@ -268,11 +274,8 @@ spdk_rpc_pmem_pool_info(struct spdk_jsonrpc_request *request,
 	spdk_json_write_object_end(w);
 	spdk_json_write_array_end(w);
 	spdk_jsonrpc_end_result(request, w);
-	free_rpc_pmem_pool_info(&req);
-	return;
 
-invalid:
+cleanup:
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
 	free_rpc_pmem_pool_info(&req);
 }
 SPDK_RPC_REGISTER("pmem_pool_info", spdk_rpc_pmem_pool_info, SPDK_RPC_RUNTIME)
@@ -302,8 +305,9 @@ spdk_rpc_delete_pmem_pool(struct spdk_jsonrpc_request *request,
 	if (spdk_json_decode_object(params, rpc_delete_pmem_pool_decoders,
 				    SPDK_COUNTOF(rpc_delete_pmem_pool_decoders),
 				    &req)) {
-		SPDK_DEBUGLOG(SPDK_LOG_BDEV_PMEM, "spdk_json_decode_object failed\n");
+		spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR,
-		goto invalid;
+						 "spdk_json_decode_object failed");
+		goto cleanup;
 	}
 
 	/* Check if file is actually pmem pool */
@@ -311,8 +315,9 @@ spdk_rpc_delete_pmem_pool(struct spdk_jsonrpc_request *request,
 	if (rc != 1) {
 		const char *msg = pmemblk_errormsg();
 
-		SPDK_ERRLOG("pmemblk_check() failed (%d): %s\n", rc, msg ? msg : "(logs disabled)");
+		spdk_jsonrpc_send_error_response_fmt(request, -SPDK_JSONRPC_ERROR_INVALID_PARAMS,
-		goto invalid;
+						     "pmemblk_check failed: %s", msg ? msg : "(logs disabled)");
+		goto cleanup;
 	}
 
 	unlink(req.pmem_file);
@@ -320,11 +325,8 @@ spdk_rpc_delete_pmem_pool(struct spdk_jsonrpc_request *request,
 	w = spdk_jsonrpc_begin_result(request);
 	spdk_json_write_bool(w, true);
 	spdk_jsonrpc_end_result(request, w);
-	free_rpc_delete_pmem_pool(&req);
-	return;
 
-invalid:
+cleanup:
-	spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, "Invalid parameters");
 	free_rpc_delete_pmem_pool(&req);
 }
 SPDK_RPC_REGISTER("delete_pmem_pool", spdk_rpc_delete_pmem_pool, SPDK_RPC_RUNTIME)