Merge ~mwhudson/curtin:partition-dasd-without-device_id into curtin:master

[Michael Hudson-Doyle]

allow adding a vtoc partition without a device id

this is needed for the "dasd passed to kvm via virtio" use case

pls note the prerequisite branch

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

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

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

review: Approve (continuous-integration)

[Paride Legovini (paride)]

LGTM from the code perspective, but I didn't test it.

review: Approve

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

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

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

review: Approve (continuous-integration)

[Michael Hudson-Doyle (mwhudson)]

Hmm prerequisite branches and a bot that squash merges do not get along.

Unmerged commits

aa3b584... by Michael Hudson-Doyle on 2020-11-19

allow adding a vtoc partition without a device id

this is needed for the "dasd passed to kvm via virtio" use case

diff --git a/curtin/block/dasd.py b/curtin/block/dasd.py
index b10651c..3423601 100644
--- a/curtin/block/dasd.py
+++ b/curtin/block/dasd.py
@@ -32,6 +32,57 @@ class DasdPartitionTable:
     def tracks_needed(self, size_in_bytes):
         return ((size_in_bytes - 1) // self.bytes_per_track) + 1
 
+    def _ptable_for_new_partition(self, partnumber, partsize):
+        if partnumber > 3:
+            raise ValueError('DASD devices only allow 3 partitions')
+
+        # first partition always starts at track 2
+        # all others start after the previous partition ends
+        if partnumber == 1:
+            start = 2
+        else:
+            start = int(self.partitions[-1].end) + 1
+        end = start + self.tracks_needed(partsize) - 1
+
+        return [
+            (p.start, p.end) for p in self.partitions[:partnumber-1]
+            ] + [(start, end)]
+
+    def add_partition(self, partnumber, partsize):
+        """ Add a partition to this DasdDevice specifying partnumber and size.
+
+        :param partnumber: integer value of partition number (1, 2 or 3)
+        :param partsize: partition sizes in bytes.
+
+        :raises: ValueError on invalid devname
+
+        Example fdasd command with defaults:
+          fdasd --verbose --config=/tmp/curtin/dasd-part1.fdasd /dev/dasdb
+        """
+        LOG.debug(
+            "add_partition: partnumber: %s partsize: %s",
+            partnumber, partsize)
+
+        partitions = self._ptable_for_new_partition(partnumber, partsize)
+        LOG.debug("fdasd: partitions to be created: %s", partitions)
+        content = "\n".join([
+            "[%s,%s]" % (part[0], part[1]) for part in partitions
+            ])
+        LOG.debug("fdasd: content=\n%s", content)
+        wfp = tempfile.NamedTemporaryFile(suffix=".fdasd", delete=False)
+        wfp.close()
+        util.write_file(wfp.name, content)
+        cmd = ['fdasd', '--verbose', '--config=%s' % wfp.name, self.devname]
+        LOG.debug('Partitioning %s with %s', self.devname, cmd)
+        try:
+            out, err = util.subp(cmd, capture=True)
+        except util.ProcessExecutionError as e:
+            LOG.error("Partitioning failed: %s", e)
+            raise
+        finally:
+            if os.path.exists(wfp.name):
+                os.unlink(wfp.name)
+
     @classmethod
     def from_fdasd_output(cls, devname, output):
         line_iter = iter(output.splitlines())
@@ -131,22 +182,6 @@ def dasd_format(devname):
 
 
 DASD_FORMAT = r"^format\s+:.+\s+(?P<value>\w+\s\w+)$"
-<<<<<<< curtin/block/dasd.py
-
-
-def find_val(regex, content):
-    m = re.search(regex, content, re.MULTILINE)
-    if m is not None:
-        return m.group("value")
-
-
-def _dasd_format(dasdview_output):
-    """ Read and return specified device "disk_layout" value.
-
-    :returns: string: One of ['cdl', 'ldl', 'not-formatted'].
-    :raises: ValueError if dasdview result missing 'format' section.
-
-=======
 
 
 def find_val(regex, content):
@@ -161,7 +196,6 @@ def _dasd_format(dasdview_output):
     :returns: string: One of ['cdl', 'ldl', 'not-formatted'].
     :raises: ValueError if dasdview result missing 'format' section.
 
->>>>>>> curtin/block/dasd.py
     """
     if not dasdview_output:
         return
@@ -243,59 +277,6 @@ class DasdDevice(CcwDevice):
     def devname(self):
         return '/dev/disk/by-path/ccw-%s' % self.device_id
 
-    def partition(self, partnumber, partsize, strict=True):
-        """ Add a partition to this DasdDevice specifying partnumber and size.
-
-        :param partnumber: integer value of partition number (1, 2 or 3)
-        :param partsize: partition sizes in bytes.
-        :param strict: boolean which enforces that dasd device exists before
-            issuing fdasd command, defaults to True.
-
-        :raises: RuntimeError if strict==True and devname does not exist.
-        :raises: ValueError on invalid devname
-
-        Example fdasd command with defaults:
-          fdasd --verbose --config=/tmp/curtin/dasd-part1.fdasd /dev/dasdb
-        """
-        if partnumber > 3:
-            raise ValueError('DASD devices only allow 3 partitions')
-
-        if strict and not os.path.exists(self.devname):
-            raise RuntimeError("devname '%s' does not exist" % self.devname)
-
-        pt = DasdPartitionTable.from_fdasd(self.devname)
-        new_partitions = []
-        for partinfo in pt.partitions[0:partnumber]:
-            new_partitions.append((partinfo.start, partinfo.end))
-
-        # first partition always starts at track 2
-        # all others start after the previous partition ends
-        if partnumber == 1:
-            start = 2
-        else:
-            start = int(pt.partitions[-1].end) + 1
-        # end is inclusive
-        end = start + pt.tracks_needed(partsize) - 1
-        new_partitions.append((start, end))
-
-        content = "\n".join(["[%s,%s]" % (part[0], part[1])
-                             for part in new_partitions])
-        LOG.debug("fdasd: partitions to be created: %s", new_partitions)
-        LOG.debug("fdasd: content=\n%s", content)
-        wfp = tempfile.NamedTemporaryFile(suffix=".fdasd", delete=False)
-        wfp.close()
-        util.write_file(wfp.name, content)
-        cmd = ['fdasd', '--verbose', '--config=%s' % wfp.name, self.devname]
-        LOG.debug('Partitioning %s with %s', self.devname, cmd)
-        try:
-            out, err = util.subp(cmd, capture=True)
-        except util.ProcessExecutionError as e:
-            LOG.error("Partitioning failed: %s", e)
-            raise
-        finally:
-            if os.path.exists(wfp.name):
-                os.unlink(wfp.name)
-
     def is_not_formatted(self):
         """ Returns a boolean indicating if the specified device_id is not yet
             formatted.
diff --git a/curtin/commands/block_meta.py b/curtin/commands/block_meta.py
index 66dfdb4..e29c1e4 100644
--- a/curtin/commands/block_meta.py
+++ b/curtin/commands/block_meta.py
@@ -946,9 +946,8 @@ def partition_handler(info, storage_config):
                    "--typecode=%s:%s" % (partnumber, typecode), disk]
             util.subp(cmd, capture=True)
         elif disk_ptable == "vtoc":
-            disk_device_id = storage_config.get(device).get('device_id')
-            dasd_device = dasd.DasdDevice(disk_device_id)
-            dasd_device.partition(partnumber, length_bytes)
+            dasd_pt = dasd.DasdPartitionTable.from_fdasd(disk)
+            dasd_pt.add_partition(partnumber, length_bytes)
         else:
             raise ValueError("parent partition has invalid partition table")
 
diff --git a/tests/unittests/test_block_dasd.py b/tests/unittests/test_block_dasd.py
index cc545a1..2058800 100644
--- a/tests/unittests/test_block_dasd.py
+++ b/tests/unittests/test_block_dasd.py
@@ -415,7 +415,6 @@ class TestDasdInfo(CiTestCase):
 
 
 class TestDasdFormat(CiTestCase):
-<<<<<<< tests/unittests/test_block_dasd.py
 
     def setUp(self):
         super(TestDasdFormat, self).setUp()
@@ -447,39 +446,6 @@ class TestDasdFormat(CiTestCase):
 
 class Test_DasdFormat(CiTestCase):
 
-=======
-
-    def setUp(self):
-        super(TestDasdFormat, self).setUp()
-        self.add_patch('curtin.block.dasd.util.subp', 'm_subp')
-        self.add_patch('curtin.block.dasd.os.path.exists', 'm_exists')
-        self.add_patch('curtin.block.dasd._dasd_format', 'm_dasd_format')
-
-        # defaults
-        self.m_exists.return_value = True
-        self.m_subp.return_value = ('', '')
-
-    def test_dasd_format_raise_error_on_invalid_device(self):
-        """dasdview raises error on invalid device path."""
-        devname = self.random_string()
-        self.m_exists.return_value = False
-        with self.assertRaises(ValueError):
-            dasd.dasd_format(devname)
-
-    def test_dasd_foramt_calls__dasd_format(self):
-        """dasdview calls parser on output from dasdview command."""
-        devname = self.random_string()
-        view = self.random_string()
-        self.m_subp.return_value = (view, self.random_string())
-        dasd.dasd_format(devname)
-        self.m_subp.assert_called_with(
-            ['dasdview', '--extended', devname], capture=True)
-        self.m_dasd_format.assert_called_with(view)
-
-
-class Test_DasdFormat(CiTestCase):
-
->>>>>>> tests/unittests/test_block_dasd.py
     view_output_template = textwrap.dedent("""\
 
     --- general DASD information ----------------------------------------------