From cc205fac90d70a328a55c7447b5e09ffd69d4a0e Mon Sep 17 00:00:00 2001
From: David Lehman <dlehman@redhat.com>
Date: Fri, 9 Oct 2015 16:59:35 -0400
Subject: [PATCH 2/3] Add support for minimal alignment of very small
 partitions.

This patch teaches blivet to use the minimal alignment if the optimal
alignment's grain size is larger than the requested partition.

Some disks export an optimal io size of 16 or even 64 MiB. Blivet
always using the optimal alignment made it impossible to allocate
a PReP partition (allowed size range 4-10 MiB) on these disks.

Related: rhbz#1267858
---
 blivet/formats/disklabel.py          | 144 +++++++++++++++++++++++++++--------
 tests/formats_test/disklabel_test.py |  66 ++++++++++++++++
 2 files changed, 178 insertions(+), 32 deletions(-)
 create mode 100644 tests/formats_test/disklabel_test.py

diff --git a/blivet/formats/disklabel.py b/blivet/formats/disklabel.py
index ba5e1a4..e60255d 100644
--- a/blivet/formats/disklabel.py
+++ b/blivet/formats/disklabel.py
@@ -25,7 +25,7 @@ import os
 from ..storage_log import log_exception_info, log_method_call
 import parted
 import _ped
-from ..errors import DiskLabelCommitError, InvalidDiskLabelError
+from ..errors import DiskLabelCommitError, InvalidDiskLabelError, AlignmentError
 from .. import arch
 from .. import udev
 from .. import util
@@ -66,8 +66,10 @@ class DiskLabel(DeviceFormat):
         self._partedDevice = None
         self._partedDisk = None
         self._origPartedDisk = None
-        self._alignment = None
-        self._endAlignment = None
+
+        self._diskLabelAlignment = None
+        self._minimalAlignment = None
+        self._optimalAlignment = None
 
         if self.partedDevice:
             # set up the parted objects and raise exception on failure
@@ -79,7 +81,7 @@ class DiskLabel(DeviceFormat):
             We can't do copy.deepcopy on parted objects, which is okay.
         """
         return util.variable_copy(self, memo,
-           shallow=('_partedDevice', '_alignment', '_endAlignment'),
+           shallow=('_partedDevice', '_optimalAlignment', '_minimalAlignment',),
            duplicate=('_partedDisk', '_origPartedDisk'))
 
     def __repr__(self):
@@ -94,8 +96,8 @@ class DiskLabel(DeviceFormat):
               "  partedDevice = %(dev)s\n" %
               {"type": self.labelType, "count": len(self.partitions),
                "sectorSize": self.sectorSize,
-               "offset": self.alignment.offset,
-               "grain": self.alignment.grainSize,
+               "offset": self.getAlignment().offset,
+               "grain": self.getAlignment().grainSize,
                "disk": self.partedDisk, "orig_disk": self._origPartedDisk,
                "dev": self.partedDevice})
         return s
@@ -113,8 +115,8 @@ class DiskLabel(DeviceFormat):
         d.update({"labelType": self.labelType,
                   "partitionCount": len(self.partitions),
                   "sectorSize": self.sectorSize,
-                  "offset": self.alignment.offset,
-                  "grainSize": self.alignment.grainSize})
+                  "offset": self.getAlignment().offset,
+                  "grainSize": self.getAlignment().grainSize})
         return d
 
     def updateOrigPartedDisk(self):
@@ -345,45 +347,123 @@ class DiskLabel(DeviceFormat):
     def partitions(self):
         return self.partedDisk.partitions
 
-    @property
-    def alignment(self):
-        """ Alignment requirements for this device. """
-        if not self._alignment:
-            try:
-                disklabel_alignment = self.partedDisk.partitionAlignment
-            except _ped.CreateException:
-                disklabel_alignment = parted.Alignment(offset=0, grainSize=1)
+    def _getDiskLabelAlignment(self):
+        """ Return the disklabel's required alignment for new partitions.
 
