On 07/10/2014 11:11 AM, Thomas Neumann wrote:
On Wednesday 09 July 2014 16:28:37 Thomas Lange wrote:
FAI 4.2.1 is now released. It includes several LVM related fixes in
setup-storage.
I just had a look at the patches that have been applied and noticed:
https://github.com/faiproject/fai/commit/aa5d11e0d1a93bc986e2d0c7fbd086e794fe50e1
"Fixes wipefs problem when attempting to preserve lv or vg"
I'm pretty sure the description is wrong. As far as I understand the modified
lines apply only if the logical volume is NOT to be preserved.
That particular block of code in which those modified lines are nested
is only executed if the user attempts to preserve an LV and as such the
description is correct. The commit message doesn't imply that wipefs is
called on an LV that's marked to be preserved.
https://github.com/faiproject/fai/commit/0ecef06a9ab31261b7d090d484c82d35577cd33a
Closes #670614; Improved bugfix by Christian Kreidl
Since it was already requested by Thomas L. to split patches to make them
easier to understand I'm pointing out this one. IMHO this patch should have
been split into an 'improve the code' and 'apply actual fix' part. It adds 13
and removes 21 lines, but I ~think~ the actual fix is removing 6 lines. I'm
unable to say whether the changes to 739-744 are 'bugfix' or 'improvement'.
Valid point. The patch was a bit bigger, because it removed the previous
bugfix and I agree that it should have been divided into at least two
patches.
Best regards
Kerim Gueney
