Merge lp:~blake-rouse/maas/fix-1630667 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Create bios_grub partition on boot disk when the machine is set to PXE boot and the boot disk is larger than 2TiB on amd64.

On i386 machines they cannot use more than a 2TiB disk so this is limited to only amd64. When the boot disk is larger than 2TiB the first partition placed on the disk is now bios_grub flagged partition. Grub will use this partition to place it stage 1 similar to how MBR works. This is only used for PXE booted machines, UEFI machines do not create this partition and still use the /boot/efi partition to place the bootloader.

[Mike Pontillo (mpontillo)]

Looks good to me.

review: Approve

=== modified file 'src/maasserver/models/partition.py'
--- src/maasserver/models/partition.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/partition.py	2016-10-07 15:48:42 +0000
@@ -172,21 +172,30 @@
 
     def get_partition_number(self):
         """Return the partition number in the table."""
+        # Circular imports.
+        from maasserver.models.partitiontable import GPT_REQUIRED_SIZE
         # Sort manually instead of with `order_by`, this will prevent django
         # from making a query if the partitions are already cached.
         partitions_in_table = self.partition_table.partitions.all()
         partitions_in_table = sorted(partitions_in_table, key=attrgetter('id'))
         idx = partitions_in_table.index(self)
         if self.partition_table.table_type == PARTITION_TABLE_TYPE.GPT:
-            # ppc64el machines get part1 skipped when this partition is on
-            # the boot disk. This is because the prep partition is part1 and
-            # is added when the preseed for storage is generated.
+            # In some instances the first partition is skipped because it
+            # is used by the machine architecture for a specific reason.
+            #   * ppc64el - reserved for prep partition
+            #   * amd64 (disk >= 2TiB) - reserved for bios_grub partition
             node = self.get_node()
             arch, _ = node.split_arch()
             boot_disk = node.get_boot_disk()
+            bios_boot_method = node.get_bios_boot_method()
             if (arch == "ppc64el" and
                     self.partition_table.block_device.id == boot_disk.id):
                 return idx + 2
+            elif (arch == "amd64" and
+                    self.partition_table.block_device.id == boot_disk.id and
+                    bios_boot_method != "uefi" and
+                    boot_disk.size >= GPT_REQUIRED_SIZE):
+                return idx + 2
             else:
                 return idx + 1
         elif self.partition_table.table_type == PARTITION_TABLE_TYPE.MBR:

=== modified file 'src/maasserver/models/partitiontable.py'
--- src/maasserver/models/partitiontable.py	2016-10-03 18:57:27 +0000
+++ src/maasserver/models/partitiontable.py	2016-10-07 15:48:42 +0000
@@ -51,6 +51,11 @@
 # is forced on the boot disk unless the disk is larger than 2TiB.
 GPT_REQUIRED_SIZE = 2 * 1024 * 1024 * 1024 * 1024
 
