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. - Keith On 02/ 5/10 08:50 AM, Keith Mitchell wrote: > Thank you for the info Strony. I have a new approach based on that and > further evaluation of the following functions: > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/cmd/kbd/kbd.c#303 > > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/kbd.c#816 > > > http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/kb8042/kb8042.c#921 > > > > It seems that the correct answer, particularly based on the first link > above, is that attempting to retrieve the KIOCLAYOUT or set the > keyboard layout is only valid if the keyboard is of type KB_USB as > defined in kbd.h. Please let me know if I'm off base with this approach. > > Mary - would you be able to test the attached ict.py in the two test > scenarios we have (using the text installer)? > > I'll create an additional webrev, and forward this thread to > caiman-discuss with a summary, so that we can evaluate the riskiness > vs. value of each approach. > > Thanks, > Keith > > On 02/ 5/10 06:28 AM, Strony Zhang wrote: >> I searched the possible error number for KIOCLAYOUT ioctl in ONNV. >> Besides the io/kbd.c, the kb8042.c also may return the EINVAL for the >> ioctl. >> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/kb8042/kb8042.c#921 >> >> >> >> I agree with you that the error should not stop the installation >> process. But I think ignoring the EINVAL should only work for the >> type of keyboards that don't support the KIOCLAYOUT ioctl. So I think >> it might be reasonable to call KIOCTYPE first and then ignore the >> EINVAL error for the known type not supporting te KIOCLAYOUT. >> >> Regards, >> Strony >> >> Keith Mitchell : >>> The webrevs for my two potential solutions are below, for reference: >>> >>> Assuming successful install when ioctl returns EINVAL or when >>> setting the keyboard layout isn't possible: >>> http://cr.opensolaris.org/~kemitche/9698_a/ >>> >>> Assuming failure in those cases: >>> http://cr.opensolaris.org/~kemitche/9698_b/ >>> >>> - Keith >>> >>> On 02/ 4/10 02:31 PM, Keith Mitchell wrote: >>>> Hi Strony, >>>> >>>> I have two potential solutions for this bug, and am hoping you can >>>> clarify something for me which will help me determine which is the >>>> better approach. >>>> >>>> The ioctl call for KIOCLAYOUT is returning an error code of 22 >>>> (EINVAL) in some cases. In those cases, we will fail to write to >>>> the /etc/default/kbd file. For the cases listed, we've determined >>>> that there are no ill effects. >>>> >>>> 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 >>>> >>>> Here is my question: Is it safe to ignore that error and skip >>>> writing the LAYOUT to the /etc/default/kbd file, or are there other >>>> scenarios in which EINVAL is returned that should be considered? >>>> What I'm trying to avoid is ignoring that error, and returning that >>>> the function succeeded, if there is a scenario when that is not >>>> true - meaning, the keyboard will not function on boot. >>>> >>>> Our other option would be to return a failure in those cases. The >>>> concern there is that this causes the installers to inform the user >>>> that the install failed, when in fact, in any cases where we know >>>> about this issue, it is not really a failure and the installed >>>> system is usable. >>>> >>>> Thank you, >>>> Keith >>>> >>>> On 02/ 4/10 01:59 PM, mary ding wrote: >>>>> Strony: >>>>> >>>>> Happy New Year and we need your help on osol stopper bug 9698. >>>>> Keith Mitchell is going to change the ict.py. He will send you >>>>> email to ask your opinion about the fix. >>>>> >>>>> I had tested his proposed fix on various sun4u machines with >>>>> tipline, system with type 4 keyboard and system with self >>>>> identifying keyboard and they all work fine. >>>>> >>>>> >>>>> >>>>> >>>>> -------- Original Message -------- >>>>> Subject: [Bug 9698] keyboard_layout() ICT fails on some Sparc >>>>> machines due to the failure of KIOCLAYOUT ioctl(2) >>>>> Date: Thu, 04 Feb 2010 20:25:38 +0000 (GMT) >>>>> From: bugzilla at defect.opensolaris.org >>>>> To: mary.ding at sun.com >>>>> References: <bug-9698-1065 at http.defect.opensolaris.org/bz/> >>>>> >>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=9698 >>>>> >>>>> >>>>> Ethan Quach <ethan.quach at sun.com> changed: >>>>> >>>>> What |Removed |Added >>>>> ---------------------------------------------------------------------------- >>>>> >>>>> >>>>> CC| |ethan.quach at sun.com >>>>> Depends on|9549 | >>>>> >>>>> >>>>> --- Comment #27 from Ethan Quach <ethan.quach at sun.com> 2010-02-04 >>>>> 20:25:34 UTC --- >>>>> Removing this bug's dependency on 9549. This bug doesn't really >>>>> depend >>>>> on it. >>>>> >>
