Merge ~mwhudson/curtin:only-eckd-dasds-extract_storage_config into curtin:master

[Michael Hudson-Doyle]

storage_config: only produce type: dasd actions for ECKD dasds

this also attempts to improve the dasd documentation a bit

This depends on https://github.com/canonical/probert/pull/101 to do anything interesting so let's not merge this until that is approved.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:763ae164a5fd43ad18b08361f4f83f251aa83015
https://jenkins.ubuntu.com/server/job/curtin-ci/16/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/16/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/16/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/16/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/16/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/16//rebuild

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

review: Needs Fixing

16e53ed... by Michael Hudson-Doyle on 2020-11-24

my probert branch emits type on dasds not dasd_type

[Michael Hudson-Doyle (mwhudson)]

Thanks for catching my thinko with the key name.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:16e53edfee1818f1fb574e660e19d1a971be5cd2
https://jenkins.ubuntu.com/server/job/curtin-ci/22/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/22/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/22/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/22/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/22/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/22//rebuild

review: Approve (continuous-integration)

[Paride Legovini (paride)]

LGTM

review: Approve

[Ryan Harper (raharper)]

Sorry for the delay, this looks good now.

review: Approve

[Server Team CI bot (server-team-bot)]

Autolanding: FAILED
More details in the following jenkins job:
https://jenkins.ubuntu.com/server/job/curtin-autoland-test/24/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-amd64/24/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-arm64/24/
    FAILURE: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-ppc64el/24/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-autoland-test/nodes=metal-s390x/24/

review: Needs Fixing (continuous-integration)

[Paride Legovini (paride)]

amd64 failure:

py3-pylint runtests: commands[0] | /jenkins/servers/server/workspace/curtin-autoland-test/nodes/metal-amd64/curtin/.tox/py3-pylint/bin/python -m pylint --errors-only curtin tests/vmtests
************* Module curtin.commands.install_grub
curtin/commands/install_grub.py:88:19: E1101: Instance of 'Distros' has no 'debian' member (no-member)
curtin/commands/install_grub.py:150:28: E1101: Instance of 'Distros' has no 'redhat' member (no-member)
curtin/commands/install_grub.py:235:28: E1101: Instance of 'Distros' has no 'debian' member (no-member)
curtin/commands/install_grub.py:239:30: E1101: Instance of 'Distros' has no 'redhat' member (no-member)
[...]

This happens from time to time, not what causes it to be flaky.

The arm64 failure is the the hung kernel (watchdog) in action:

======================================================================
ERROR: test suite for <class 'vmtests.test_basic.FocalTestScsiBasic'>
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/nose/plugins/multiprocess.py", line 788, in run
    self.setUp()
  File "/usr/lib/python3/dist-packages/nose/suite.py", line 292, in setUp
    self.setupContext(ancestor)
  File "/usr/lib/python3/dist-packages/nose/plugins/multiprocess.py", line 770, in setupContext
    super(NoSharedFixtureContextSuite, self).setupContext(context)
  File "/usr/lib/python3/dist-packages/nose/suite.py", line 315, in setupContext
    try_run(context, names)
  File "/usr/lib/python3/dist-packages/nose/util.py", line 471, in try_run
    return func()
  File "/jenkins/servers/server/workspace/curtin-autoland-test/nodes/metal-ppc64el/curtin/tests/vmtests/__init__.py", line 1308, in setUpClass
    '\n'.join(errors))
Exception: FocalTestScsiBasic:Errors during curtin installeripconfig: BOOTIF: SIOCGIFINDEX: No such device
ipconfig: no devices to configure
no search or nameservers found in /run/net-BOOTIF.conf /run/net-*.conf /run/net6-*.conf
[ 242.655062] INFO: task kworker/u4:0:7 blocked for more than 120 seconds.
[ 242.655211] Not tainted 5.4.0-48-generic #52-Ubuntu
[ 242.655258] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 242.656187] [c00000007e4b7db0] [c000000000172e24] kthread+0x1a4/0x1b0
[ 242.656247] [c00000007e4b7e20] [c00000000000b648] ret_from_kernel_thread+0x5c/0x74
[ 242.656326] INFO: task kworker/u4:1:86 blocked for more than 120 seconds.
[ 242.656385] Not tainted 5.4.0-48-generic #52-Ubuntu
[ 242.656431] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 242.657413] [c00000007eb07db0] [c000000000172e24] kthread+0x1a4/0x1b0
[ 242.657472] [c00000007eb07e20] [c00000000000b648] ret_from_kernel_thread+0x5c/0x74
[ 242.657545] INFO: task systemd-udevd:141 blocked for more than 120 seconds.
[ 242.657603] Not tainted 5.4.0-48-generic #52-Ubuntu
[ 242.657650] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 242.658266] [c00000007e57bd00] [c00000000022d208] __do_sys_finit_module+0xc8/0x150
[ 242.658337] [c00000007e57be20] [c00000000000b278] system_call+0x5c/0x68
[ 242.658398] INF...

[Server Team CI bot (server-team-bot)]

review: Approve (continuous-integration)

diff --git a/curtin/storage_config.py b/curtin/storage_config.py
index 494b142..a47aa23 100644
--- a/curtin/storage_config.py
+++ b/curtin/storage_config.py
@@ -981,6 +981,8 @@ class DasdParser(ProbertParser):
     probe_data_key = 'dasd'
 
     def asdict(self, dasd_config):
