> On Fri, 2012-03-09 at 09:28:06 -0600, David Lehman wrote:
>> On Fri, 2012-03-09 at 08:43 -0600, Mark Hamzy wrote:
>Can we rename this to updatePPCBootList so that common searches for
>ppc-specific stuff will find this? Maybe even updateNVRAMBootList
>instead?
Yes. Although, if I name it updateNVRAMBootList, will your common search
for ppc-specific stuff find it? Will everyone think of nvram as a ppc only
search term?
>> + if len(buf) == 0:
>> + log.error ("FAIL: nvram --print-config=boot-device")
>
>I'd personally prefer a more generic message here like "failed to
>determine nvram boot device". Remember that program.log will contain a
>record of every command run, complete with arguments and output. And,
>since it's already going to have "ERROR" at the beginning of the line in
>the log you could certainly leave out the "FAIL:" part. Likewise for the
>other log statements in this method.
Okay.
>> + return
>
>Installation will continue unless you raise an exception -- is this your
>intention?
Yes. Failure in this function should not abort installation. The bios
can be canged manually after an install.
>> + log.debug("updatePowerPCBootList: boot_disk = %s" % boot_disk)
>
>It will be easy to see what this value is by looking in program.log, so
>this log statement can go.
Okay.
>> + return
>
>Should this be a fatal error? Returning instead of raising an exception
>here means the bootloader installation will continue instead of
>aborting.
Yes. Failure in this function should not abort installation. The bios
can be canged manually after an install.
>> + log.debug("updatePowerPCBootList: updated boot_list = %s" % boot_list)
>
>Just a few lines down this information will get entered into program.log
>as the command line for nvram --update-config so you don't need to log
>it here separately.
Okay.
>> + update_value = "boot-device=%s" % " ".join(boot_list)
>
>Does the argument need to be quoted for the case where boot_list
>contains multiple devices/strings?
Yes. Thanks for catching that.
>> + log.info("Updated PPC boot list with the command: nvram --update-config %s" % update_value)
>
>This second log statement is not needed. It will already be logged in
>program.log.
Okay.
>> + defaults.write("GRUB_TERMINFO=\"%s -g %dx%d %s\"\n" % ("terminfo", 80, 24, "console"))
>
>You might as well just do:
>
> defaults.write("GRUB_TERMINFO=\"terminfo -g 80x24 console\"\n")
I wanted to remind future developers that the specific sub-variables could change.
>> class PPC(Platform):
>> _ppcMachine = iutil.getPPCMachine()
>> - _bootloaderClass = bootloader.Yaboot
>> + _bootloaderClass = bootloader.GRUB2
>> _boot_stage1_device_types = ["partition"]
>
>Is this necessary? It makes the default bootloader on ppc grub2. I
>realize it may not matter if all the ppc platforms that actually get
>used have their own bootloader class definition, but if grub2 can't
>actually boot most ppcs maybe we should leave this as yaboot for now.
Yes. We are removing the need for two boot loaders and moving to only one.
_______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
