On 07/10/10 07:45 PM, Jack Schwartz wrote:
Hi everyone.
This webrev supercedes the one I published a week ago, because the
situation with SNAP->ON integration has changed. (Thanks to those who
reviewed the last one!) It also includes some new changes.
Webrev:
http://cr.opensolaris.org/~schwartz/100710.1/webrev/
Webrev vs old one (for those who reviewed the last one):
http://cr.opensolaris.org/~schwartz/100710.1/webrev.diff/
Please review by Weds 7/14 COB.
Changes:
- Since we cannot be sure 6944352, which addresses changes we needed
for installboot and installgrub, would be done in time to make the ON
cutoff for feature freeze, bugs related to that fix have been
removed. Added is support for labeled branded zones, and fixes to
be_create_menu to return proper errors.
- Error message for BE_ERR_UNKNOWN is not "Unknown error" not "Unknown
external error" as all external commands are now covered by other errors.
- Cleanup of be_create_menu() error handling for 16512 is new.
- Addition in libbe.h of "labeled" in BE_ZONE_SUPPORTED_BRANDS for
13865 is new.
Note: The new be_run_cmd function in be_utils.c (the bulk of original
be_utils.c changes) has not changed.
Tested:
Labeled zones:
- installed trusted solaris and created a zone. Then verified that
the zone (and files it had) was properly handled in a new BE.
- Ran regression test of libbe test suite from the STC gate and
verified that there were no new errors.
Messaging of external commands:
Hid ict.py, deleted menu.lst and tried to create a new menu to check
error output if splash screen could not be added to new menu.lst.
Hid installgrub, changed capability file to force installgrub call,
and then did beadm activate to check error output when installgrub
could not be executed.
Modules I changed are lint clean. All files are hg nits clean.
Thanks for your time,
Jack
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
Hey Jack,
Here's my input. Hope this helps. I realize we discussed some of this
already but I listed my comments so others could see them and perhaps
add to them.
Joe
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
usr/src/lib/libbe/be_activate.c
Suggestion to consider:
-----------------------
Would it maybe be better to update be_print_err() to invoke gettext?
e.g.:
Change from
833 be_print_err(gettext(...
To:
833 be_print_err(...
And update be_print_err() to invoke gettext.
+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=+-=
usr/src/lib/libbe/be_utils.c
Idea:
----
2936 /* Handle if caller doesn't want stdout or stderr output. */
2937 if (stdout_buf == NULL) {
2938 stdout_buf_remain = 0;
2939 }
2940 if (stderr_buf == NULL) {
2941 stderr_buf_remain = 0;
2942 }
1st off I like this code the way it is because it continues if
it can. However if the user specifies a buffer size but a NULL
buffer maybe it would be less confusing if the code didn't
continue and returned return (BE_ERR_EXTCMD);
if ((stdout_buf == NULL) && (stdout_bufsize != 0)) {
return (BE_ERR_EXTCMD);
}
if ((stderr_buf == NULL) && (stderr_bufsize != 0)) {
return (BE_ERR_EXTCMD);
}
Just something to think about. Either way is OK with me.
Issue:
------
2871 * Function: be_run_cmd
This solution, although feature rich, seems heavy handed to me.
While reviewing this I wrote a simple C program using popen() so
I understand the problem with capturing stderr using it from C.
But Python does a good job of doing this already perhaps
implementing this in Pyhon might result in simpler code.
Or using popen() and directing stderr to a file that could be read after
the command completed.
Issue:
------
Typo returing -> returning
2909 * - There were errors extracting or returing subprocess
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
