Bug 1117908

Summary: missing size argument to kickstart logvol silently fails
Product: Red Hat Enterprise Linux 7 Reporter: Ross Smith <rjsm>
Component: pykickstartAssignee: mulhern <amulhern>
Status: CLOSED ERRATA QA Contact: Release Test Team <release-test-team-automation>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.0CC: clumens, mbanas, pholica, pkotvan
Target Milestone: rc   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: pykickstart-1.99.43.14 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-03-05 08:18:44 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1154068    
Bug Blocks:    
Attachments:
Description Flags
anaconda log with error message.
none
kickstart that produces the error. none

Description Ross Smith 2014-07-09 15:34:48 UTC
Created attachment 916861 [details]
anaconda log with error message.

Description of problem:
Providing a kickstart file that uses LVM and does not specify a --size argument to a logvol command will fail to proceed past the summary hub.  There is no error presented in the gui, beyond "No disks selected".  selecting a disk and automatic partitioning does not allow installation to continue. manually selecting partitions does allow installation to continue.

Version-Release number of selected component (if applicable):
rhel7.0-GA

How reproducible:
Always

Steps to Reproduce:
1. Start an installation with a kickstart that specifies a logvol missing a --size
2. wait for summary hub to display

Actual results:
'no disks selected' warning under partitioning item

Expected results:
critical error message and aborted installation.  Other kickstart file problems produce this result.

Additional info:
error is on line 60-62 of anaconda.log in my attachment.

Comment 1 Ross Smith 2014-07-09 15:35:52 UTC
Created attachment 916862 [details]
kickstart that produces the error.

Comment 3 mulhern 2014-08-19 13:04:45 UTC
The particular error message, "Size required", was changed in master commit 41e1071f7b850a75bfdbc5b2ef109bf301388412.

However, if --grow is specified without a minimum size using --size, a KickstartValueError is raised by LogVolData.execute() consistently in both branches.

That error is clearly logged in the anaconda log. The problem seems to be that in the GUI, and perhaps the TUI, that error is not reflected.

Unfortunately, ksvalidator is not able to detect a problem with the kickstart file.

There is a good argument for making such an error a fatal one, as suggested.

Comment 4 mulhern 2014-08-19 16:25:19 UTC
It's possible to divide kickstart errors into at least two classes:

1) Essentially grammatical ones which ksvalidator can catch,
2) Ones where the kickstart does not match the world it finds itself in.

The missing --size flag is in the second category, because if the lv actually existed the minimum size could be inferred from the current size of the lv.
Hence, this is not an error when the kickstart file is parsed, but only when its commands get matched up with the environment in which it is expected to operate.

Errors in class (2) are not considered fatal, the GUI/TUI allows the user to try to make a save (or quit, of course, if they want to tinker with the kickstart file a little more). That's reasonable, and also a fairly long standing policy, so changing it would be a regression, most likely, hence not an option.

I was not able to reproduce exactly your error, probably because my disks are a bit different, but I think that you should be able to click on the Installation Destination area on the Summary Hub and proceed to the storage spoke.
While there, you should be able to click on the bottom bar where it should say "Error checking storage configuration. Click for details." and that should bring up a dialogue that informs you of the problem with the kickstart file. You are then given the option to either quit or else do some storage configuration via the GUI.

I think it is still an error, however, that the message on the summary screen is "No disks selected." A much better choice would be "Error checking storage configuration."

Comment 5 Ross Smith 2014-08-19 17:59:59 UTC
The documentation [1] doesn't have any indication that --size is optional if the LV already exists. I think this is a type 1 bug.  If logvol doesn't have a --useexisting or --noformat option, it should have a --size option because the kickstart file hasn't indicated that the lv should already exist. I think this should be able to be handled by a syntax checker. 

I think assuming there's a human to intervene when a kickstart file is provided isn't a good default. Typically there's no one to intervene for our installs, or if there is, it's an untrusted person that happened by the system while it was errored.  It's useful when developing a kickstart file, but becomes a problem for us when we put it to production.  Perhaps this is better discussed elsewhere though.

I agree that the error message is wrong on the hub screen and that the proposed change is a much improved choice if the error is not going to be fatal.



[1] https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/7/html/Installation_Guide/sect-kickstart-syntax.html#sect-kickstart-commands

Comment 6 mulhern 2014-08-20 16:48:29 UTC
I concur that it really ought to be a type 1 bug. The information about using size of preexisting device was a red herring.

Two steps for this bug are:

1) Move a chunk of static kickstart verification code into pykickstart and out of anaconda. This means that ksvalidator will now detect a problem and anaconda will terminate immediately with an error.
2) Improve the error message for type 2 errors to "Kickstart information insufficient to complete storage allocation". Note that the status message has to be inferred solely from the state of the storage data structures, there is no history of the failure that got it to this point. So the criterion is using a kickstart file and no root device found and no <some special case kind of messages>.

Note that at this point, the kickstart file is rejected because of the error on
line 4, not line 6.

logvol swap --vgname=vg1 --recommended --maxsize=10240 --name=lv_swap

There is an updates image that works against anaconda version 19.31.79 available at http://mulhern.fedorapeople.org/1117908.img, if you are interested in testing.

Comment 7 Ross Smith 2014-08-25 13:45:03 UTC
Tested, and looks good to me. thanks!

Comment 8 mulhern 2014-09-09 17:07:05 UTC
Acked as well.

Comment 9 mulhern 2014-09-09 18:18:56 UTC
Expect to be able to switch component to pykickstart tomorrow and reobtain bz acks.

Comment 10 mulhern 2014-09-10 11:56:31 UTC
Changed to pykickstart, also notice it's failing in current builds...hmmm.

Comment 12 mulhern 2014-09-11 19:50:41 UTC
anaconda changes also pushed.

Comment 14 mulhern 2014-10-06 14:25:20 UTC
Pending fix on #1149718.

Comment 16 Peter Kotvan 2014-12-05 15:09:02 UTC
Verified on RHEL-7.1-20141204.2 with anaconda-19.31.111-1.el7.

Comment 18 errata-xmlrpc 2015-03-05 08:18:44 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHBA-2015-0353.html