+# The amount of space required to be reserved for the bios_grub partition.
+# bios_grub partition is required on amd64 architectures when grub is used
+# on the boot disk and the disk is larger than GPT_REQUIRED_SIZE.
+BIOS_GRUB_PARTITION_SIZE = 1 * 1024 * 1024  # 1MiB
+
 
 class PartitionTable(CleanSave, TimestampedModel):
     """A partition table on a block device.
@@ -90,6 +95,10 @@
         node_arch, _ = self.block_device.node.split_arch()
         if node_arch == "ppc64el":
             extra_space += PREP_PARTITION_SIZE
+        elif (node_arch == "amd64" and
+                self.block_device.node.bios_boot_method != "uefi" and
+                self.block_device.size >= GPT_REQUIRED_SIZE):
+            extra_space += BIOS_GRUB_PARTITION_SIZE
         return extra_space
 
     def get_used_size(self, ignore_partitions=[]):

=== modified file 'src/maasserver/models/tests/test_partition.py'
--- src/maasserver/models/tests/test_partition.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/tests/test_partition.py	2016-10-07 15:48:42 +0000
@@ -6,7 +6,6 @@
 __all__ = []
 
 import random
-from unittest import skip
 from unittest.mock import sentinel
 from uuid import uuid4
 
@@ -25,6 +24,7 @@
     PARTITION_ALIGNMENT_SIZE,
 )
 from maasserver.models.partitiontable import (
+    BIOS_GRUB_PARTITION_SIZE,
     PARTITION_TABLE_EXTRA_SPACE,
     PREP_PARTITION_SIZE,
 )
@@ -306,10 +306,10 @@
             self.expectThat(idx, Equals(partition.get_partition_number()))
             idx += 1
 
-    @skip("XXX: GavinPanella 2016-04-12 bug=1569365: Fails spuriously.")
     def test_get_partition_number_returns_starting_at_2_for_ppc64el(self):
         node = factory.make_Node(
-            architecture="ppc64el/generic", bios_boot_method="uefi")
+            architecture="ppc64el/generic", bios_boot_method="uefi",
+            with_boot_disk=False)
         block_device = factory.make_PhysicalBlockDevice(
             node=node,
             size=(
@@ -328,6 +328,28 @@
             self.expectThat(idx, Equals(partition.get_partition_number()))
             idx += 1
 
+    def test_get_partition_number_returns_starting_at_2_for_amd64_gpt(self):
+        node = factory.make_Node(
+            architecture="amd64/generic", bios_boot_method="pxe",
+            with_boot_disk=False)
+        block_device = factory.make_PhysicalBlockDevice(
+            node=node,
+            size=(
+                (2 * (1024 ** 4)) + PARTITION_TABLE_EXTRA_SPACE +
+                BIOS_GRUB_PARTITION_SIZE))
+        node.boot_disk = block_device
+        node.save()
+        partition_table = factory.make_PartitionTable(
+            block_device=block_device, table_type=PARTITION_TABLE_TYPE.GPT)
+        partitions = [
+            partition_table.add_partition(size=MIN_BLOCK_DEVICE_SIZE)
+            for _ in range(4)
+        ]
+        idx = 2
+        for partition in partitions:
+            self.expectThat(idx, Equals(partition.get_partition_number()))
+            idx += 1
+
     def test_get_partition_number_returns_correct_numbering_for_mbr(self):
         block_device = factory.make_PhysicalBlockDevice(
             size=(MIN_BLOCK_DEVICE_SIZE * 6) + PARTITION_TABLE_EXTRA_SPACE)

=== modified file 'src/maasserver/models/tests/test_partitiontable.py'
--- src/maasserver/models/tests/test_partitiontable.py	2016-10-03 18:57:27 +0000
+++ src/maasserver/models/tests/test_partitiontable.py	2016-10-07 15:48:42 +0000
@@ -20,6 +20,7 @@
     PARTITION_ALIGNMENT_SIZE,
 )
 from maasserver.models.partitiontable import (
+    BIOS_GRUB_PARTITION_SIZE,
     PARTITION_TABLE_EXTRA_SPACE,
     PREP_PARTITION_SIZE,
 )
@@ -59,6 +60,20 @@
                 False),
             partition_table.get_size())
 
+    def test_get_size_returns_block_device_size_minus_amd64_gpt(self):
+        node = factory.make_Node(architecture="amd64/generic")
+        block_device = factory.make_PhysicalBlockDevice(
+            node=node, size=2 * (1024 ** 4))
+        partition_table = factory.make_PartitionTable(
+            block_device=block_device)
+        self.assertEqual(
+            round_size_to_nearest_block(
+                partition_table.block_device.size -
+                PARTITION_TABLE_EXTRA_SPACE - BIOS_GRUB_PARTITION_SIZE,
+                PARTITION_ALIGNMENT_SIZE,
+                False),
+            partition_table.get_size())
+
     def test_get_block_size_returns_block_device_block_size(self):
         partition_table = factory.make_PartitionTable()
         self.assertEqual(
@@ -148,6 +163,16 @@
             PARTITION_TABLE_EXTRA_SPACE + PREP_PARTITION_SIZE,
             partition_table.get_overhead_size())
 
+    def test_get_overhead_size_for_amd64_gpt(self):
+        node = factory.make_Node(architecture="amd64/generic")
+        block_device = factory.make_PhysicalBlockDevice(
+            node=node, size=2 * (1024 ** 4))
+        partition_table = factory.make_PartitionTable(
+            block_device=block_device)
+        self.assertEquals(
+            PARTITION_TABLE_EXTRA_SPACE + BIOS_GRUB_PARTITION_SIZE,
+            partition_table.get_overhead_size())
+
     def test_get_available_size(self):
         block_size = 4096
         device = factory.make_BlockDevice(

=== modified file 'src/maasserver/preseed_storage.py'
--- src/maasserver/preseed_storage.py	2016-10-04 18:37:44 +0000
+++ src/maasserver/preseed_storage.py	2016-10-07 15:48:42 +0000
@@ -17,6 +17,7 @@
 )
 from maasserver.models.partition import Partition
 from maasserver.models.partitiontable import (
+    BIOS_GRUB_PARTITION_SIZE,
     GPT_REQUIRED_SIZE,
     INITIAL_PARTITION_OFFSET,
     PARTITION_TABLE_EXTRA_SPACE,
@@ -118,6 +119,15 @@
             self.boot_disk.id == block_device.id and
             arch == "ppc64el")
 
+    def _requires_bios_grub_partition(self, block_device):
+        """Return True if block device requires the bios_grub partition."""
+        arch, _ = self.node.split_arch()
+        bios_boot_method = self.node.get_bios_boot_method()
+        return (
+            arch == "amd64" and
+            bios_boot_method != "uefi" and
+            block_device.size >= GPT_REQUIRED_SIZE)
+
     def _add_partition_operations(self):
         """Add all the partition operations.
 
