Hi Drew,

On 07/26/11 02:32 AM, Drew Fisher wrote:
Niall,

Aside from Jack's comment you could also use the functools.partial object in solaris_install/__init__.py to replace 556-559:

from solaris_install import run
cmd = ["/usr/sbin/eeprom", "boot-device"]
p =run(cmd)

All the arguments are already set for you.

Ah, this one is new to me.

This is just an observation. You certainly don't have to implement it if you don't want. Just thought I'd show that bit of code to you for future considerations.
I tried it out and it works fine, so I put it in. Thanks for the tip :-)

Thanks again!

Niall

Otherwise, this looks fine to me.

-Drew


On 7/25/11 4:52 AM, Niall Power wrote:
Hi,

I'd like to request a code review for CR  7063508:
Solaris 11 installers wipes out existing boot-device entries in eeprom

Bugster:http://bt2ws.central.sun.com/loadcr.jnlp?jnlp_url=http://bugster.central.sun.com/&arg=7063508
CrPrint:http://bt2ws.central.sun.com/CrPrint?id=7063508
Monaco:http://monaco.sfbay.sun.com/detail.jsf?cr=7063508

The issue is that when setting the openprom boot-device property, it overwrites pre-existing boot-device settings. The fix is to prepend the boot device(s) to the boot-device property rather than clobbering it. Additionally, duplicate is avoided by filterin the existing boot-device list if there are devices that are also in the new boot device(s).

Checked for pep8 and pylint cleanliness.

Webrev:
https://cr.opensolaris.org/action/browse/caiman/niall/7063508

Thanks,
Niall


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to