[Bug 1991929] Re: Using size: -1 in autoinstall results in wrong computation of gaps

[Olivier Gayot]

** Changed in: subiquity (Ubuntu)
       Status: In Progress => Fix Committed

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to subiquity in Ubuntu.
https://bugs.launchpad.net/bugs/1991929

Title:
  Using size: -1 in autoinstall results in wrong computation of gaps

Status in subiquity package in Ubuntu:
  Fix Committed

Bug description:
  Specifying a negative size for a partition in autoinstall is supported
  in a specific scenario:

  Quote from https://ubuntu.com/server/docs/install/autoinstall-reference:
  For the last partition specified for a particular device, you can specify the size as “-1” to indicate that the partition should fill the remaining space.

  In practice, Subiquity considers that all negative values should
  result in the partition to fill the remaining space.

  There is one problem with our implementation though.
  When a negative size is specified, Subiquity calls largest_gap(disk), which in turns calls parts_and_gaps(disk), to find out what size the partition should be.
  But this function loops through all partitions of the disk, including the one that at this point still has a negative size.
  There is no check for negative sizes in parts_and_gaps, so the computations are affected by the negative size and so is the result.

  The issue is obvious when a large negative size is specified (which is
  not a supported use-case), but having a size of -1 also influences the
  computations (and this one IS a supported use-case).

  Simple use-case:

          make_disk(model, serial='aaaa', size=dehumanize_size("100M"))
          fake_up_blockdata(model)
          model.apply_autoinstall_config([
              {
                  'type': 'disk',
                  'id': 'disk0',
              },
              {
                  'type': 'partition',
                  'id': 'part0',
                  'device': 'disk0',
                  'size': dehumanize_size('50M'),
              },
              {
                  'type': 'partition',
                  'id': 'part1',
                  'device': 'disk0',
                  'size': -50000000,    # Large negative number
              },
              ])
          disk = model._one(type="disk")
          part1 = model._one(type="partition", id="part1")
  >       self.assertEqual(
              part1.size, disk.available_for_partitions - dehumanize_size('50M'))
  E       AssertionError: 99614720 != 50331648

  If we want to determine the size of a partition, we should make sure
  to exclude it when listing the available gaps.

  As a side note, I think it is questionable choice to use -1 as the
  value to indicate that the partition should fill the remaining space.
  I would prefer to use a sentinel value of a different type (e.g. the
  string "fill" in the YAML) so we don't accidentally interpret a
  negative value as a real size.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/subiquity/+bug/1991929/+subscriptions