Ritu Kamboj wrote:
> Kindly find the webrev for MySQL integration at :
>
> http://cr.opensolaris.org/~rkamboj/MySQLWebRev/
>
> Code reviews are solicited from this team and review/feedback is
> requested by 11/8.
- new/usr/src/cmd/mysql-5-0/Makefile.sfw
> + CFLAGS="-xO3 -xprefetch=auto -xprefetch_level=3 -mt -fns=no -fsimple=1
> -ftrap=%none -nofstore -xbuiltin=%all -xlibmil -xlibmopt -xtarget=generic" \
> + CXXFLAGS="-xO3 -xprefect=auto -xprefecth_level=3 -mt -fns=no -fsimple=1
> -ftrap=%none -nofstore -xbuiltin=%all -features=no%except -xlibmil -xlibmopt
> -xtarget=generic" \
Erm... are you sure this works ?
1. "-fsimple=1" and "-xlibmopt": Did you check the code whether this
really works ?
2. "-ftrap=%none" is AFAIK the default
3. "-nofstore" is x86-only
4. Do NOT use "-features=no%except". Every normal C++ application which
likes to use your applications and C++ exceptions will be in trouble
otherwise (it may be Ok if "mysql" never exposes any C++ API... but I
would double or trippe-check this against upstream)
5. It may be nice to use the following flags for CC:
-- snip --
CC="/opt/SUNWspro/bin/cc -xc99=%all -D_XOPEN_SOURCE=600
-D__EXTENSIONS__=1"
-- snip --
... this is the same we do for ksh93 and enables both C99 compiler
features and XPG6 mode+extensions (which includes some nice things like
that |popen()|&co. use a /usr/xpg4/bin/sh etc. (which is much closer to
what opensource projects assume as "minimum system shell")) - otherwise
you're locked to Sun Studio's default mode... and that's not really
"fun".. ;-(
6. Please add "-xstrconst" to the CFLAGS to make sure that string
literals are put into the read-only section and not duplicated each time
in a read-write section (gcc does this by default) - this should save
some memory
> --- /dev/null Sat Nov 3 15:11:48 2007
> +++ new/usr/src/cmd/mysql-5-0/install-mysql Sat Nov 3 15:11:48 2007
> @@ -0,0 +1,109 @@
> +#!/bin/sh
AFAIK it may be nice to use /usr/bin/ksh or /usr/bin/ksh93 (or "bash" if
it supports the "errexit" flag) for this script and then do a $ set -o
errexit # at the beginning that the script - the "errexit" flag causes
the shell to immediately abort the script when it hits a command which
returns a failure instead of continuing operation (and maybe running
amok (and ordering 15 new A380 planes via your Ebay-account etc. =:-) ))
BTW: Is the script only called from with the Makefile ? If "not" (or in
the case you want to be on the "safe" side please set PATH to a fixed
value (where "set" really means set and not something like
PATH="${PATH}:/bin/" (which may cause trouble if the user's PATH may
contain something unexpected...)).
> +
> +. ${SRC}/tools/install.subr
Please use "source" instead of "." ("source" is identical except that
it's more readable and that you can check for errors - see
http://www.opensolaris.org/os/project/shell/shellstyle/#use_source_not_dot)
> +VERS=`grep "^MYSQL_VERSION=" Makefile.sfw | sed s/MYSQL_VERSION=//`
Plese use POSIX shell/ksh-syntax for this, e.g. replace x=`foo` with
x="$(foo)", e.g. in this case:
-- snip --
VERS="$(grep "^MYSQL_VERSION=" Makefile.sfw | sed "s/MYSQL_VERSION=//")"
-- snip --
> +fix_install() {
Please use "function fix_install" and not "fix_install()" (assuming you
follow the advice above to use ksh/ksh93, see
http://www.opensolaris.org/os/project/shell/shellstyle/#use_ksh_style_function_syntax).
... and maybe read
http://www.opensolaris.org/os/project/shell/shellstyle/ ... since I
don't have time to bicker the whole list up&&down today I simply
attached a fixed version of the script as
"install-mysql.new_20071105.txt" (note: untested, I just added quotes
and other stuff from the style guide). One missing item is that the
script doesn't set PATH as requested above... (erm... yes.. I know...
code reviews shouldn't work like that but it's faster for me to just fix
the scripts than writing long emails complaining about them... ;-/ ).
Same applies to the SMF startup script, I attached a cleaned-up version
as "mysql.new_20071105.txt" (e.g. added quotes, minor restructurisation,
used "typeset -r" for constant data, use [[ ]] instead of [ ] etc. etc.)
...
----
Bye,
Roland
--
__ . . __
(o.\ \/ /.o) roland.mainz at nrubsig.org
\__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer
/O /==\ O\ TEL +49 641 7950090
(;O/ \/ \O;)
-------------- next part --------------
#!/usr/bin/ksh93
#
# CDDL HEADER START
#
# The contents of this file are subject to the terms of the
# Common Development and Distribution License (the "License").
# You may not use this file except in compliance with the License.
#
# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
# or http://www.opensolaris.org/os/licensing.
# See the License for the specific language governing permissions
# and limitations under the License.
#
# When distributing Covered Code, include this CDDL HEADER in each
# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
# If applicable, add the following below this CDDL HEADER, with the
# fields enclosed by brackets "[]" replaced with your own identifying
# information: Portions Copyright [yyyy] [name of copyright owner]
#
# CDDL HEADER END
#
#
# Copyright 2007 Sun Microsystems, Inc. All rights reserved.
# Use is subject to license terms.
#
#ident "@(#)install-mysql 1.1 07/10/12 SMI"
# stop at the first error
set -o errexit
# use builtin commands (faster)
builtin cp
builtin rm
builtin mv
builtin chmod
source "${SRC}/tools/install.subr"
typeset VERS="$(grep '^MYSQL_VERSION=' Makefile.sfw | sed 's/MYSQL_VERSION=//')"
typeset MYSQLDIR="mysql-${VERS}"
typeset CONFDIR="${ROOT}/etc/mysql/5.0"
typeset DATADIR="${ROOT}/var/mysql"
typeset PREFIX="${ROOT}/usr/mysql-5-0/${VERS}"
typeset SYSMAN1DIR="${ROOT}/usr/share/man/man1"
typeset CWD="$PWD"
typeset DOCDIR="${CWD}/${MYSQLDIR}/Docs"
typeset MANIFESTDIR="${ROOT}/var/svc/manifest/application/database"
typeset METHODDIR="${ROOT}/lib/svc/method"
# ----- Common Utility Functions -----
function install_dir
{
# exit function on error
set -o errexit
typeset dstdir="$1"
typeset mode="$2"
mkdir -p "$dstdir"
chmod "$mode" "$dstdir"
return 0
}
#Copy the install files to $CONFDIR
function fix_install
{
# exit function on error
set -o errexit
# Ship a default my.cnf to simplify ease of use.
cd "${CWD}/${MYSQLDIR}/support-files"
_install N my-small.cnf "${CONFDIR}/my.cnf" 644
# ship the standard my.cnf files
_install N my-small.cnf "${CONFDIR}/my-small-cnf.cnf" 644
_install N my-huge.cnf "${CONFDIR}/my-huge.cnf" 644
_install N my-large.cnf "${CONFDIR}/my-large.cnf" 644
_install N my-medium.cnf "${CONFDIR}/my-medium.cnf" 644
_install N my-innodb-heavy-4G.cnf "${CONFDIR}/my.innodb-heavy-4G.cnf"
644
# remove the unwanted files that are installed by make install
cd "${ROOT}/usr/mysql/5.0/lib/mysql"
rm *.la libvio.a libheap.a
#set the correct permission for data directory
install_dir "${DATADIR}" 755
install_dir "${DATADIR}/5.0" 755
install_dir "${DATADIR}/5.0/data" 700
return 0
}
#Copy the MySQL SMF files from build area to $MANIFESTDIR
function install_smf_files
{
# exit function on error
set -o errexit
cp -f Solaris/mysql.xml "${MANIFESTDIR}"
chmod 444 "${MANIFESTDIR}/mysql.xml"
cp -f Solaris/mysql "${METHODDIR}"
chmod 555 "${METHODDIR}/mysql"
return 0
}
# doc related
#Copy the docs to $SYSMAN1DIR and give it appropriate permission
function install_docs
{
set -o errexit
cd "${CWD}"
cp -f Solaris/mysql.1.sunman "${SYSMAN1DIR}/mysql.1"
chmod 444 "${SYSMAN1DIR}/mysql.1"
return 0
}
# ----- START HERE - actual script processing starts here -----
# Even though this is called "install-mysql", it doesn't really
# install the whole thing. Much of mysql itself is installed by
# make install - we need to fix only permissions. What we install here
# are stuffs that mysql won't install as part of its normal build.
# Each install task is a function, so it's relatively easy to add new
# stuff.
#
fix_install
install_docs
install_smf_files
exit 0
-------------- next part --------------
#!//usr/bin/ksh93
#
# CDDL HEADER START
#
# The contents of this file are subject to the terms of the
# Common Development and Distribution License (the "License").
# You may not use this file except in compliance with the License.
#
# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
# or http://www.opensolaris.org/os/licensing.
# See the License for the specific language governing permissions
# and limitations under the License.
#
# When distributing Covered Code, include this CDDL HEADER in each
# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
# If applicable, add the following below this CDDL HEADER, with the
# fields enclosed by brackets "[]" replaced with your own identifying
# information: Portions Copyright [yyyy] [name of copyright owner]
#
# CDDL HEADER END
#
# Copyright 2007 Sun Microsystems, Inc. All rights reserved.
# Use is subject to license terms.
#
#ident "@(#)mysql 1.1 07/01/02 SMI"
source /lib/svc/share/smf_include.sh
# SMF_FMRI is the name of the target service. This allows multiple instances
# to use the same script.
function getproparg
{
typeset val="$(svcprop -p "$1" "$SMF_FMRI")"
[[ -n "$val" ]] && print -r -- "$val"
}
function mysql_start
{
${MYSQLBIN}/mysqld --user=mysql --datadir=${MYSQLDATA}
--pid-file=${PIDFILE} > /dev/null &
}
function mysql_stop
{
${MYSQLBIN}/mysqladmin shutdown
}
function mysql_restart
{
mysql_stop
while pgrep mysqld > /dev/null ; do
sleep 1
done
mysql_start
}
# main
typeset -r MYSQLBIN="$(getproparg mysql/bin)"
typeset -r MYSQLDATA="$(getproparg mysql/data)"
typeset -r PIDFILE="${MYSQLDATA}/$(/usr/bin/uname -n).pid"
# prechecks
if [[ -z "$SMF_FMRI" ]] ; then
print "SMF framework variables are not initialized."
exit $SMF_EXIT_ERR
fi
if [[ -z "${MYSQLDATA}" ]] ; then
print "mysql/data property not set"
exit $SMF_EXIT_ERR_CONFIG
fi
if [[ ! -d "${MYSQLDATA}" ]] ; then
print "mysql/data directory ${MYSQLDATA} is not a valid MySQL data
directory"
exit $SMF_EXIT_ERR_CONFIG
fi
# setup
if [[ ! -d "${MYSQLDATA}/mysql" ]] ; then
${MYSQLBIN}/mysql_install_db --user=mysql --datadir=${MYSQLDATA}
fi
# method dispatcher
case "$1" in
'start')
mysql_start
;;
'stop')
mysql_stop
;;
*)
print "Usage: $0 {start|stop}"
exit 1
;;
esac
exit $SMF_EXIT_OK
