LGTM. --emk
Thorsten Frueauf wrote:
> Hi Neil, Nick, Ed et al,
>
> thanks for your review. I have fixed the typos found by Neil. In
> addition a new problem got identified, where I would like an
> additional review - the fix is again very small. The webrev is updated
> to contain the fix for the new CR as well:
>
> http://cr.opensolaris.org/~frueauf/colorado-1-ha-ipkg-zone-6812924-6814263/
>
>
> The additional problem is with the validate method for the sczsmf
> component:
>
> If a native non-global zone is getting created and installed on
> Solaris 10 and Solaris Express (aka Nevada), it will inherit the
> locales installed within the global zone. So while the default locales
> do not need to be set to match between global zone and non.global
> zones, the locale itself would be
> available.
> OpenSolaris does not have a native brand-type - the default there is the
> ipkg brand type. If a ipkg non-global zone is getting created and
> installed
> on OpenSolaris, it will not inherit the locales installed within the
> global zone. Instead it has the very basic set (like C, POSIX), but none
> else. Even the locale command is not getting installed.
> On such a default installation, if you perform
>
> zlogin <zonename> <command>
>
> you will see a message send to stderr like:
>
> en_US.UTF-8: unknown locale
>
> if en_US.UTF-8 is the default locale within the global zone.
> And in most cases stderr is not being processed. But within the
> validate method for sczsmf there is one check to see if the SMF
> service being provided in the config file exists and all of the
> services it depends on are online:
>
> [...] if STATE=`${ZLOGIN} ${ZONE} svcs -Hd ${SERVICE} 2>
> ${LOGFILE} | /usr/bin/awk '{if ($1 != "online") print $3 " which is "
> $1}'`
> then
> if [ -s "${LOGFILE}" ]
> then
> # e.g. svcs: Pattern 'X' doesn't match any instances
> log_message error validate
> rc_validate=1
> fi
> [...]
>
> And here the output to stderr is captured and evaluated. Normally it
> is expected
> to be empty - if it is not it triggers validate to fail.Since the
> zlogin will always create the "unknown locale" message to stderr,
> the code will always think the provided SMF service does not exist and
> error out.
>
> I would appreciate a quick review so that I could still be able to
> putback later today.
>
> Greets
> Thorsten
>
> Neil Garthwaite wrote:
>> Hi Thorsten,
>>
>> Good explanation, just some minor nits.
>>
>> line 930 retreive should be retrieve
>> line 931 Therefor should be Therefore
>>
>> Also the blog link didn't work, instead I used http://blogs.sun.com/stw/
>>
>> Regards
>> Neil
>>
>> On 4 Mar 2009, at 23:05, Thorsten Frueauf wrote:
>>
>>> Hi,
>>>
>>> please review the following webrev:
>>> http://cr.opensolaris.org/~frueauf/colorado-1-ha-ipkg-zone-6812924/
>>>
>>> The sczbt components allows to make an IP configured through a
>>> SUNW.LogicalHostname (LH) resource available within the non-global
>>> zone, after the zone is booting.
>>>
>>> This is done by setting the zone flag with ifconfig on the
>>> corresponding adapter, which hosts the IP for the LH, when the zone
>>> is booting, and removing that flag before the zone is halted.
>>>
>>> Clearview did change the way IPMP works. Details can be found at
>>> http://www.opensolaris.org/os/project/clearview/ipmp-highlevel-design.pdf
>>>
>>>
>>> and a very good blog with many example output:
>>> http://blogs.sun.com/stw/entry/ipmp_re_architecture_is_delivered
>>>
>>> The sczbt functions file does implement the get_ipmp_state()
>>> function. In the lack of a better CLI API (pre Clearview), it uses
>>> scstat to retrieve the adapter list for a given IPMP group, for
>>> which the LH is configured, and tries to determine the adapter where
>>> the IP is currently configured on, to then set the zone flag.
>>>
>>> scstat does not display any Clearview information. Clearview will
>>> configure the IP not on any physical adapter, instead it uses the
>>> new ipmp<number> interface.
>>>
>>> The current code does not work with that logic and needs
>>> modification, which is tracked with CR 6812924.
>>>
>>> Clearview did introduce a new command line interface: /sbin/ipmpstat
>>>
>>> The option -g will output information that can be used to determine
>>> the ipmp interface for which the zone flag needs to get set. Example:
>>>
>>> # ipmpstat -g
>>> GROUP GROUPNAME STATE FDT INTERFACES
>>> ipmp0 sc_ipmp0 ok -- e1000g0
>>>
>>> The fix has been verified with OpenSolaris build 108. Regression
>>> test has been done on OpenSolaris 2008.11 (build 101a), which has no
>>> Clearview.
>>>
>>> The review will timeout at Friday, 6. March, COB MET.
>>>
>>> Greets
>>> Thorsten
>
