Patches for CentOS + Anaconda

[Colin Walters]

I didn't write these patches, but I'm forwarding them here because:

1) I see no reason they shouldn't be upstream, now that CentOS is part of the family
2) We're trying to ship Atomic CentOS, and without these patches, important things like the EFI variable break

See:

https://git.centos.org/tree/!!!!rpms!anaconda.git/84445803224cd926e73643ee36...

[Colin Walters]

My review:

- I think installclasses/centos.py should inherit and override from rhel.py instead of being a copy
- s/class InstallClass/class CentOSInstallClass/
- We should consider a utility function like "if productIsRHELBased(productName)" or something to avoid the lots of if (rhel || centos)
- EULA location will need some dynamic autodetection

[David Shea]

On 01/05/2015 04:59 PM, Colin Walters wrote:
> My review:
> I think installclasses/centos.py should inherit and override from rhel.py instead of being a copy
> s/class InstallClass/class CentOSInstallClass/

I'm with you on those points as far making life easier for whoever 
maintains the CentOS installclass file but since it's their file in 
their tree I don't really have any strong opinions on how they do it.
> We should consider a utility function like "if productIsRHELBased(productName)" or something to avoid the lots of if (rhel || centos)

The message I get from if (rhel || centos) changes is that such 
information should be part of the installclass in the first place. For 
the bootloader fstype, maybe we could check is the installclass 
defaultFS is one of the supported types for the bootloader and reorder 
stage2_format_types to put it first in that case. It shouldn't need to 
be checking the productName.

The other productName.startswith("Red Hat") that I see is for the 
unsupported hardware dialog, and I think if CentOS wants to add that in 
it should be an explicit thing they do and not something we activate 
based on just being rhel-based.

The other conditional I see added in the patch set is to disable 
fastestmirror if centos, and I think they could do that just be leaving 
fastestmirror off of the install media.
> EULA location will need some dynamic autodetection

I don't have any bright ideas about this.

[David Shea]

Now that this email finally decided to wend its way to my inbox, I can 
reply to it, too!

On 01/05/2015 04:55 PM, Colin Walters wrote:
> I didn't write these patches, but I'm forwarding them here because:
> I see no reason they shouldn't be upstream, now that CentOS is part of the family

I don't think a centos installclass needs to be part of upstream 
anaconda. We should provide a means of configuring anaconda for the 
needs of downstream projects without a need to modify anaconda, and an 
installclass can be considered part of that configuration. The Fedora 
products are effectively downstream products now and will be providing 
installclass data in packages separate from anaconda. The same kind of 
thing can work for CentOS. This way Fedora or CentOS or anyone can make 
changes to default filesystems or efi directory names or whatever 
without needing an upstream anaconda change.
> We're trying to ship Atomic CentOS, and without these patches, important things like the EFI variable break

Break how?

[Colin Walters]

On Tue, Jan 6, 2015, at 09:21 AM, David Shea wrote:
> Now that this email finally decided to wend its way to my inbox, I can 
> reply to it, too!
>
> On 01/05/2015 04:55 PM, Colin Walters wrote:
> > I didn't write these patches, but I'm forwarding them here because:
> > I see no reason they shouldn't be upstream, now that CentOS is part of the family
>
> I don't think a centos installclass needs to be part of upstream 
> anaconda.

Ok.  That's reasonable, but the major pain point here is that the install class improvements aren't in the RHEL7.0 Anaconda that is the subject of the patches.

I guess we'll just deal with it until newer versions come down.
> > We're trying to ship Atomic CentOS, and without these patches, important things like the EFI variable break
>
> Break how?

The current state means that when doing Atomic CentOS, we need to merge the CentOS anaconda patchset with the Atomic patchset to get the correct efi_dir.  (But only for UEFI, which is why this bug wasn't more obvious earlier)