+            :rtype: :class:`parted.Alignment`
+        """
+        if not self._diskLabelAlignment:
             try:
-                optimum_device_alignment = self.partedDevice.optimumAlignment
+                self._diskLabelAlignment = self.partedDisk.partitionAlignment
             except _ped.CreateException:
-                optimum_device_alignment = None
+                self._diskLabelAlignment = parted.Alignment(offset=0,
+                                                            grainSize=1)
+
+        return self._diskLabelAlignment
 
+    def _getMinimalAlignment(self):
+        """ Return the device's minimal alignment for new partitions.
+
+            :rtype: :class:`parted.Alignment`
+        """
+        if not self._minimalAlignment:
+            disklabel_alignment = self._getDiskLabelAlignment()
             try:
-                minimum_device_alignment = self.partedDevice.minimumAlignment
+                minimal_alignment = self.partedDevice.minimumAlignment
             except _ped.CreateException:
-                minimum_device_alignment = None
+                # handle this in the same place we'd handle an ArithmeticError
+                minimal_alignment = None
 
             try:
-                a = optimum_device_alignment.intersect(disklabel_alignment)
+                alignment = minimal_alignment.intersect(disklabel_alignment)
             except (ArithmeticError, AttributeError):
+                alignment = disklabel_alignment
+
+            self._minimalAlignment = alignment
+
+        return self._minimalAlignment
+
+    def _getOptimalAlignment(self):
+        """ Return the device's optimal alignment for new partitions.
+
+            :rtype: :class:`parted.Alignment`
+
+            .. note::
+
+                If there is no device-supplied optimal alignment this method
+                returns the minimal device alignment.
+        """
+        if not self._optimalAlignment:
+            disklabel_alignment = self._getDiskLabelAlignment()
+            try:
+                optimal_alignment = self.partedDevice.optimumAlignment
+            except _ped.CreateException:
+                # if there is no optimal alignment, use the minimal alignment,
+                # which has already been intersected with the disklabel
+                # alignment
+                alignment = self._getMinimalAlignment()
+            else:
                 try:
-                    a = minimum_device_alignment.intersect(disklabel_alignment)
-                except (ArithmeticError, AttributeError):
-                    a = disklabel_alignment
+                    alignment = optimal_alignment.intersect(disklabel_alignment)
+                except ArithmeticError:
+                    alignment = disklabel_alignment
+
+            self._optimalAlignment = alignment
+
+        return self._optimalAlignment
 
-            self._alignment = a
+    def getAlignment(self, size=None):
+        """ Return an appropriate alignment for a new partition.
 
