[PATCH 3/3] Make sure autopart requests fit in somewhere (#978266)

David Lehman dlehman at redhat.com
Wed Sep 17 14:23:30 UTC 2014


On 09/17/2014 02:14 AM, Vratislav Podzimek wrote:
> When doing LVM/BTRFS autopart we implicitly schedule one partition on each disk
> as a device for LVM/BTRFS. Those implicitly scheduled partitions are requested
> to be at least 500 MiB big and grow if possible. That's fine and works well
> until we have many small disks (e.g. DASDs) and need to fit another partition
> (for e.g. swap) that is in combination with the 500MiB implicit partition bigger
> than any disk we have.
>
> Thus we need to make sure all our partition requests fit somewhere with the
> implicitly scheduled partitions by making the latter ones smaller or by removing
> them if needed.

I still think this is a nonsense use-case and we should not be spending 
time on it. Six DASDs of 2GiB each is just silly. Since I realize people 
actually care about this, I will grudgingly move past the silliness.

What about changing the swap request to one that is growable with a 
small base size so it can adapt using existing code?

>
> Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> ---
>   blivet/partitioning.py | 32 ++++++++++++++++++++++++++++----
>   1 file changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/blivet/partitioning.py b/blivet/partitioning.py
> index 9e45c1a..cddeb14 100644
> --- a/blivet/partitioning.py
> +++ b/blivet/partitioning.py
> @@ -119,7 +119,7 @@ def _scheduleImplicitPartitions(storage, disks):
>
>       return devs
>
> -def _schedulePartitions(storage, disks):
> +def _schedulePartitions(storage, disks, implicit_devices):
>       """ Schedule creation of autopart partitions.
>
>           This only schedules the requests for actual partitions.
> @@ -236,8 +236,32 @@ def _schedulePartitions(storage, disks):
>                                     parents=dev)
>               storage.createDevice(luks_dev)
>
> -    # make sure preexisting broken lvm/raid configs get out of the way
> -    return
> +        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
> +            biggest_free = all_free[0]
> +            smallest_implicit = sorted(implicit_devices, key=lambda d: d.size)[0]
> +            if (request.size + smallest_implicit.size) > biggest_free:
> +                # 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
> +                new_size = biggest_free - request.size
> +
> +                # subtract the size from the biggest free region and reorder the
> +                # list
> +                biggest_free -= request.size
> +                all_free.sort(reverse=True)
> +

This seems like it might have limitations due to the fact that all_free 
is not updated as you account for each partition.

> +                # cannot set size of a non-existing partition, just destroy it
> +                # and create new (if it makes sense)
> +                implicit_devices.remove(smallest_implicit)
> +                storage.destroyDevice(smallest_implicit)
> +                if new_size > Size(0):
> +                    new_dev = _createImplicitPartition(storage, size=new_size, grow=False)
> +                    implicit_devices.append(new_dev)

You could replace the above block (starting with the comment) with this:

if new_size > Size(0):
     smallest_implicit._size = new_size
     smallest_implicit.req_size = new_size
     smallest_implicit.req_base_size =  new_size
     smallest_implicit.req_grow = False

Or you could implement _setSize for non-existing partitions. Replacing 
the device just seems a bit wasteful.

> +
> +    return implicit_devices
>
>   def _scheduleVolumes(storage, devs):
>       """ Schedule creation of autopart lvm/btrfs volumes.
> @@ -390,7 +414,7 @@ def doAutoPartition(storage, data):
>           raise NotEnoughFreeSpaceError(_("Not enough free space on disks for "
>                                         "automatic partitioning"))
>
> -    _schedulePartitions(storage, disks)
> +    devs = _schedulePartitions(storage, disks, devs)
>
>       # run the autopart function to allocate and grow partitions
>       doPartitioning(storage)
>



More information about the anaconda-patches mailing list