@@ -126,14 +136,16 @@
         """
         for block_device in self.node.blockdevice_set.order_by('id'):
             requires_prep = self._requires_prep_partition(block_device)
+            requires_bios_grub = self._requires_bios_grub_partition(
+                block_device)
             partition_table = block_device.get_partitiontable()
             if partition_table is not None:
                 partitions = list(partition_table.partitions.order_by('id'))
                 for idx, partition in enumerate(partitions):
-                    # If this is the last partition and prep partition is
-                    # required then set boot_disk_first_partition so extra
-                    # space can be removed.
-                    if requires_prep and idx == 0:
+                    # If this is the first partition and prep or bios_grub
+                    # partition is required then set boot_disk_first_partition
+                    # so partition creation can occur in the correct order.
+                    if (requires_prep or requires_bios_grub) and idx == 0:
                         self.boot_disk_first_partition = partition
                     self.operations["partition"].append(partition)
 
@@ -195,6 +207,7 @@
         # Set the partition table type if a partition table exists or if this
         # is the boot disk.
         add_prep_partition = False
+        add_bios_grub_partition = False
         partition_table = block_device.get_partitiontable()
         if partition_table is not None:
             disk_operation["ptable"] = self._get_ptable_type(
@@ -207,8 +220,10 @@
                 disk_operation["ptable"] = "gpt"
                 if node_arch == "ppc64el":
                     add_prep_partition = True
-            elif block_device.size >= GPT_REQUIRED_SIZE:
+            elif (block_device.size >= GPT_REQUIRED_SIZE and
+                    node_arch == "amd64"):
                 disk_operation["ptable"] = "gpt"
+                add_bios_grub_partition = True
             else:
                 disk_operation["ptable"] = "msdos"
 
@@ -221,10 +236,16 @@
             disk_operation["grub_device"] = True
         self.storage_config.append(disk_operation)
 
-        # Add the prep partition at the end of the disk when it is required.
+        # Add the prep partition at the beginning of the disk
+        # when it is required.
         if add_prep_partition:
             self._generate_prep_partition(block_device.get_name())
 
+        # Add the bios_grub partition at the beginning of the disk
+        # when it is required.
+        if add_bios_grub_partition:
+            self._generate_bios_grub_partition(block_device.get_name())
+
     def _get_ptable_type(self, partition_table):
         """Return the value for the "ptable" entry in the physical operation.
         """
@@ -254,6 +275,20 @@
         }
         self.storage_config.append(partition_operation)
 
+    def _generate_bios_grub_partition(self, device_name):
+        """Generate the bios_grub partition at the beginning of the device."""
+        partition_operation = {
+            "id": "%s-part1" % (device_name),
+            "type": "partition",
+            "number": 1,
+            "offset": "%dB" % INITIAL_PARTITION_OFFSET,
+            "size": "%dB" % BIOS_GRUB_PARTITION_SIZE,
+            "device": device_name,
+            "wipe": "zero",
+            "flag": "bios_grub",
+        }
+        self.storage_config.append(partition_operation)
+
     def _generate_partition_operations(self):
         """Generate all partition operations."""
         for partition in self.operations["partition"]:
@@ -261,7 +296,16 @@
                 # This is the first partition in the boot disk and add prep
                 # partition at the beginning of the partition table.
                 device_name = partition.partition_table.block_device.get_name()
-                self._generate_prep_partition(device_name)
+                if self._requires_prep_partition(
+                        partition.partition_table.block_device):
+                    self._generate_prep_partition(device_name)
+                elif self._requires_bios_grub_partition(
+                        partition.partition_table.block_device):
+                    self._generate_bios_grub_partition(device_name)
+                else:
+                    raise ValueError(
+                        "boot_disk_first_partition set when prep and "
+                        "bios_grub partition are not required.")
                 self._generate_partition_operation(
                     partition, include_initial=False)
             else:

=== modified file 'src/maasserver/tests/test_preseed_storage.py'
--- src/maasserver/tests/test_preseed_storage.py	2016-10-03 18:57:27 +0000
+++ src/maasserver/tests/test_preseed_storage.py	2016-10-07 15:48:42 +0000
@@ -22,6 +22,7 @@
     VolumeGroup,
 )
 from maasserver.models.partitiontable import (
+    BIOS_GRUB_PARTITION_SIZE,
     PARTITION_TABLE_EXTRA_SPACE,
     PREP_PARTITION_SIZE,
 )
@@ -546,6 +547,14 @@
             ptable: gpt
             path: /dev/disk/by-id/wwn-0x55cd2e400009bf84
             grub_device: true
+          - id: sdb-part1
+            type: partition
+            number: 1
+            size: 1048576B
+            device: sdb
+            wipe: zero
+            offset: 4194304B
+            flag: bios_grub
           - id: sda-part1
             name: sda-part1
             type: partition
@@ -570,13 +579,14 @@
 
     def test__renders_expected_output(self):
         node = factory.make_Node(
-            status=NODE_STATUS.ALLOCATED, with_boot_disk=False)
+            status=NODE_STATUS.ALLOCATED, architecture="amd64/generic",
+            with_boot_disk=False)
         first_disk = factory.make_PhysicalBlockDevice(
             node=node, size=8 * 1024 ** 3, name="sda",
             model="QEMU HARDDISK", serial="QM00001")  # 8 GiB
         boot_disk = factory.make_PhysicalBlockDevice(
-            node=node, size=8 * 1024 ** 4, name="sdb",
-            id_path="/dev/disk/by-id/wwn-0x55cd2e400009bf84")
+            node=node, size=2 * 1024 ** 4, name="sdb",
+            id_path="/dev/disk/by-id/wwn-0x55cd2e400009bf84")  # 2 TiB
         node.boot_disk = boot_disk
         node.save()
         partition_table = factory.make_PartitionTable(
@@ -596,6 +606,77 @@
         self.assertStorageConfig(self.STORAGE_CONFIG, config)
 
 
+class TestGPTPXELargeBootDiskLayout(
+        MAASServerTestCase, AssertStorageConfigMixin):
+
+    STORAGE_CONFIG = dedent("""\
+        config:
+          - id: sda
+            name: sda
+            type: disk
+            wipe: superblock
+            ptable: gpt
+            model: QEMU HARDDISK
+            serial: QM00001
+            grub_device: true
+          - id: sda-part1
+            type: partition
+            number: 1
+            size: 1048576B
+            device: sda
+            wipe: zero
+            offset: 4194304B
+            flag: bios_grub
+          - id: sda-part2
+            name: sda-part2
+            type: partition
+            number: 2
+            uuid: 6efc2c3d-bc9d-4ee5-a7ed-c6e1574d5398
+            size: 8581545984B
+            device: sda
+            wipe: superblock
+          - id: sda-part2_format
+            type: format
+            fstype: ext4
+            label: root
+            uuid: 90a69b22-e281-4c5b-8df9-b09514f27ba1
+            volume: sda-part2
+          - id: sda-part2_mount
+            type: mount
+            path: /
+            options: rw,relatime,errors=remount-ro,data=journal
+            device: sda-part2_format
+        """)
+
+    def test__renders_expected_output(self):
+        node = factory.make_Node(
+            status=NODE_STATUS.ALLOCATED, architecture="amd64/generic",
+            with_boot_disk=False)
+        boot_disk = factory.make_PhysicalBlockDevice(
+            node=node, size=2 * 1024 ** 4, name="sda",
+            model="QEMU HARDDISK", serial="QM00001")  # 2 TiB
+        node.boot_disk = boot_disk
+        node.save()
+        partition_table = factory.make_PartitionTable(
+            table_type=PARTITION_TABLE_TYPE.GPT, block_device=boot_disk)
+        root_partition = factory.make_Partition(
+            partition_table=partition_table,
+            uuid="6efc2c3d-bc9d-4ee5-a7ed-c6e1574d5398",
+            size=(
+                (8 * 1024 ** 3) -
+                PARTITION_TABLE_EXTRA_SPACE -
+                BIOS_GRUB_PARTITION_SIZE),
+            bootable=False)
+        factory.make_Filesystem(
+            partition=root_partition, fstype=FILESYSTEM_TYPE.EXT4,
+            uuid="90a69b22-e281-4c5b-8df9-b09514f27ba1", label="root",
+            mount_point="/", mount_options=(
+                "rw,relatime,errors=remount-ro,data=journal"))
+        node._create_acquired_filesystems()
+        config = compose_curtin_storage_config(node)
+        self.assertStorageConfig(self.STORAGE_CONFIG, config)
+
+
 class TestComplexDiskLayout(
         MAASServerTestCase, AssertStorageConfigMixin):