From 2d590e74b469760b9a6ec4d06e1f559c96e3a101 Mon Sep 17 00:00:00 2001 From: Sebastian Brzezinka Date: Tue, 21 Feb 2023 14:17:57 +0100 Subject: [PATCH] bdev/lvol: add param `size_in_mib` to replace `size` in bytes `rpc_bdev_lvol_resize` and `rpc_bdev_lvol_create` can now use size in MiB instead of `size` in bytes. This change make param `size` deprecated and using both `size` and `size_in_mib` return error. Since `bdev_lvol_resize` and `bdev_lvol_create` use size in MiB, name of param should reflect it, previously used param `size` is positional therefore there is no need to keep it as deprecated. This patch fix issue: #2346 Signed-off-by: Sebastian Brzezinka Change-Id: Ibbe2c056bb63d9f82dee91c87fdd501ce441d5f8 Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/16901 Reviewed-by: Jim Harris Reviewed-by: Tomasz Zawadzki Tested-by: SPDK CI Jenkins --- deprecation.md | 9 +++++++ doc/jsonrpc.md | 10 ++++--- module/bdev/lvol/vbdev_lvol_rpc.c | 43 ++++++++++++++++++++++++++++--- python/spdk/rpc/lvol.py | 12 ++++----- python/spdk/spdkcli/ui_node.py | 3 +-- scripts/rpc.py | 8 +++--- 6 files changed, 65 insertions(+), 20 deletions(-) diff --git a/deprecation.md b/deprecation.md index 3f9c0d19b..b187356da 100644 --- a/deprecation.md +++ b/deprecation.md @@ -72,3 +72,12 @@ See GitHub issue [2801](https://github.com/spdk/spdk/issues/2801) for additional New SPDK partition types should use GUID `6527994e-2c5a-4eec-9613-8f5944074e8b` which will create a bdev of the correct size. + +### lvol + +#### `vbdev_lvol_rpc_req_size` + +Param `size` in rpc commands `rpc_bdev_lvol_create` and `rpc_bdev_lvol_resize` is deprecated and +replace by `size_in_mib`. + +See GitHub issue [2346](https://github.com/spdk/spdk/issues/2346) for additional details. diff --git a/doc/jsonrpc.md b/doc/jsonrpc.md index b256db36e..faa45f797 100644 --- a/doc/jsonrpc.md +++ b/doc/jsonrpc.md @@ -9632,7 +9632,8 @@ Create a logical volume on a logical volume store. Name | Optional | Type | Description ----------------------- | -------- | ----------- | ----------- lvol_name | Required | string | Name of logical volume to create -size | Required | number | Desired size of logical volume in bytes +size | Optional | number | Desired size of logical volume in bytes (Deprecated. Please use size_in_mib instead.) +size_in_mib | Optional | number | Desired size of logical volume in MiB thin_provision | Optional | boolean | True to enable thin provisioning uuid | Optional | string | UUID of logical volume store to create logical volume on lvs_name | Optional | string | Name of logical volume store to create logical volume on @@ -9656,7 +9657,7 @@ Example request: "id": 1, "params": { "lvol_name": "LVOL0", - "size": 1048576, + "size_in_mib": 1, "lvs_name": "LVS0", "clear_method": "unmap", "thin_provision": true @@ -9802,7 +9803,8 @@ Resize a logical volume. Name | Optional | Type | Description ----------------------- | -------- | ----------- | ----------- name | Required | string | UUID or alias of the logical volume to resize -size | Required | number | Desired size of the logical volume in bytes +size | Optional | number | Desired size of the logical volume in bytes (Deprecated. Please use size_in_mib instead.) +size_in_mib | Optional | number | Desired size of the logical volume in MiB #### Example @@ -9815,7 +9817,7 @@ Example request: "id": 1, "params": { "name": "51638754-ca16-43a7-9f8f-294a0805ab0a", - "size": 2097152 + "size_in_mib": 2 } } ~~~ diff --git a/module/bdev/lvol/vbdev_lvol_rpc.c b/module/bdev/lvol/vbdev_lvol_rpc.c index 5d1a99cf6..8f9405e17 100644 --- a/module/bdev/lvol/vbdev_lvol_rpc.c +++ b/module/bdev/lvol/vbdev_lvol_rpc.c @@ -269,6 +269,7 @@ struct rpc_bdev_lvol_create { char *lvs_name; char *lvol_name; uint64_t size; + uint64_t size_in_mib; bool thin_provision; char *clear_method; }; @@ -286,7 +287,8 @@ static const struct spdk_json_object_decoder rpc_bdev_lvol_create_decoders[] = { {"uuid", offsetof(struct rpc_bdev_lvol_create, uuid), spdk_json_decode_string, true}, {"lvs_name", offsetof(struct rpc_bdev_lvol_create, lvs_name), spdk_json_decode_string, true}, {"lvol_name", offsetof(struct rpc_bdev_lvol_create, lvol_name), spdk_json_decode_string}, - {"size", offsetof(struct rpc_bdev_lvol_create, size), spdk_json_decode_uint64}, + {"size", offsetof(struct rpc_bdev_lvol_create, size), spdk_json_decode_uint64, true}, + {"size_in_mib", offsetof(struct rpc_bdev_lvol_create, size_in_mib), spdk_json_decode_uint64, true}, {"thin_provision", offsetof(struct rpc_bdev_lvol_create, thin_provision), spdk_json_decode_bool, true}, {"clear_method", offsetof(struct rpc_bdev_lvol_create, clear_method), spdk_json_decode_string, true}, }; @@ -311,6 +313,10 @@ invalid: spdk_strerror(-lvolerrno)); } +SPDK_LOG_DEPRECATION_REGISTER(vbdev_lvol_rpc_req_size, + "rpc_bdev_lvol_create/resize req.size", + "v23.09", 0); + static void rpc_bdev_lvol_create(struct spdk_jsonrpc_request *request, const struct spdk_json_val *params) @@ -319,6 +325,7 @@ rpc_bdev_lvol_create(struct spdk_jsonrpc_request *request, enum lvol_clear_method clear_method; int rc = 0; struct spdk_lvol_store *lvs = NULL; + uint64_t size = 0; SPDK_INFOLOG(lvol_rpc, "Creating blob\n"); @@ -331,6 +338,18 @@ rpc_bdev_lvol_create(struct spdk_jsonrpc_request *request, goto cleanup; } + if (req.size > 0 && req.size_in_mib > 0) { + SPDK_LOG_DEPRECATED(vbdev_lvol_rpc_req_size); + spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "size is deprecated. Specify only size_in_mib instead."); + goto cleanup; + } else if (req.size_in_mib > 0) { + size = req.size_in_mib * 1024 * 1024; + } else { + SPDK_LOG_DEPRECATED(vbdev_lvol_rpc_req_size); + size = req.size; + } + rc = vbdev_get_lvol_store_by_uuid_xor_name(req.uuid, req.lvs_name, &lvs); if (rc != 0) { spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); @@ -352,7 +371,7 @@ rpc_bdev_lvol_create(struct spdk_jsonrpc_request *request, clear_method = LVOL_CLEAR_WITH_DEFAULT; } - rc = vbdev_lvol_create(lvs, req.lvol_name, req.size, req.thin_provision, + rc = vbdev_lvol_create(lvs, req.lvol_name, size, req.thin_provision, clear_method, rpc_bdev_lvol_create_cb, request); if (rc < 0) { spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); @@ -712,6 +731,7 @@ SPDK_RPC_REGISTER("bdev_lvol_decouple_parent", rpc_bdev_lvol_decouple_parent, SP struct rpc_bdev_lvol_resize { char *name; uint64_t size; + uint64_t size_in_mib; }; static void @@ -722,7 +742,8 @@ free_rpc_bdev_lvol_resize(struct rpc_bdev_lvol_resize *req) static const struct spdk_json_object_decoder rpc_bdev_lvol_resize_decoders[] = { {"name", offsetof(struct rpc_bdev_lvol_resize, name), spdk_json_decode_string}, - {"size", offsetof(struct rpc_bdev_lvol_resize, size), spdk_json_decode_uint64}, + {"size", offsetof(struct rpc_bdev_lvol_resize, size), spdk_json_decode_uint64, true}, + {"size_in_mib", offsetof(struct rpc_bdev_lvol_resize, size_in_mib), spdk_json_decode_uint64, true}, }; static void @@ -749,6 +770,7 @@ rpc_bdev_lvol_resize(struct spdk_jsonrpc_request *request, struct rpc_bdev_lvol_resize req = {}; struct spdk_bdev *bdev; struct spdk_lvol *lvol; + uint64_t size = 0; SPDK_INFOLOG(lvol_rpc, "Resizing lvol\n"); @@ -761,6 +783,18 @@ rpc_bdev_lvol_resize(struct spdk_jsonrpc_request *request, goto cleanup; } + if (req.size > 0 && req.size_in_mib > 0) { + SPDK_LOG_DEPRECATED(vbdev_lvol_rpc_req_size); + spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "size is deprecated. Specify only size_in_mib instead."); + goto cleanup; + } else if (req.size_in_mib > 0) { + size = req.size_in_mib * 1024 * 1024; + } else { + SPDK_LOG_DEPRECATED(vbdev_lvol_rpc_req_size); + size = req.size; + } + bdev = spdk_bdev_get_by_name(req.name); if (bdev == NULL) { SPDK_ERRLOG("no bdev for provided name %s\n", req.name); @@ -774,7 +808,8 @@ rpc_bdev_lvol_resize(struct spdk_jsonrpc_request *request, goto cleanup; } - vbdev_lvol_resize(lvol, req.size, rpc_bdev_lvol_resize_cb, request); + + vbdev_lvol_resize(lvol, size, rpc_bdev_lvol_resize_cb, request); cleanup: free_rpc_bdev_lvol_resize(&req); diff --git a/python/spdk/rpc/lvol.py b/python/spdk/rpc/lvol.py index 9d814a2d8..5ce534a34 100644 --- a/python/spdk/rpc/lvol.py +++ b/python/spdk/rpc/lvol.py @@ -58,12 +58,12 @@ def bdev_lvol_grow_lvstore(client, uuid=None, lvs_name=None): return client.call('bdev_lvol_grow_lvstore', params) -def bdev_lvol_create(client, lvol_name, size, thin_provision=False, uuid=None, lvs_name=None, clear_method=None): +def bdev_lvol_create(client, lvol_name, size_in_mib, thin_provision=False, uuid=None, lvs_name=None, clear_method=None): """Create a logical volume on a logical volume store. Args: lvol_name: name of logical volume to create - size: desired size of logical volume in bytes (will be rounded up to a multiple of cluster size) + size_in_mib: desired size of logical volume in MiB (will be rounded up to a multiple of cluster size) thin_provision: True to enable thin provisioning uuid: UUID of logical volume store to create logical volume on (optional) lvs_name: name of logical volume store to create logical volume on (optional) @@ -76,7 +76,7 @@ def bdev_lvol_create(client, lvol_name, size, thin_provision=False, uuid=None, l if (uuid and lvs_name) or (not uuid and not lvs_name): raise ValueError("Either uuid or lvs_name must be specified, but not both") - params = {'lvol_name': lvol_name, 'size': size} + params = {'lvol_name': lvol_name, 'size_in_mib': size_in_mib} if thin_provision: params['thin_provision'] = thin_provision if uuid: @@ -136,16 +136,16 @@ def bdev_lvol_rename(client, old_name, new_name): return client.call('bdev_lvol_rename', params) -def bdev_lvol_resize(client, name, size): +def bdev_lvol_resize(client, name, size_in_mib): """Resize a logical volume. Args: name: name of logical volume to resize - size: desired size of logical volume in bytes (will be rounded up to a multiple of cluster size) + size_in_mib: desired size of logical volume in MiB (will be rounded up to a multiple of cluster size) """ params = { 'name': name, - 'size': size, + 'size_in_mib': size_in_mib, } return client.call('bdev_lvol_resize', params) diff --git a/python/spdk/spdkcli/ui_node.py b/python/spdk/spdkcli/ui_node.py index e01ed2ed3..2d87c5c6d 100644 --- a/python/spdk/spdkcli/ui_node.py +++ b/python/spdk/spdkcli/ui_node.py @@ -268,10 +268,9 @@ class UILvolBdev(UIBdev): lvs_name = lvs size = self.ui_eval_param(size, "number", None) - size *= (1024 * 1024) thin_provision = self.ui_eval_param(thin_provision, "bool", False) - ret_uuid = self.get_root().create_lvol_bdev(lvol_name=name, size=size, + ret_uuid = self.get_root().create_lvol_bdev(lvol_name=name, size_in_mib=size, lvs_name=lvs_name, uuid=uuid, thin_provision=thin_provision) self.shell.log.info(ret_uuid) diff --git a/scripts/rpc.py b/scripts/rpc.py index 04cd6354b..dab9059d5 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -1938,7 +1938,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse def bdev_lvol_create(args): print_json(rpc.lvol.bdev_lvol_create(args.client, lvol_name=args.lvol_name, - size=args.size * 1024 * 1024, + size_in_mib=args.size_in_mib, thin_provision=args.thin_provision, clear_method=args.clear_method, uuid=args.uuid, @@ -1951,7 +1951,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse p.add_argument('-c', '--clear-method', help="""Change default data clusters clear method. Available: none, unmap, write_zeroes""", required=False) p.add_argument('lvol_name', help='name for this lvol') - p.add_argument('size', help='size in MiB for this bdev', type=int) + p.add_argument('size_in_mib', help='size in MiB for this bdev', type=int) p.set_defaults(func=bdev_lvol_create) def bdev_lvol_snapshot(args): @@ -2003,11 +2003,11 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse def bdev_lvol_resize(args): rpc.lvol.bdev_lvol_resize(args.client, name=args.name, - size=args.size * 1024 * 1024) + size_in_mib=args.size_in_mib) p = subparsers.add_parser('bdev_lvol_resize', help='Resize existing lvol bdev') p.add_argument('name', help='lvol bdev name') - p.add_argument('size', help='new size in MiB for this bdev', type=int) + p.add_argument('size_in_mib', help='new size in MiB for this bdev', type=int) p.set_defaults(func=bdev_lvol_resize) def bdev_lvol_set_read_only(args):