Created attachment 459153 [details]
updates image with proposed patch

Attaching the updates image if anyone wants it.

This request was evaluated by Red Hat Product Management for inclusion
in a Red Hat Enterprise Linux maintenance release. Product Management has 
requested further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed 
products. This request is not yet committed for inclusion in an Update release.

=== In Red Hat Customer Portal Case 00368394 ===
--- Comment by Ruemker, John on 11/9/2010 12:30 PM ---

Actually, on closer inspection, the --percent option did not do what it was supposed to, even with the patch.  The installation proceeded past the point where it normally bombed out, and completed, but the lv takes the entire vg. 

  --- Volume group ---
  VG Name               vg_dhcp52138
  System ID             
  Format                lvm2
  Metadata Areas        1
  Metadata Sequence No  2
  VG Access             read/write
  VG Status             resizable
  MAX LV                0
  Cur LV                1
  Open LV               1
  Max PV                0
  Cur PV                1
  Act PV                1
  VG Size               9.80 GiB
  PE Size               4.00 MiB
  Total PE              2509
  Alloc PE / Size       2509 / 9.80 GiB
  Free  PE / Size       0 / 0   
  VG UUID               l3HFI4-zP33-QorK-1cC5-kVZT-FuTv-c2Mm49

  --- Logical volume ---
  LV Name                /dev/vg_dhcp52138/LogVol01
  VG Name                vg_dhcp52138
  LV UUID                BNvtFb-1Xvv-dp8A-0Kz2-bwm6-kGCO-iReftM
  LV Write Access        read/write
  LV Status              available
  # open                 1
  LV Size                9.80 GiB
  Current LE             2509
  Segments               1
  Allocation             inherit
  Read ahead sectors     auto
  - currently set to     256
  Block device           253:0

=== In Red Hat Customer Portal Case 00368394 ===
--- Comment by Ruemker, John on 11/9/2010 1:12 PM ---

I see the problem.  Immediately after it grows everything, it does:

        # now there shouldn't be any free space left, but if there is we
        # should allocate it to one of the LVs
        vg_free = vg.freeSpace
        log.debug("vg %s has %dMB free" % (vg.name, vg_free))
        if vg_free:
            for lv in lvs:
                if not lv.req_grow:
                    continue

                if lv.req_max_size and lv.size == lv.req_max_size:
                    continue

                if lv.format.maxSize and lv.size == lv.format.maxSize:
                    continue

                # first come, first served
                projected = lv.size + vg.freeSpace
                if lv.req_max_size and projected > lv.req_max_size:
                    projected = lv.req_max_size

                if lv.format.maxSize and projected > lv.format.maxSize:
                    projected = lv.format.maxSize

                log.debug("giving leftover %dMB to %s" % (projected - lv.size,
                                                          lv.name))
                lv.size = projected

Obviously this doesn't allow for any leftover space.  This needs to be changed so people can leave free space in the vg.  It looks like this could easily be changed to check if this lv has a percent specified, and if so leave it alone.  Perhaps:

diff --git a/storage/partitioning.py b/storage/partitioning.py
index e12b3e3..3bf0e53 100644
--- a/storage/partitioning.py
+++ b/storage/partitioning.py
@@ -1798,6 +1798,9 @@ def growLVM(storage):
                 if not lv.req_grow:
                     continue
 
+                if lv.req_percent > 0:
+                    continue
+
                 if lv.req_max_size and lv.size == lv.req_max_size:
                     continue

I just tested this and it did what I expected to after applying this. 

Should this go in another bug? 

-John

Created attachment 459203 [details]
new patch

This patch is inclusive of the previous, and includes a proposed fix for the fact that even with a percent specified, anaconda will grow the lv to take up remaining space in the vg.  When checking for eligible lvs to take up the remaining space, this simply skips the lv if percent > 0.  This could probably be more graceful to see if the percent is somewhere near the size, but I couldn't think of a good way to implement that logic.

Created attachment 459204 [details]
updated updates image with 2nd patch

Thanks for the patch.  This will be included in the next build of anaconda.

updates=http://clumens.fedorapeople.org/percent.img

Created attachment 467772 [details]
new patch (inclusive of previous) that accounts for --percent and --recommended together in same vg

I've tested this patch and it does solve the issues described in the last few comments.  It does so using the logic I described in my last comment.  Specifically, if there is any lv in the vg that uses --percent, it skips the step where it dishes out any remaining space to logvols with --grow.

Created attachment 467773 [details]
new updates image with latest patch

VERIFIED on RHEL6.1-20110330.2 with following kickstart file: http://file.brq.redhat.com/~mbanas/651445.cfg . 
I reproduced the bug on RHEL6.0 and verified it's fixed on RHEL6.1-20110330.2.

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0530.html

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0530.html