On Mon 28 Jan 2008 at 04:02PM, Steve Peng wrote:
>
> The full nightly build was built successfully and testings look good as
> well. I have posted the new incremental webrev under
>
> http://cr.opensolaris.org/~stevep/6351623-2
>
> The previous one has been moved under 6351623-1. Please take a look at
> and if you can get back to me with any new comment
> by cob Wed then that will great.
I just wanted to followup and confirm: have you done stress testing with
'svcadm _smf_repository_switch' in a tight loop? Tested with zones?
A couple of other minor stylistic comments; none should impact your testing.
1120 + * Copying the repository. If the sw_back flag is not set, we are
"Copying" -> "Copy"
1161 + configd_critical("Backend copy failed: openning %s:
%s\n",
"openning" -> "opening"
1186 + configd_critical("Backend copy failed: copy bytes
mismatch\n");
Message is a little vague.
1237 + if (r == SQLITE_OK && (info.rs_result !=
1238 + REP_PROTOCOL_FAIL_NOT_FOUND && val ==
1239 + BACKEND_SCHEMA_VERSION))
The parens here are I think unneeded, and actually are confusing.
Code might be a lot more readable as:
if (r == SQLITE_OK &&
info.rs_result != REP_PROTOCOL_FAIL_NOT_FOUND &&
val == BACKEND_SCHEMA_VERSION)
1246 + * Backend switch entry point and is called to perform
1247 + * the backend copy and switch from src to dst. First,
1248 + * it blocks all other clients from accessing the repository
1249 + * by calling backend_lock to lock the repository. Upon
1250 + * successful lock, coping and switching of the repository
1251 + * are performed.
1246 is sort of a run-on sentence...
"coping" -> "copying"
Also, please format to 76 columns or so. Ditto at line 1275.
1299 configd_critical(
1300 + "Backend switch fails: integrity check
%s: %s\n",
1301 + dst, errp);
"fails" --> "failed", same at 1305.
2154 + * If the system crashed during a backend switch, there might
2155 + * be a leftover transient database which containing useful
2156 + * information which can be used for recovery.
"which containing" --> "which contains"
repcache_protocol.h:
359 + * When the flag is set to fast, the operation moves the main
repository
360 + * from the default location (/etc/svc) to the tmpfs location
361 + * (/etc/svc/volatile). When it is set to perm, the operation
performs
362 + * the reversed switch.
Suggest: "When the flag is set to 'fast', move the main repository
^ ^ ^^^^^^^^^^^^^^^^^^^
from the default location (/etc/svc) to the tmpfs location (/etc/svc/volatile).
When it is set to 'perm', the switch is reversed."
^ ^ ^^^^^^^^^^^^^^^^^^^
Otherwise, looks good. Thanks for tackling this problem.
-dp
--
Daniel Price - Solaris Kernel Engineering - dp at eng.sun.com - blogs.sun.com/dp
