Hi Thorsten, Looks good.
Regards Neil On 6 Mar 2009, at 15:42, 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 > > -- > ~ > ~ > ~ > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > Sitz der Gesellschaft: > Sun Microsystems GmbH, Sonnenallee 1, D-85551 Kirchheim-Heimstetten > Amtsgericht Muenchen: HRB 161028 > Geschaeftsfuehrer: Thomas Schroeder, Wolfgang Engels, Dr. Roland > Boemer > Vorsitzender des Aufsichtsrates: Martin Haering > ~ > ~ > ~ > ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
