On Fri, Apr 24, 2009 at 12:00:41PM -0500, David Lehman wrote: > On Fri, 2009-04-24 at 18:31 +0200, Joel Granados Moreno wrote: > > In the new_partitions list (partitioning.py:617) we must make sure that > > the forced primary partitions appear first so we don't end up trying to > > put a primary partition in an extended one in partitioning.py:696. > > (#493058) > > I don't see how this relates to the bug you referenced, although I > realize you have looked more deeply than I have. Still, I think if this > was a matter of sorting requests we would get a normal failure to > allocate (not enough space or similar). This is actually what I was seeing. Note that comment #1 describes a change in the code. So, currently, there is no traceback. > > It seems at first glance like we are adding multiple devices with the > same path to the device tree, which is a different matter entirely as I > see it. How did you get to partition request sorting from there? If I remember correctly: we use getNextPartitionType to get the type of the next partition that should be added to the disk (partitioning.py:688). With the test case described in the bug, we get to a point where _part.req_primary resolves to True and there are no primary slots left, That is, (new_part_type != parted.PARTITION_NORMAL) resolves to True too. This is in partitioning.py:694. The before mentioned situation occurred when partitioning was on the last partition of the 4 that I tested with. My test consisted in one /boot partition. 2 7G forced primary partition and one swap partition that grew to fill the device. So my immediate question was: Why is the forced primary partition being left for the end instead of the swap (that was not forced). This in turn led me to the partition comparison function. I _do_ see that this might be either in the comparison or in the getNextPartitionType functions. Considering your comments it might be better to place the patch in the getNextPartitionType function. Will study this possibility. > > > --- > > storage/partitioning.py | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/storage/partitioning.py b/storage/partitioning.py > > index 663d1f1..5c028d0 100644 > > --- a/storage/partitioning.py > > +++ b/storage/partitioning.py > > @@ -366,12 +366,12 @@ def partitionCompare(part1, part2): > > # bootable partitions to the front > > ret -= cmp(part1.req_bootable, part2.req_bootable) * 1000 > > > > + # primary-only to the front of the list > > + ret -= cmp(part1.req_primary, part2.req_primary) * 600 > > + > > # more specific disk specs to the front of the list > > ret += cmp(len(part1.parents), len(part2.parents)) * 500 > > > > - # primary-only to the front of the list > > - ret -= cmp(part1.req_primary, part2.req_primary) * 200 > > - > > # larger requests go to the front of the list > > ret -= cmp(part1.req_base_size, part2.req_base_size) * 100 > > > > This makes me very nervous. Even if the change is justified, the way I > chose the scores/weights is such that the sum of all lower scores cannot > overturn a higher score. At the very least you want to maintain this by > shifting the scores accordingly. > understood. > This change would require a great deal of testing to make sure it > doesn't break other cases. We need to be sure it is the best fix for the > problem before we go changing something so central to how partitioning > works. I'll evaluate other solutions and get back to the list. > > Dave > > > _______________________________________________ > Anaconda-devel-list mailing list > Anaconda-devel-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/anaconda-devel-list -- Joel Andres Granados Brno, Czech Republic, Red Hat. _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
