These lvm, clamp things really bother me.....
Have some questions about the patch... I would just like some additional explination in some parts.
1.
@@ -161,12 +168,38 @@ def vgremove(vgname):
.
args = ["lvm", "vgremove", vgname]
.
+ log(string.join(args, ' '))
rc = iutil.execWithRedirect(args[0], args,
stdout = output,
stderr = output,
searchPath = 1)
if rc:
raise SystemError, "vgremove failed"
+ # now iterate all the PVs we've just freed up, so we reclaim the metadata
+ # space. This is an LVM bug, AFAICS.
+ for pvname in pvs:
+ args = ["lvm", "pvremove", pvname]
+
+ log(string.join(args, ' '))
+ rc = iutil.execWithRedirect(args[0], args,
+ stdout = output,
+ stderr = output,
+ searchPath = 1)
+
+ if rc:
+ raise SystemError, "pvremove failed"
+
+ args = ["lvm", "pvcreate", "-ff", "-y", "-v", pvname]
+
+ log(string.join(args, ' '))
+ rc = iutil.execWithRedirect(args[0], args,
+ stdout = output,
+ stderr = output,
+ searchPath = 1)
+
+ if rc:
+ raise SystemError, "pvcreate failed for %s" % (pvname,)
+
So let me get this strait. When we remove a vg we also remove the pv that the vg was in. only to create the pv again? what the deal. Whats the failure?
2.
@@ -409,5 +421,10 @@ def getVGUsedSpace(vgreq, requests, diskset):
.
def getVGFreeSpace(vgreq, requests, diskset):
used = getVGUsedSpace(vgreq, requests, diskset)
+ log("used space is %s" % (used,))
+.....
+ total = vgreq.getActualSize(requests, diskset)
+ log("actual space is %s" % (total,))
+ return total - used
.....
return vgreq.getActualSize(requests, diskset) - used
Do we ever reach this second return ^^^^
3.
@@ -752,21 +752,19 @@ class VolumeGroupRequestSpec(RequestSpec):
def getActualSize(self, partitions, diskset):
"""Return the actual size allocated for the request in megabytes."""
.
- # this seems like a bogus check too...
- if self.physicalVolumes is None:
- return 0
-
# if we have a preexisting size, use it
if self.preexist and self.preexist_size:
- totalspace = ((self.preexist_size / self.pesize) *
- self.pesize)
+ totalspace = lvm.clampPVSize(self.preexist_size, self.pesize)
else:
totalspace = 0
for pvid in self.physicalVolumes:
pvreq = partitions.getRequestByID(pvid)
size = pvreq.getActualSize(partitions, diskset)
- size = lvm.clampPVSize(size, self.pesize)
- totalspace = totalspace + size
+ #log("size for pv %s is %s" % (pvid, size))
^^^^^ can we erase this?
+ clamped = lvm.clampPVSize(size, self.pesize)
+ log(" got pv.size of %s, clamped to %s" % (size,clamped))
+ #log(" clamped size is %s" % (size,))
^^^^^ can we erase this?
+ totalspace = totalspace + clamped
.
For the rest of the patch I see a lot of clamping going on, Which I guess is to avoid a miscalculation in the pvs. In the worst case scenario we will be creating a vg/lv/partition that does not *completely* occupy the pv. Additionally, It very complex to truly see the extent of the patch in the whole partitioning code. In this case I would put my vote on "commit and let the automatic testing catch stuff that has been overlooked".
When the comment above are addressed/answered, I would give it a go ahead :)
----- "Radek Vykydal" <rvykydal@xxxxxxxxxx> wrote:
> Hi,
>
> lvm partitioning (clamping of PV size) again, this time for 4.8.
> This is fix for 4.8 #233050. It required some commits to be
> ported from 5.3:
>
> commit cc9d0f4e57b11a37fc5d33d0374509e43a97840c
> commit 8b4c702d0c2c6130c5263a4944405efa1301ced9
> commit 4f9c6e49d113a88a28c55c51bb5eab6ad756612b
> commit a68ab7d2823836ee90171cf419864e248ad99ce7
> commit 4aa9ca1c35b867fa5a4d94c41591700ca7ab5edb
> (5.3 #415871)
> commit 08233b0c42f8b453ff7d8bde03c9adc57d92d7ed
> (5.3 #463780)
>
> I tested the patch with reproducer from bz and reproducers
> from #415871 and #463780 which are the last two commits
> of the list above.
>
> Note: I didn't port patches fixing other corner cases hit and
> fixed in 5.3:
> - wiping old lvm metadata
> https://bugzilla.redhat.com/show_bug.cgi?id=468431
> https://bugzilla.redhat.com/show_bug.cgi?id=469700
> - handling corner case when clamped size == partition size
> https://bugzilla.redhat.com/show_bug.cgi?id=468944
> - computing size of preexisting VG in UI
> https://bugzilla.redhat.com/show_bug.cgi?id=217913
> - lvm on raid chunk alignment
> https://bugzilla.redhat.com/show_bug.cgi?id=463431
>
> Radek
>
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
--
Joel Andres Granados
Red Hat / Brno Czech Republic
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
