On 02/ 5/10 05:09 PM, Keith Mitchell wrote: > > > On 02/ 5/10 11:05 AM, Joseph J. VLcek wrote: >> On 02/ 5/10 12:09 PM, Keith Mitchell wrote: >>> Hi all, >>> >>> I have 3 proposed fixes for 9698, and am hoping to get some feedback >>> on risk vs. reward for making it into build 133 or 134. >>> >>> The summary of the issue is: Certain SPARC systems either fail to >>> retrieve the KIOCLAYOUT ioctl, or do not support setting the >>> keyboard layout. >>> >>>> Case 1: Older sun4u machines that are tipline only. (These are >>>> identified by the fact that they return "ioctl (kbd layout): >>>> invalid argument" when the command "kbd -l" is run). >>>> >>>> Case 2: sun4u's with terminal/keyboard attached, where the keyboard >>>> is not self-identifying. These can be identified by their output >>>> from "kbd -l" and "kbd -s" >>>> kbd -l: >>>> type=4 >>>> layout=34 (0x22) >>>> delay(ms)=500 >>>> rate(ms)=40 >>>> kbd -s: >>>> Type 4 Sun keyboard >>>> The -s option does not apply for this keyboard type. >>>> Only USB/PS2 type keyboards support this option >>> >>> For the systems affected by Case 1, the keyboard_layout ICT fails >>> with an uncaught exception, which breaks install-finish per >>> http://defect.opensolaris.org/bz/show_bug.cgi?id=6255#c3 >>> >>> The most minimal solution for this problem is to wrap the offending >>> code in a try/except block and fail if an exception occurs: >>> http://cr.opensolaris.org/~kemitche/9698_b/ >>> >>> This is a very low risk solution that leaves the system in a usable >>> state, assuming no other ICTs fail. However, it leaves the ICT >>> returning a failure, which causes installers to report that to the >>> user that the install failed, when in fact the install completed >>> fine and the system is usable. >>> >>> An alternate solution was proposed yesterday, that checks the errno >>> of the exception raised and reports a successful ICT call if EINVAL >>> was the reason for failure. However, after further discussion and >>> review (see email thread below), this appears to be the wrong >>> approach (particularly because it is a 'band-aid' fix for the >>> greater problem captured by bug 14247). >>> >>> Further examination of the kbd code (again, see thread below for >>> relevant discussion and links to files) yields a third solution: >>> http://cr.opensolaris.org/~kemitche/9698_c/ >>> >>> However, that proposed solution has a much higher chance of >>> unintended side effects, such as possibly not setting the keyboard >>> layout properly when it should have been. I'm in the process of >>> testing that solution now, but I don't feel that I can hit a wide >>> enough set of scenarios to be confident about it. >>> >>> In summary, I would like to propose fixing 9698 using the first >>> solution above (http://cr.opensolaris.org/~kemitche/9698_b/), >>> opening another bug against this ICT to explore the need for finer >>> grained control over when we can or cannot set the keyboard layout, >>> and let the remaining issues get captured by the fix to bug 14247. >>> >> >> >> >> >> Why not return success here. I view this as the ICT is not actually >> failing. >> >> Wouldn't it make sense to issue a message so it will be in the log >> using, info_msg instead of perror, then return success: >> >> >> if status != 0: >> kbd.close() >> info_msg('fcntl ioctl KIOCLAYOUT_FAILED: status=' + >> str(status)) >> return 0 >> >> >> Joe >> >> > > Joe, > > How do you feel about the solution as originally discussed: > http://cr.opensolaris.org/~kemitche/9698_a/ > > - Keith > I think that's perfect.
Huge thanks for chasing this down Keith! Joe
