Hi Alok.
Thanks for your review.
On 07/15/10 03:48 PM, Alok Aggarwal wrote:
Hi Jack,
I looked at this code review briefly.
My main concern is with the be_run_cmd() and that it
seems awfully complicated for what you're trying to
achieve here.
The traditional way of doing something like this would be to fork(2) a
child process and exec(2) the command
you'd like to run as part of that process. Use a pipe(2)
to facilitate inter-process communication between the parent and the
child and use dup(2)/dup2(3C) as appropriate
to duplicate stdout/stderr fd's. That's the very simple way of doing
what you're doing here.
This is more complicated than what I did. popen(3C) and mkfifo(3C) is
simpler.
With monitoring of two pipes, I have to call poll(2) to know when to
read them.
Did you find doing it this way complex for some reason?
Please see the comments I made to Sanjay yesterday to answer this
specific question.
Thanks,
Jack
Thanks,
Alok
On Sat, 10 Jul 2010, 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
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
