[blivet][master/rhel7-branch][PATCH] Shrink the last of the biggest implicit partitions if needed (#1171116)

David Lehman dlehman at redhat.com
Tue Jan 20 16:46:45 UTC 2015


On 01/09/2015 09:45 AM, Vratislav Podzimek wrote:
> If there is a big partition requested to be allocated on a disk with basically
> the same size (e.g. 1.8 GiB swap on a 2 GiB DASD with BTRFS autopart) we need to
> make one of the implicit partitions smaller (they default to 500 MiB) to fit
> into the disk. We choose the last of the biggest ones because the biggest ones
> have the biggest chance to "survive" shrinking and last of them won't collide
> with /boot being on the same disk (there's one implicit partition per disk so
> the first one will likely end up on the same disk as /boot when doing autopart).
>
> Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> ---
>   blivet/partitioning.py | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> index 9f6eea8..ff744cf 100644
> --- a/blivet/partitioning.py
> +++ b/blivet/partitioning.py
> @@ -130,6 +130,7 @@ def _schedulePartitions(storage, disks, implicit_devices, min_luks_entropy=0):
>           :returns: None
>           :rtype: None
>       """
> +
>       # basis for requests with requiredSpace is the sum of the sizes of the
>       # two largest free regions
>       all_free = (Size(reg.getLength(unit="B")) for reg in getFreeRegions(disks))
> @@ -238,10 +239,16 @@ def _schedulePartitions(storage, disks, implicit_devices, min_luks_entropy=0):
>
>           if storage.autoPartType in (AUTOPART_TYPE_LVM, AUTOPART_TYPE_LVM_THINP,
>                                       AUTOPART_TYPE_BTRFS):
> -            # doing LVM/BTRFS -- make sure the newly created partition fits in some
> -            # free space together with one of the implicitly requested partitions
> -            smallest_implicit = sorted(implicit_devices, key=lambda d: d.size)[0]
> -            if (request.size + smallest_implicit.size) > all_free[0]:
> +            # doing LVM/BTRFS -- make sure the newly created partition fits in
> +            # some free space together with one of the implicitly requested
> +            # partitions (the last of the biggest ones because the biggest ones
> +            # have the biggest chance to "survive" shrinking and last of them
> +            # won't collide with /boot being on the same disk)
> +            # HINT: sorted().reverse() is different from sorted(reverse=True)
> +            sorted_implicits = sorted(implicit_devices, key=lambda d: d.size)
> +            sorted_implicits.reverse()
> +            biggest_last_implicit = sorted_implicits[0]
> +            if (request.size + biggest_last_implicit.size) > all_free[0]:

This all seems a bit fragile. I think we'd have been better off giving 
the member devices minimal base sizes once we identify this situation to 
use existing logic instead of adding all this.

>                   # not enough space to allocate the smallest implicit partition
>                   # and the request, make the implicit partition smaller with
>                   # fixed size in order to make space for the request
> @@ -253,10 +260,10 @@ def _schedulePartitions(storage, disks, implicit_devices, min_luks_entropy=0):
>                   all_free.sort(reverse=True)
>
>                   if new_size > Size(0):
> -                    smallest_implicit.size = new_size
> +                    biggest_last_implicit.size = new_size / 2

I know this isn't what you're changing now, but this should really be 
checking for more than > 0, especially if you're about to divide it in 
half. Shouldn't you be checking something like

   new_size > biggest_last_implicit.format.minSize * 2

instead?

>                   else:
> -                    implicit_devices.remove(smallest_implicit)
> -                    storage.destroyDevice(smallest_implicit)
> +                    implicit_devices.remove(biggest_last_implicit)
> +                    storage.destroyDevice(biggest_last_implicit)
>
>       return implicit_devices
>
>



More information about the anaconda-patches mailing list