+        if dasd_config.get("type", "ECKD") != "ECKD":
+            return None
         dasd_name = os.path.basename(dasd_config['name'])
         device_id = dasd_config['device_id']
         blocksize = dasd_config['blocksize']
diff --git a/doc/topics/storage.rst b/doc/topics/storage.rst
index ebcf8a4..75c5537 100644
--- a/doc/topics/storage.rst
+++ b/doc/topics/storage.rst
@@ -55,14 +55,21 @@ commands include:
 
 Dasd Command
 ~~~~~~~~~~~~
-The ``dasd`` command sets up a s390x system DASD device for use by curtin.
-DASD devices require several parameters to configure the low-level structure
-of the block device.  Curtin will examine the configuration and determine if
-the specified DASD matches the configuration.  If the device does not match
-the configuration Curtin will perform a format of the device to achieve the
-required configuration.  Once a DASD device has been formatted it may be used
-like regular Linux block devices and can be partitioned (with limitations)
-with Curtin's ``disk`` command.  The ``dasd`` command may contain the following
+
+The ``dasd`` command sets up an ECKD s390x system DASD device for use
+by curtin.  FBA DASD devices do not require this low level set
+up. ECKD DASD drives can also be passed via virtio to KVM virtual
+machines and in this case this set up must be done on the host prior
+to starting the virtual machine.
+
+DASD devices require several parameters to configure the low-level
+structure of the block device.  Curtin will examine the configuration
+and determine if the specified DASD matches the configuration.  If the
+device does not match the configuration Curtin will perform a format
+of the device to achieve the required configuration.  Once a DASD
+device has been formatted it may be used like regular Linux block
+devices and can be partitioned (with limitations) with Curtin's
+``disk`` command.  The ``dasd`` command may contain the following
 keys:
 
 **device_id**: *<ccw bus_id: X.Y.ZZZZ>*
@@ -138,11 +145,12 @@ The disk command sets up disks for use by curtin. It can wipe the disks, create
 partition tables, or just verify that the disks exist with an existing partition
 table. A disk command may contain all or some of the following keys:
 
-**ptable**: *msdos, gpt*
+**ptable**: *msdos, gpt, vtoc*
 
 If the ``ptable`` key is present and a curtin will create an empty
-partition table of that type on the disk.  Curtin supports msdos and
-gpt partition tables.
+partition table of that type on the disk.  On almost all drives,
+curtin supports msdos and gpt partition tables; ECKD DASD drives on
+s390x mainframes can only use the "vtoc" partition table.
 
 **serial**: *<serial number>*
 
diff --git a/tests/unittests/test_storage_config.py b/tests/unittests/test_storage_config.py
index a38f9cd..2d27d4e 100644
--- a/tests/unittests/test_storage_config.py
+++ b/tests/unittests/test_storage_config.py
@@ -783,7 +783,6 @@ class TestDasdParser(CiTestCase):
 
     def test_dasd_asdict(self):
         """ DasdParser converts known dasd_data to expected dict. """
-        devname = "/dev/dasda"
         expected_dict = {
             'type': 'dasd',
             'id': 'dasd-dasda',
@@ -792,9 +791,35 @@ class TestDasdParser(CiTestCase):
             'mode': 'quick',
             'disk_layout': 'cdl',
         }
-        dasd_data = self.dasd.class_data[devname]
+        dasd_data = self.dasd.class_data["/dev/dasda"]
         self.assertDictEqual(expected_dict, self.dasd.asdict(dasd_data))
 
+    def test_dasd_includes_ECKD(self):
+        """ DasdParser converts known dasd_data to expected dict. """
+        expected_dict = {
+            'type': 'dasd',
+            'id': 'dasd-dasda',
+            'device_id': '0.0.1522',
+            'blocksize': 4096,
+            'mode': 'quick',
+            'disk_layout': 'cdl',
+        }
+        dasd_data = self.dasd.class_data["/dev/dasda"]
+        dasd_data['type'] = "ECKD"
+        self.assertDictEqual(expected_dict, self.dasd.asdict(dasd_data))
+
+    def test_dasd_skips_FBA(self):
+        """ DasdParser skips FBA dasds. """
+        dasd_data = self.dasd.class_data["/dev/dasda"]
+        dasd_data['type'] = "FBA"
+        self.assertIsNone(self.dasd.asdict(dasd_data))
+
+    def test_dasd_skips_virt(self):
+        """ DasdParser skips virt dasds. """
+        dasd_data = self.dasd.class_data["/dev/dasda"]
+        dasd_data['type'] = "virt"
+        self.assertIsNone(self.dasd.asdict(dasd_data))
+
     @skipUnlessJsonSchema()
     def test_dasd_parser_parses_all_dasd_devs(self):
         """ DasdParser returns expected dicts for known dasd probe data."""
@@ -802,6 +827,15 @@ class TestDasdParser(CiTestCase):
         self.assertEqual(5, len(configs))
         self.assertEqual(0, len(errors))
 
+    @skipUnlessJsonSchema()
+    def test_dasd_parser_skips_virt_FBA(self):
+        """ DasdParser returns expected dicts for known dasd probe data."""
+        self.dasd.class_data["/dev/dasda"]['type'] = "FBA"
+        self.dasd.class_data["/dev/dasdb"]['type'] = "virt"
+        configs, errors = self.dasd.parse()
+        self.assertEqual(3, len(configs))
+        self.assertEqual(0, len(errors))
+
 
 class TestDmCryptParser(CiTestCase):