From 64debe0453499f55ce068d9dd261b567dcac0d38 Mon Sep 17 00:00:00 2001 From: jiaqizho Date: Wed, 3 Mar 2021 15:24:46 +0800 Subject: [PATCH] rpc: add a command parser The changes in the nvmf_create_transport show how this command parser work. And there have two benefit for this changed. 1. Simplify the definition of rpc method. no need add so many args anymore. Also it retains its original functions, so we can also check the input args. 2. Make the rpc call more versatile, for example. when user extend the subparsers(add new args into subparsers), they can pass some private args into the rpc method by command parser. Signed-off-by: jiaqizho Change-Id: Iaf916e3454f23715cf9216794bb80c65b2b4603f Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/6652 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Ben Walker Reviewed-by: Aleksey Marchuk --- CHANGELOG.md | 5 +++ scripts/rpc.py | 31 +------------ scripts/rpc/cmd_parser.py | 31 +++++++++++++ scripts/rpc/nvmf.py | 92 ++++++--------------------------------- 4 files changed, 52 insertions(+), 107 deletions(-) create mode 100644 scripts/rpc/cmd_parser.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d3cbf94f5..a7f7d358a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -173,6 +173,11 @@ use `enable-zerocopy-send-server` or `enable-zerocopy-send-client` instead. Parameter `disable-zerocopy-send` of RPC `sock_impl_set_options` is deprecated and will be removed in SPDK 21.07, use `disable-zerocopy-send-server` or `disable-zerocopy-send-client` instead. +Added cmd_parser.py used to parse the args from argparse. There are +two benefit to use command parser: +- Simplify the definition of rpc method. It will reduce the rpc method code. +- Make the rpc call more versatile. User can add private args into rpc method. + ### rpm Added support for new RPM spec, rpmbuild/spdk.spec, which can be used for packaging the diff --git a/scripts/rpc.py b/scripts/rpc.py index f60863cb0..1e4f57bf3 100755 --- a/scripts/rpc.py +++ b/scripts/rpc.py @@ -1864,28 +1864,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse p.set_defaults(func=nvmf_set_config) def nvmf_create_transport(args): - rpc.nvmf.nvmf_create_transport(args.client, - trtype=args.trtype, - tgt_name=args.tgt_name, - max_queue_depth=args.max_queue_depth, - max_qpairs_per_ctrlr=args.max_qpairs_per_ctrlr, - max_io_qpairs_per_ctrlr=args.max_io_qpairs_per_ctrlr, - in_capsule_data_size=args.in_capsule_data_size, - max_io_size=args.max_io_size, - io_unit_size=args.io_unit_size, - max_aq_depth=args.max_aq_depth, - num_shared_buffers=args.num_shared_buffers, - buf_cache_size=args.buf_cache_size, - num_cqe=args.num_cqe, - max_srq_depth=args.max_srq_depth, - no_srq=args.no_srq, - c2h_success=args.c2h_success, - dif_insert_or_strip=args.dif_insert_or_strip, - sock_priority=args.sock_priority, - acceptor_backlog=args.acceptor_backlog, - abort_timeout_sec=args.abort_timeout_sec, - no_wr_batching=args.no_wr_batching, - control_msg_num=args.control_msg_num) + rpc.nvmf.nvmf_create_transport(**vars(args)) p = subparsers.add_parser('nvmf_create_transport', help='Create NVMf transport') p.add_argument('-t', '--trtype', help='Transport type (ex. RDMA)', type=str, required=True) @@ -1969,13 +1948,7 @@ Format: 'user:u1 secret:s1 muser:mu1 msecret:ms1,user:u2 secret:s2 muser:mu2 mse p.set_defaults(func=nvmf_delete_subsystem) def nvmf_subsystem_add_listener(args): - rpc.nvmf.nvmf_subsystem_add_listener(args.client, - nqn=args.nqn, - trtype=args.trtype, - traddr=args.traddr, - tgt_name=args.tgt_name, - adrfam=args.adrfam, - trsvcid=args.trsvcid) + rpc.nvmf.nvmf_subsystem_add_listener(**vars(args)) p = subparsers.add_parser('nvmf_subsystem_add_listener', help='Add a listener to an NVMe-oF subsystem') p.add_argument('nqn', help='NVMe-oF subsystem NQN') diff --git a/scripts/rpc/cmd_parser.py b/scripts/rpc/cmd_parser.py new file mode 100644 index 000000000..4cf728f75 --- /dev/null +++ b/scripts/rpc/cmd_parser.py @@ -0,0 +1,31 @@ +args_global = ['server_addr', 'port', 'timeout', 'verbose', 'dry_run', 'conn_retries', + 'is_server', 'rpc_plugin', 'called_rpc_name', 'func', 'client'] + + +def strip_globals(kwargs): + for arg in args_global: + kwargs.pop(arg, None) + + +def remove_null(kwargs): + keys = [] + for key, value in kwargs.items(): + if value is None: + keys.append(key) + + for key in keys: + kwargs.pop(key, None) + + +def apply_defaults(kwargs, **defaults): + for key, value in defaults.items(): + if key not in kwargs: + kwargs[key] = value + + +def group_as(kwargs, name, values): + group = {} + for arg in values: + if arg in kwargs and kwargs[arg] is not None: + group[arg] = kwargs.pop(arg, None) + kwargs[name] = group diff --git a/scripts/rpc/nvmf.py b/scripts/rpc/nvmf.py index bd8eb3497..3070e49e0 100644 --- a/scripts/rpc/nvmf.py +++ b/scripts/rpc/nvmf.py @@ -1,4 +1,5 @@ from .helpers import deprecated_alias +from .cmd_parser import * @deprecated_alias('set_nvmf_target_max_subsystems') @@ -91,28 +92,7 @@ def nvmf_get_targets(client): return client.call("nvmf_get_targets") -def nvmf_create_transport(client, - trtype, - tgt_name=None, - max_queue_depth=None, - max_qpairs_per_ctrlr=None, - max_io_qpairs_per_ctrlr=None, - in_capsule_data_size=None, - max_io_size=None, - io_unit_size=None, - max_aq_depth=None, - num_shared_buffers=None, - buf_cache_size=None, - num_cqe=None, - max_srq_depth=None, - no_srq=False, - c2h_success=True, - dif_insert_or_strip=None, - sock_priority=None, - acceptor_backlog=None, - abort_timeout_sec=None, - no_wr_batching=None, - control_msg_num=None): +def nvmf_create_transport(client, **params): """NVMf Transport Create options. Args: @@ -138,50 +118,14 @@ def nvmf_create_transport(client, Returns: True or False """ - params = {} - params['trtype'] = trtype - if tgt_name: - params['tgt_name'] = tgt_name - if max_queue_depth: - params['max_queue_depth'] = max_queue_depth - if max_qpairs_per_ctrlr: + strip_globals(params) + apply_defaults(params, no_srq=False, c2h_success=True) + remove_null(params) + + if 'max_qpairs_per_ctrlr' in params: print("WARNING: max_qpairs_per_ctrlr is deprecated, please use max_io_qpairs_per_ctrlr.") - params['max_qpairs_per_ctrlr'] = max_qpairs_per_ctrlr - if max_io_qpairs_per_ctrlr: - params['max_io_qpairs_per_ctrlr'] = max_io_qpairs_per_ctrlr - if in_capsule_data_size is not None: - params['in_capsule_data_size'] = in_capsule_data_size - if max_io_size: - params['max_io_size'] = max_io_size - if io_unit_size: - params['io_unit_size'] = io_unit_size - if max_aq_depth: - params['max_aq_depth'] = max_aq_depth - if num_shared_buffers: - params['num_shared_buffers'] = num_shared_buffers - if buf_cache_size is not None: - params['buf_cache_size'] = buf_cache_size - if num_cqe: - params['num_cqe'] = num_cqe - if max_srq_depth: - params['max_srq_depth'] = max_srq_depth - if no_srq: - params['no_srq'] = no_srq - if c2h_success is not None: - params['c2h_success'] = c2h_success - if dif_insert_or_strip: - params['dif_insert_or_strip'] = dif_insert_or_strip - if sock_priority is not None: - params['sock_priority'] = sock_priority - if acceptor_backlog is not None: - params['acceptor_backlog'] = acceptor_backlog - if abort_timeout_sec: - params['abort_timeout_sec'] = abort_timeout_sec - if no_wr_batching is not None: - params['no_wr_batching'] = no_wr_batching - if control_msg_num is not None: - params['control_msg_num'] = control_msg_num + return client.call('nvmf_create_transport', params) @@ -274,7 +218,8 @@ def nvmf_create_subsystem(client, return client.call('nvmf_create_subsystem', params) -def nvmf_subsystem_add_listener(client, nqn, trtype, traddr, trsvcid, adrfam, tgt_name=None): +def nvmf_subsystem_add_listener(client, **params): + """Add a new listen address to an NVMe-oF subsystem. Args: @@ -288,20 +233,11 @@ def nvmf_subsystem_add_listener(client, nqn, trtype, traddr, trsvcid, adrfam, tg Returns: True or False """ - listen_address = {'trtype': trtype, - 'traddr': traddr} - if trsvcid: - listen_address['trsvcid'] = trsvcid - - if adrfam: - listen_address['adrfam'] = adrfam - - params = {'nqn': nqn, - 'listen_address': listen_address} - - if tgt_name: - params['tgt_name'] = tgt_name + strip_globals(params) + apply_defaults(params, tgt_name=None) + group_as(params, 'listen_address', ['trtype', 'traddr', 'trsvcid', 'adrfam']) + remove_null(params) return client.call('nvmf_subsystem_add_listener', params)