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 <tomasz.zawadzki@intel.com>
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 <sys_sgci@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: <dongx.yi@intel.com>

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 = [
+    json_subsystems = []
-        {"subsystem": "bdev", "config": []},
+
-        {"subsystem": "nvmf", "config": []},
+    bdev_subsystem = {"subsystem": "bdev", "config": []}
-        {"subsystem": "vhost", "config": []},
-        {"subsystem": "iscsi", "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|'

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"
-        }
-      ]
     }
   ]
 }

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)