Jan, *usr/src/cmd/auto-install/auto_install.c*:
750-770: This is not a comment on your webrev. If the user password is not supplied, I don't think the user can login to the system. The default setup in opensolaris doesn't allow any user to login with null password (/etc/default/login: PASSREQ). We may have to file a bug not to allow or change the settings in the /etc/default/login file 1777: This error "Couldn't unmount target BE" still looks cryptic. can we come up with a more meaningful error message? *usr/src/cmd/auto-install/auto_parse.c*: 715: Can you add "from SC manifest" to the error message as follows? "Couldn't parse %s property from SC manifest" * usr/src/cmd/slim-install/svc/live-fs-root*: 380, 396: Can you add the whole path for solaris.zlib and solarismisc.zlib including $url? 396: server --> install server *usr/src/lib/install_utils/install_utils.py*: Suggestion: 432-436: The checking of log to none can be made similar to other checking of log in lines 477-483, 490-495, and 517-522 if log == None construct could be changed to if log != None so that it will be consistent *usr/src/lib/liborchestrator/om_misc.c*: 83, 87: Can we add more information to the error message like "Installing from the package repository failed" if it is AI and "Transferring the files from cd failed" in the case of livecd installer. If it is difficult to figure out the installer type, you can say "Transferring the files from the source failed. Please see the previous messages for more details" 95: Can we change the error message to "One or more installation completion tasks failed. Please see the previous messages for more details" 59- 96: What there are only 9 errors defined in the array om_failure_description_array? Are these the only API used by auo_install code? *usr/src/lib/liborchestrator/orchestrator_api.h:* 285 int code; /* see definitions below for possible error codes */ What definitions the comments are referring here? - Sundar Jan Damborsky wrote: > Hi, > > Could I please ask for reviewing the fix for bucket of AI bugs > related to error reporting ? > > Please provide your code review comments by COB Wednesday > October 28th, 2009. If you need more time, please let me know. > > Thank you very much, > Jan > > * Background > ============ > > As far as AI error handling and reporting is concerned, there is > a plan to hook AI into new error handling service which is to be based > on work Evan initiated. > As it was identified that it will take some time to implement the > final solution (there are still things to be sorted out and dependencies > to be addressed - e.g. CUD effort, progress reporting or new > logging service), we decided to address existing bugs filed against AI > error reporting - those which are annoying or might be source of > confusion, but which doesn't involve too much effort to fix, since it is > likely that some portion of the changes will go away once AI switches to > new error and logging service. > > * Following bugs are addressed > ============================== > > 6651 autoinstall needs more useful error messages from Orchestrator > module > 7433 autoinstall should include output from pkg(5) > 8127 Complete /tmp/install_log not copied to > /var/sadm/system/logs/install_log > 8639 be_init() failed with error code 4044 when username is "" > in SC manifest > 10040 Error reporting when unable to download > solaris.zlib/solarismisc.zlib is unobvious > 10688 AI should validate repository reachability > 11421 ddm_drive_is_cdrom(): ioctl(DKIOCREMOVABLE) > failed in /tmp/install_log > 11640 decrypt liborchestrator IPS debug messages > > * Webrev > ======== > > http://cr.opensolaris.org/~dambi/ai-err-report/ > > * list of files modified - associated with bugs > 6651 > - auto_install.c > - installation-screen.c > - om_misc.c > - orchestrator_api.h > - perform_slim_install.c > > 7433 > - transfer_mod.py > - install_utils.py > - libtransfer.c > > 8127 > - auto_install.c > - ict.c > - orchestrator_api.h > - perform_slim_install.c > > 8639 > - auto_install.c > - auto_parse.c > > 10040 > - live-fs-root > > 10688 - addressed by bug 7433 > > 11421 > - td_dd.c > > 11640 > - perform_slim_install.c > > * PEP8 effort > ============= > > Following Python files were made PEP8 compliant and 'pylinted' > (they are not pylint clean, but has ratio >7/10 with the exception > mentioned below): > > - install_utils.py (no Errors, no Fatals) - rated at 8.71/10 > > Following green Cdiff webrev sections belong to bug fixes, everything > else is PEP8 related: > --- 404,414 ---- > --- 424,441 ---- > --- 451,539 ---- > > > - install-finish (no Errors, no Fatals) - rated at 8.85/10 > > Following green Cdiff webrev sections belong to bug fixes, everything > else is PEP8 related: > --- 212,238 ---- > > > - transfer_mod.py (No Fatals)- rated at 0.33/10 > > The problem of this file is following section: > ... > execfile('/usr/lib/python2.4/vendor-packages/osol_install/transfer_defs.py') > > ... > I have verified that all errors generated by pylint(1) complain about > undefined symbols coming from that file. > I was thinking about addressing this problem as Jean suggested (import > explicitly all symbols used). It worked in case of 'install-utils.py', > but it turned out that transfer_mod.py uses values which are generated > on-the-fly, so it didn't help. I have found bug 5559 which will change > the format of transfer_defs.py anyway, so I decided not to address > that problem by this fix. > > Following green Cdiff webrev sections belong to bug fixes, everything > else is PEP8 related: > --- 21,67 ---- > --- 318,327 ---- > --- 830,852 ---- > --- 889,904 ---- > --- 928,946 ---- > --- 976,999 ---- > --- 1011,1025 ---- > --- 1044,1059 ---- > --- 1100,1130 ---- > --- 1146,1160 ---- > > > * Testing > Please see following file for test used procedures and test results: > > http://cr.opensolaris.org/~dambi/ai_error_reporting-test/test-procedures.txt > > > Also Jeffrey Huang is involved in testing process. > > > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20091029/a9e5c2b7/attachment.html>
