From a9a0761bccc15218f28631a1b9d3360e03c36a4b Mon Sep 17 00:00:00 2001 From: Tomasz Zawadzki Date: Tue, 12 Jan 2021 06:37:04 -0500 Subject: [PATCH] scripts/config_converter: skip sections not present in legacy config Sections 'Nvme', 'Nvmf', 'Bdev' and 'iSCSI' were always present in the output of config_converter. Even when they were not present in the legacy config. This posed an issue with iSCSI section. Support for iscsi_set_options is part of lib/iscsi, that is not compiled with all applications (such as nvmf_tgt). Fixes #1741 This patch checks if there is corresponding section in the legacy config, and only then presents the JSON output. Added test to verify output from empty legacy config. Signed-off-by: Tomasz Zawadzki Change-Id: Ief94e92115dbd80c890dcb434b7c6d5376421c9e Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5879 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Jim Harris Reviewed-by: Shuhei Matsumoto Reviewed-by: --- scripts/config_converter.py | 34 +++++---- test/config_converter/spdk_config_virtio.json | 71 ------------------- test/config_converter/test_converter.sh | 3 + 3 files changed, 22 insertions(+), 86 deletions(-) diff --git a/scripts/config_converter.py b/scripts/config_converter.py index 072eafcd55..967070a20e 100755 --- a/scripts/config_converter.py +++ b/scripts/config_converter.py @@ -54,26 +54,34 @@ no_yes_map = {"no": False, "No": False, "Yes": True, "yes": True} def generate_new_json_config(): - json_subsystem = [ - {"subsystem": "bdev", "config": []}, - {"subsystem": "nvmf", "config": []}, - {"subsystem": "vhost", "config": []}, - {"subsystem": "iscsi", "config": []} - ] + json_subsystems = [] + + bdev_subsystem = {"subsystem": "bdev", "config": []} for method in subsystem['bdev']: for item in subsystem['bdev'][method]: - json_subsystem[0]['config'].append(item) + bdev_subsystem['config'].append(item) + if bdev_subsystem['config']: + json_subsystems.append(bdev_subsystem) + nvmf_subsystem = {"subsystem": "nvmf", "config": []} for method in subsystem['nvmf']: for item in subsystem['nvmf'][method]: - json_subsystem[1]['config'].append(item) + nvmf_subsystem['config'].append(item) + if nvmf_subsystem['config']: + json_subsystems.append(nvmf_subsystem) + vhost_subsystem = {"subsystem": "vhost", "config": []} for method in subsystem['vhost']: for item in subsystem['vhost'][method]: - json_subsystem[2]['config'].append(item) + vhost_subsystem['config'].append(item) + if vhost_subsystem['config']: + json_subsystems.append(vhost_subsystem) + iscsi_subsystem = {"subsystem": "iscsi", "config": []} for method in subsystem['iscsi']: for item in subsystem['iscsi'][method]: - json_subsystem[3]['config'].append(item) + iscsi_subsystem['config'].append(item) + if iscsi_subsystem['config']: + json_subsystems.append(iscsi_subsystem) - return {"subsystems": json_subsystem} + return {"subsystems": json_subsystems} section_to_subsystem = { @@ -613,10 +621,6 @@ if __name__ == "__main__": except Exception as e: print("Exception while parsing config: %s" % e) exit(1) - # Add missing sections to generate default configuration - for section in ['Nvme', 'Nvmf', 'Bdev', 'iSCSI']: - if section not in config.sections(): - config.add_section(section) for section in config.sections(): match = re.match(r'(Bdev|Nvme|Malloc|VirtioUser\d+|Split|Pmem|AIO|' diff --git a/test/config_converter/spdk_config_virtio.json b/test/config_converter/spdk_config_virtio.json index e0d3ede30e..32e62328fd 100644 --- a/test/config_converter/spdk_config_virtio.json +++ b/test/config_converter/spdk_config_virtio.json @@ -3,30 +3,6 @@ { "subsystem": "bdev", "config": [ - { - "params": { - "bdev_io_pool_size": 65536, - "bdev_io_cache_size": 256 - }, - "method": "bdev_set_options" - }, - { - "params": { - "retry_count": 4, - "timeout_us": 0, - "nvme_adminq_poll_period_us": 1000000, - "nvme_ioq_poll_period_us": 0, - "action_on_timeout": "none" - }, - "method": "bdev_nvme_set_options" - }, - { - "params": { - "enable": false, - "period_us": 100000 - }, - "method": "bdev_nvme_set_hotplug" - }, { "params": { "name": "VirtioScsi0", @@ -61,53 +37,6 @@ "method": "bdev_virtio_attach_controller" } ] - }, - { - "subsystem": "nvmf", - "config": [ - { - "params": { - "acceptor_poll_rate": 10000 - }, - "method": "nvmf_set_config" - }, - { - "params": { - "max_subsystems": 1024 - }, - "method": "nvmf_set_max_subsystems" - } - ] - }, - { - "subsystem": "vhost", - "config": [] - }, - { - "subsystem": "iscsi", - "config": [ - { - "params": { - "allow_duplicated_isid": false, - "default_time2retain": 20, - "mutual_chap": false, - "require_chap": false, - "immediate_data": true, - "node_base": "iqn.2016-06.io.spdk", - "nop_in_interval": 30, - "max_connections_per_session": 2, - "first_burst_length": 8192, - "max_queue_depth": 64, - "nop_timeout": 60, - "chap_group": 0, - "max_sessions": 128, - "error_recovery_level": 0, - "disable_chap": false, - "default_time2wait": 2 - }, - "method": "iscsi_set_options" - } - ] } ] } diff --git a/test/config_converter/test_converter.sh b/test/config_converter/test_converter.sh index 1b3279a855..218cc8a1d3 100755 --- a/test/config_converter/test_converter.sh +++ b/test/config_converter/test_converter.sh @@ -19,6 +19,9 @@ function on_error_exit() { trap 'on_error_exit' ERR +empty_json=$(echo "" | $SPDK_BUILD_DIR/scripts/config_converter.py | jq -c) +[[ ${empty_json} == '{"subsystems":[]}' ]] + $SPDK_BUILD_DIR/scripts/config_converter.py < $CONVERTER_DIR/config.ini > $CONVERTER_DIR/config_converter.json $SPDK_BUILD_DIR/scripts/config_converter.py < $CONVERTER_DIR/config_virtio.ini > $CONVERTER_DIR/config_virtio_converter.json diff -I "cpumask" -I "max_queue_depth" -I "queue_depth" <(jq -S . $CONVERTER_DIR/config_converter.json) <(jq -S . $CONVERTER_DIR/spdk_config.json)