[PATCH 2/2] Hook formatByDefault back up to the storage spoke.

David Shea dshea at redhat.com
Thu Jan 22 21:48:18 UTC 2015


On 01/22/2015 04:16 PM, Adam Williamson wrote:
> On Wed, 2015-01-21 at 17:58 -0500, David Shea wrote:
>> Disallow the reuse of /boot and /var and some other things.
>> ---
>>   pyanaconda/ui/gui/spokes/custom.py | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/pyanaconda/ui/gui/spokes/custom.py
>> b/pyanaconda/ui/gui/spokes/custom.py
>> index 2866011..fc1e7ad 100644
>> --- a/pyanaconda/ui/gui/spokes/custom.py
>> +++ b/pyanaconda/ui/gui/spokes/custom.py
>> @@ -654,8 +654,8 @@ class CustomPartitioningSpoke(NormalSpoke,
>> StorageChecker):
>>               error = _("%s cannot be encrypted") % mountpoint
>>           elif encrypted and new_fs_type in
>> PARTITION_ONLY_FORMAT_TYPES:
>>               error = _("%s cannot be encrypted") % new_fs_type
>> -        elif mountpoint == "/" and device.format.exists and not
>> reformat:
>> -            error = _("You must create a new file system on the
>> root device.")
>> +        elif not reformat and not
>> self.storage.formatByDefault(device):
>> +            error = _("You must create a new file system on the %s
>> device.") % mountpoint
> Isn't that the wrong way around? By my read, formatByDefault returns
> true if the device must be reformatted, so it should be:
>
> elif not reformat and self.storage.formatByDefault(device):
>
> you also seem to have dropped the 'and device.format.exists' condition
> - is that not needed?

Yeah. Additional fun facts: formatByDefault does not check /, and it was 
always returning false anyway since format.mountpoint isn't set here. So 
this needs some work.

>
> btw, wouldn't it be somewhat nicer UI to grey out the format checkbox
> for mount points that must be reformatted and add a little note or
> tooltip explaining the situation, rather than letting people create an
> invalid configuration then raising an error?

Not doing any UI updates until the user hits the update button is 
consistent with the rest of how custom storage works, and given how many 
fiddlable bits there are on the mountpoint input I thought we had 
settled on that being the best policy.



More information about the anaconda-patches mailing list