-        return self._alignment
+            :keyword size: proposed partition size (optional)
+            :type size: :class:`~.size.Size`
+            :returns: the appropriate alignment to use
+            :rtype: :class:`parted.Alignment`
+            :raises :class:`~.errors.AlignmentError`: if the partition is too
+                                                         small to be aligned
+        """
+        # default to the optimal alignment
+        alignment = self._getOptimalAlignment()
+        if size is None:
+            return alignment
+
+        # use the minimal alignment if the requested size is smaller than the
+        # optimal io size
+        minimal_alignment = self._getMinimalAlignment()
+        optimal_grain_size = Size(alignment.grainSize * self.sectorSize)
+        minimal_grain_size = Size(minimal_alignment.grainSize * self.sectorSize)
+        if size < minimal_grain_size:
+            raise AlignmentError("requested size cannot be aligned")
+        elif size < optimal_grain_size:
+            alignment = minimal_alignment
+
+        return alignment
+
+    def getEndAlignment(self, size=None, alignment=None):
+        """ Return an appropriate end-alignment for a new partition.
+
+            :keyword size: proposed partition size (optional)
+            :type size: :class:`~.size.Size`
+            :keyword alignment: the start alignment (optional)
+            :type alignment: :class:`parted.Alignment`
+            :returns: the appropriate alignment to use
+            :rtype: :class:`parted.Alignment`
+            :raises :class:`~.errors.AlignmentError`: if the partition is too
+                                                         small to be aligned
+        """
+        if alignment is None:
+            alignment = self.getAlignment(size=size)
+
+        return parted.Alignment(offset=alignment.offset - 1,
+                            grainSize=alignment.grainSize)
 
     @property
-    def endAlignment(self):
-        if not self._endAlignment:
-            self._endAlignment = parted.Alignment(
-                                        offset = self.alignment.offset - 1,
-                                        grainSize = self.alignment.grainSize)
+    def alignment(self):
+        return self.getAlignment()
 
-        return self._endAlignment
+    @property
+    def endAlignment(self):
+        return self.getEndAlignment()
 
     @property
     def free(self):
diff --git a/tests/formats_test/disklabel_test.py b/tests/formats_test/disklabel_test.py
new file mode 100644
index 0000000..363e952
--- /dev/null
+++ b/tests/formats_test/disklabel_test.py
@@ -0,0 +1,66 @@
+import parted
+import six
+import unittest
+
+import blivet
+from blivet.size import Size
+
+if six.PY2:
+    import mock
+else:
+    from unittest import mock
+
+patch = mock.patch
+
+class DiskLabelTestCase(unittest.TestCase):
+    @patch("blivet.formats.disklabel.DiskLabel.freshPartedDisk", None)
+    def testGetAlignment(self):
+        dl = blivet.formats.disklabel.DiskLabel()
+        dl._partedDisk = mock.Mock()
+        dl._partedDevice = mock.Mock()
+        dl._partedDevice.sectorSize = 512
+
+        # 512 byte grain sze
+        disklabel_alignment = parted.Alignment(grainSize=1, offset=0)
+        dl._partedDisk.partitionAlignment = disklabel_alignment
+
+        # 1 MiB grain size
+        minimal_alignment = parted.Alignment(grainSize = 2048, offset=0)
+        dl._partedDevice.minimumAlignment = minimal_alignment
+
+        # 4 MiB grain size
+        optimal_alignment = parted.Alignment(grainSize=8192, offset=0)
+        dl._partedDevice.optimumAlignment = optimal_alignment
+
+        # expected end alignments
+        optimal_end_alignment = parted.Alignment(
+                                        grainSize=optimal_alignment.grainSize,
+                                        offset=-1)
+        minimal_end_alignment = parted.Alignment(
+                                        grainSize=minimal_alignment.grainSize,
+                                        offset=-1)
+
+        # make sure the private methods all return the expected values
+        self.assertEqual(dl._getDiskLabelAlignment(), disklabel_alignment)
+        self.assertEqual(dl._getMinimalAlignment(), minimal_alignment)
+        self.assertEqual(dl._getOptimalAlignment(), optimal_alignment)
+
+        # validate result when passing a start alignment to getEndAlignment
+        self.assertEqual(dl.getEndAlignment(alignment=optimal_alignment),
+                                            optimal_end_alignment)
+        self.assertEqual(dl.getEndAlignment(alignment=minimal_alignment),
+                         minimal_end_alignment)
+
+        # by default we should return the optimal alignment
+        self.assertEqual(dl.getAlignment(), optimal_alignment)
+        self.assertEqual(dl.getEndAlignment(), optimal_end_alignment)
+
+        # when passed a size smaller than the optimal io size we should return
+        # the minimal alignment
+        self.assertEqual(dl.getAlignment(size=Size("2 MiB")), minimal_alignment)
+        self.assertEqual(dl.getEndAlignment(size=Size("2 MiB")),
+                         minimal_end_alignment)
+
+        # test the old deprecated properties' values
+        self.assertEqual(dl.alignment, dl._getOptimalAlignment())
+        self.assertEqual(dl.endAlignment, dl.getEndAlignment())
-- 
2.4.3

