From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39335) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWWn4-0001EP-EE for qemu-devel@nongnu.org; Thu, 08 Nov 2012 13:25:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TWWmz-0008Fz-3i for qemu-devel@nongnu.org; Thu, 08 Nov 2012 13:25:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:57781) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TWWmy-0008FH-QP for qemu-devel@nongnu.org; Thu, 08 Nov 2012 13:25:17 -0500 Message-ID: <509BF900.2050700@redhat.com> Date: Thu, 08 Nov 2012 11:25:04 -0700 From: Eric Blake MIME-Version: 1.0 References: <509B9FEF.4040604@hitachi.com> <509BA754.3090403@hitachi.com> In-Reply-To: <509BA754.3090403@hitachi.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigDC6E16351B7EDDCC406947BA" Subject: Re: [Qemu-devel] [PATCH 2/2] qemu-ga: sample fsfreeze-script List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Tomoki Sekiyama Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigDC6E16351B7EDDCC406947BA Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: quoted-printable On 11/08/2012 05:36 AM, Tomoki Sekiyama wrote: > Adds sample scripts for --fsfreeze-script option of qemu-ga. > -fsfreeze.sh: iterates and execute scripts in fsfreeze.d/ > -fsfreeze.d/mysql-flush.sh: quiesce MySQL before snapshot >=20 > Signed-off-by: Tomoki Sekiyama > --- > +++ b/docs/qemu-guest-agent/fsfreeze-script-sample/fsfreeze.d/mysql-flu= sh.sh > @@ -0,0 +1,41 @@ > +#!/bin/bash Any particular reason you have to use a bash-ism, or would it be appropriate to use /bin/sh here? > +MYSQL=3D"mysql -uroot" #"-prootpassword" > +FIFO=3D/tmp/mysql-flush.fifo > + > +flush_and_wait() { > + echo 'FLUSH TABLES WITH READ LOCK \G' > + read < $FIFO > + echo 'UNLOCK TABLES \G' > +} > + > +if [ "$1" =3D "freeze" ]; then > + > + mkfifo $FIFO No error checking? > + flush_and_wait | $MYSQL & > + # wait until every block is flushed > + while [ "`echo 'SHOW STATUS LIKE "Key_blocks_not_flushed"' |\ I prefer $() over `` > + $MYSQL | tail -1 | cut -f 2`" -gt 0 ]; do > + sleep 1 > + done > + # for InnoDB, wait until every log is flushed > + while :; do > + INNODB_STATUS=3D/tmp/mysql-innodb.status This name is highly predictable, and therefore highly insecure. I hope I'm never caught installing something this insecure on my system. > + echo 'SHOW ENGINE INNODB STATUS \G' | $MYSQL > $INNODB_STATUS > + LOG_CURRENT=3D`grep 'Log sequence number' $INNODB_STATUS |\ > + tr -s " " | cut -d' ' -f4` > + LOG_FLUSHED=3D`grep 'Log flushed up to' $INNODB_STATUS |\ > + tr -s " " | cut -d' ' -f5` More instances where $() is nicer than `` > + rm $INNODB_STATUS > + [ $LOG_CURRENT =3D $LOG_FLUSHED ] && break Are you sure that $LOG_CURRENT and $LOG_FLUSHED will always be non-empty and contain no whitespace? If not, you are missing quoting here. > + sleep 1 > + done > + > +elif [ "$1" =3D "thaw" ]; then > + > + if [ -p $FIFO ]; then > + echo > $FIFO > + rm $FIFO > + fi > + > +fi > + > diff --git a/docs/qemu-guest-agent/fsfreeze-script-sample/fsfreeze.sh b= /docs/qemu-guest-agent/fsfreeze-script-sample/fsfreeze.sh > new file mode 100755 > index 0000000..b402107 > --- /dev/null > +++ b/docs/qemu-guest-agent/fsfreeze-script-sample/fsfreeze.sh > @@ -0,0 +1,17 @@ > +#!/bin/bash Again, I see no bash-isms, why not use /bin/sh. > + > +# This script is executed when a guest agent receives fsfreeze-freeze = and > +# fsfreeze-thaw command when it is specified in --fsfreeze-script/-F o= ption > +# of qemu-ga or placed in default path (/etc/qemu/fsfreeze-script.sh).= > +# When the agent receives fsfreeze-freeze command, the script is issue= d with > +# "freeze" argument before the filesystem is freezed.. And for fsfreez= e-thaw, > +# it is issued with "thaw" argument after filesystem is thawed. > +# > +# This script iterates executables in directory "fsfreeze.d" with the > +# specified argument. > + > +cd `dirname $0` > +cd fsfreeze.d Unsafe if $0 contains spaces or starts with '-'. Although you could argue that either of those situations represents installation error, it never hurts to be robust. Also, why bother with two cd when one would do, and where is your error checking? > +for x in *; do > + [ -x ./$x ] && ./$x $1 Should you be filtering out files such as *~ or *.bak or ~.rpmsave, and so forth? This is insecure if $x contains spaces. And rather than unquoted $1, it is better to pass "$@", as in: [ -x "$x" ] && "./$x" "$@" > +done >=20 --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigDC6E16351B7EDDCC406947BA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQEcBAEBCAAGBQJQm/kBAAoJEKeha0olJ0NqBmgH/ilBcypk0BqaupjXoPXoDdhB J6D7rLy9HvPrWI5S85lV0R61FTF8OFpmO8G1ONkAxaOSHd1C+fTPWg9uMYCCQ7o5 Bl0jECR0FqpUoMhUXBIYHnQWBvtKsTuCUlDTkArzpaarZx28ganD2ZqIjDoFOHoT iHIq5mBRP/7biJLcj4u0DF1CwUiGF+6Y3fYiy5QWQJM8XICnXgUB4uG6Qh2eey84 VYb7U783NdslFAKFmVNjvzQS18vXFCwgGBT/0ebmxFSQHnzMiKSjTil6hOWNb9k2 kNuILfhZDepqs4OgjjCJJXlWBzoW6X2Bki+gXIGS8Hg0XRag47j5ek3j69IfTDY= =e83q -----END PGP SIGNATURE----- --------------enigDC6E16351B7EDDCC406947BA--