Thanks Jan.
This looks good now.
Joe
On 11/10/10 08:30 AM, Jan Damborsky wrote:
Hi Joe,
thank you very much for review.
Please see my response in line.
Jan
On 11/10/10 01:23 PM, [email protected] wrote:
On 11/10/10 05:06 AM, Jan Damborsky wrote:
Hi Joe,
could I please ask you to take a look at following two bug fixes
residing
in ICT world ?
CRs:
6991570 Do not configure savecore directory, let svc:/system/dumpadm go
with default value
6996538 Remove configure_nwam() ICT task - that workaround is no longer
needed
webrev:
http://cr.opensolaris.org/~dambi/bug-6991570
Thank you very much,
Jan
tests accomplished:
* LiveCD, text, AI install media built containing modified bits (ict.py
& install-finish)
* installations with those tested - it was verified that
for CR 6991570
- before reboot, /a/etc/dumpadm.conf on target does not contain
DUMPADM_SAVDIR definition
- after reboot, dumpadm service populated DUMPADM_SAVDIR in
/etc/dumpadm.conf with
default value (/var/crash/`uname -n`)
for CR 6996538
- /etc/nwam/llp is no longer being created on the target system
- nwam brought up all active NICs
Looks pretty good Jan.
I just noticed a couple of things...
Hope this helps!
Joe
- - -
ict.py
Issue:
1145 prerror('Failure. Returning: ICT_UPDATE_DUMPADM_NODENAME_FAILED')
Should be:
prerror('Failure. Returning: ICT_UPDATE_DUMPADM_FAILED')
Thank you for catching this. I have changed it.
Suggestion:
I don't think in this case that it is 100% necessary but it might be
safest to wrap line 1133 in a try except block. The only way I could
image this to be necessary is if by some strange chance the variable
"dumpadmfile" gets clobbered. Which is unlikely so I will make the
suggestion but leave it up to you.
Since 'dumpadmfile' variable is initialized with constant value couple
of lines above,
I agree that wrapping os.access() within try-except wouldn't buy us
anything.
Based on that, I would prefer to leave the code as is in order to save
couple of
lines.
Issue:
1134 _dbg_msg('Dump device was not configured during the installation,' +
1135 dumpadmfile + ' will not be created on the target.')
1136 return 0
Is it sufficient to only log this for debugging? Shouldn't the user
see this in the log file?
Maybe info_msg would be a better choice than _dbg_msg()
That sounds reasonable. I have changed it and published updated diff
webrev:
http://cr.opensolaris.org/~dambi/bug-6991570-diff/
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
