On 10/ 1/09 05:40 AM, Jerry Jelinek wrote:
Edward Pilatowicz wrote:
i'm not done yet, but i've attached what i've got so far.
Ed,
Thanks for your comments. I'll start to work through
these while we're waiting for the rest of your input and
respond if there is anything we're not going to address.
Thank again,
Jerry
___
zones-discuss mailing list
zones-discuss@opensolaris.org
Hi Jerry,
I have a few nits and questions aside from Ed's.
Thanks,
Jordan
--
usr/src/lib/brand/solaris10/cmd/s10_automount.sh
usr/src/lib/brand/solaris10/cmd/s10_automountd.sh
Shouldn't the scripts' parameters be included at the end of the last
line (the exec command) as in s10_isaexec_wrapper.sh?
--
usr/src/lib/brand/solaris10/librtld_db/common/solaris10_librtld_db.c
The ps_plog() invocation in s10_ldb_fini32() displays lx_ldb_fini when
it should be s10_ldb_fini (right?). According to the diff, the sn1
version also uses lx_ldb_fini.
--
usr/src/uts/common/brand/solaris10/s10_brand.c
165-171: Are we going to retain the lx brand comments? I remember
asking this two or three months ago, but someone answered that we wanted
it to look the same as in the lx brand file from which the comment
originated. Why?
740-743: I could've simplified this a bit by combining both cases.
These lines can be condensed to
case CT_TGET:
case CT_TSET:
return (ctfs_ioctl(rval, fdes, cmd, arg));
927-928: I could've improved this comment by stating that the path of
the *dynamic linker* is the second parameter of s10_native_exec().
1260-1261,1286-1287,1313,etc.: Couldn't we make arg1 a zoneid_t, arg2 an
int, arg3 a char *, and arg4 a size_t and eliminate some of the casts in
s10_zone() (as well as some of the automatic variables, e.g., buf and
bufsize)?
1298: Shouldn't we move this truss point below the switch block? As it
currently stands, if a process issues SYS_zone to get an attribute of
the global zone other than ZONE_ATTR_NAME and ZONE_ATTR_BRAND, then
truss would report two SYS_zone syscalls instead of one.
--
usr/src/lib/brand/solaris10/s10_support/s10_support.c
289-296: Isn't this whole loop simply looking for SUNWcakr's pkginfo
file in the zone? If so, then looping through the zone's /var/sadm/pkg
directory's entries for SUNWcakr is superfluous: get_ku_patchlist()
could simply construct the path
$ZONEPATH/root/var/sadm/pkg/SUNWcakr/pkginfo and stat() will fail if it
doesn't exist. (Are we planning to examine other packages for patch lists?)
get_image_emul_version(): I agree with Ed that get_image_emul_version()
is superfluous. Now that I've thought about it,
$ZONEROOT/usr/lib/brand/solaris10/version should be sufficient for the
brand to determine whether it can host the associated S10C. All we need
to do is hard-code the maximum version number supported by the brand
(for example, as a preprocessor constant), fetch the version number
stored in $ZONEROOT/usr/lib/brand/solaris10/version (or zero if the file
does not exist), check whether the latter exceeds the former, and set
the brand's emulation number to that stored in
$ZONEROOT/usr/lib/brand/solaris10/version.
467,471-472,476-477: The first conditional can be changed to argc != 3
and the other two can be deleted along with their invocations of usage().
___
zones-discuss mailing list
zones-discuss@opensolaris.org
