curtin-dev team mailing list archive

[Olivier Gayot <mp+441769@xxxxxxxxxxxxxxxxxx>]

Olivier Gayot has proposed merging ~ogayot/curtin:flag-logical+swap into curtin:master.

Commit message:
block_meta_v2: don't solely expect flag='logical' for logical partitions

Most of the time, a logical partition has its flag set to 'local'.
However, it some cases, it can also have flag set to 'swap', 'boot' and
possibly other values.

To determine if a partition is actually logical on a DOS partition
table, we need to check the partition number.


Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/441769

In block_meta_v2, we expect logical partitions to have their flag set to 'logical'. However, there are valid scenarios where this is a wrong assumption.

One use-case is the creation of a logical swap partition. This partition would end up with flag='swap' and not flag='logical' ; despite being a logical partition.

Despite being an unsupported use-case in Ubuntu, sometimes a bootable ESP partition is a logical partition.
In curtin's storage_config code, we have the following statement:

 # if the boot flag is set, use this as the flag, logical
 # flag is not required as we can determine logical via
 # partition number

https://git.launchpad.net/curtin/tree/curtin/storage_config.py?id=33411a726532d24e982e29c62f48abd82e2bc4fe#n809
-- 
Your team curtin developers is requested to review the proposed merge of ~ogayot/curtin:flag-logical+swap into curtin:master.

diff --git a/curtin/commands/block_meta_v2.py b/curtin/commands/block_meta_v2.py
index 79dfd39..4ebeb53 100644
--- a/curtin/commands/block_meta_v2.py
+++ b/curtin/commands/block_meta_v2.py
@@ -248,12 +248,24 @@ class DOSPartTable(SFDiskPartTable):
     label = 'dos'
     _extended = None
 
+
     def add(self, action):
+        def is_logical(action) -> bool:
+            flag = action.get('flag', None)
+            if flag == 'logical':
+                return True
+            # In some scenarios, a swap partition can be in the extended
+            # partition. When it does, the flag is set to 'swap'.
+            # In some other scenarios, a bootable partition can also be in the
+            # extended partition. This is not a supported use-case but is
+            # yet another scenario where flag is not set to 'logical'.
+            return action.get('number', 0) > 4
+
         flag = action.get('flag', None)
         start = action.get('offset', None)
         if start is not None:
             start = self.bytes2sectors(start)
-        if flag == 'logical':
+        if is_logical(action):
             if self._extended is None:
                 raise Exception("logical partition without extended partition")
             prev = None