storage: when handling a partition, ignore any FS at the disk level

[ogayot]

The parts_and_gaps function returned no partition nor gap when the disk itself has (or had) a file-system signature. This is to make sure we don't partition a disk that is already used unpartioned.

However, when we are operating on an existing partition of a disk, we should not pretend that the underlying disk has no partition ; even if the disk itself has a FS signature. This leads to a common issue when doing GuidedTargetReformat:

  Traceback (most recent call last):
    File "subiquity/common/api/server.py", line 164, in handler
      result = await implementation(**args)
    File "subiquity/server/controllers/filesystem.py", line 1184, in v2_guided_POST
      await self.guided(data)
    File "subiquity/server/controllers/filesystem.py", line 726, in guided
      gap = self.start_guided(choice.target, disk)
    File "/usr/lib/python3.10/functools.py", line 926, in _method
      return method.__get__(obj, cls)(*args, **kwargs)
    File "subiquity/server/controllers/filesystem.py", line 645, in start_guided_reformat
      self.reformat(disk, wipe="superblock-recursive")
    File "subiquity/common/filesystem/manipulator.py", line 257, in reformat
      self.delete_partition(p, True)
    File "subiquity/common/filesystem/manipulator.py", line 120, in delete_partition
      self.model.remove_partition(part)
    File "subiquity/models/filesystem.py", line 2110, in remove_partition
      for p2 in movable_trailing_partitions_and_gap_size(part)[0]:
    File "/usr/lib/python3.10/functools.py", line 889, in wrapper
      return dispatch(args[0].__class__)(*args, **kw)
    File "subiquity/common/filesystem/gaps.py", line 281, in
         _movable_trailing_partitions_and_gap_size_partition
      part_idx = pgs.index(partition)
  ValueError: Partition(device=disk-sda, ...) is not in list

Using this change, I was able to proceed with a Guided-Reformat installation on a disk that had a ZFS signature + partitions. The solution is not perfect though. Going through manual partitioning with such as disk also causes errors:

• the UI does not show the list of existing partitions
• clicking on "New partition table" causes a ValueError: .... is not in list exception.

Another approach would be to treat a disk that has a filesystem signature + partitions as a normal partitioned disk, which would translate to using:

 def parts_and_gaps_disk(device):
-    if device._fs is not None:
+    if device._fs is not None and not device.partitions():
         return []

Suggestions are welcome!

LP:#2081738

[dbungert]

I'm not opposed to this PR - I think it's net improvement - but I suspect the latter part of your PR description is correct.

 def parts_and_gaps_disk(device):
-    if device._fs is not None:
+    if device._fs is not None and not device.partitions():
         return []

If a device has partitions, I think a FS signature is irrelevant. Am I oversimplifying there?

[ogayot]

If a device has partitions, I think a FS signature is irrelevant. Am I oversimplifying there?

I think in theory it's mostly irrelevant yes.

In practice, I assume the change would have some impact on how we handle installation medias since they typically have an iso9660 signature + partitions to them. I'm not sure what would be the impact TBH.

There's also this side-effect which I was thinking about. If we have one disk with a FS signature and partitions, part_and_gaps would return values. If we decide to remove the partitions, then we suddenly have a disk with a FS signature and no partitions, so parts_and_gaps will return no values.
If we go this direction, maybe we need Subiquity to remember that the disk was initially partitioned (and therefore can be partitioned again).

I'm tempted to land the content of this PR and and have a deeper look when we tackle manual partitioning. Wdtyt?

[dbungert]

I'm tempted to land the content of this PR and and have a deeper look when we tackle manual partitioning. Wdtyt?

Go for it.