| Message ID | 20121207083932.10624.8173.stgit@melchior2.sdl.hitachi.co.jp |
|---|---|
| State | New |
| Headers | show |
On Fri, 07 Dec 2012 17:39:32 +0900 Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. > - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ > - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > --- > .gitignore | 1 > Makefile | 2 - > scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ > .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ > 4 files changed, 90 insertions(+), 1 deletion(-) > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > > diff --git a/.gitignore b/.gitignore > index bd6ba1c..286822d 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -93,3 +93,4 @@ cscope.* > tags > TAGS > *~ > +!scripts/qemu-guest-agent/fsfreeze-hook.d Why? Do we expect to have *~ files in there? > diff --git a/Makefile b/Makefile > index 9ecbcbb..466dcd7 100644 > --- a/Makefile > +++ b/Makefile > @@ -248,7 +248,7 @@ clean: > # avoid old build problems by removing potentially incorrect old files > rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h > rm -f qemu-options.def > - find . -name '*.[od]' -exec rm -f {} + > + find . -name '*.[od]' -type f -exec rm -f {} + What does this change have to do with this patch? > rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ > rm -Rf .libs > rm -f qemu-img-cmds.h > diff --git a/scripts/qemu-guest-agent/fsfreeze-hook b/scripts/qemu-guest-agent/fsfreeze-hook > new file mode 100755 > index 0000000..4f7ff15 > --- /dev/null > +++ b/scripts/qemu-guest-agent/fsfreeze-hook > @@ -0,0 +1,33 @@ > +#!/bin/sh > + > +# This script is executed when a guest agent receives fsfreeze-freeze and > +# fsfreeze-thaw command, if it is specified in --fsfreeze-hook (-F) > +# option of qemu-ga or placed in default path (/etc/qemu/fsfreeze-hook). > +# When the agent receives fsfreeze-freeze request, this script is issued with > +# "freeze" argument before the filesystem is freezed. And for fsfreeze-thaw > +# request, it is issued with "thaw" argument after filesystem is thawed. > + > +LOGFILE=/var/log/qga-fsfreeze-hook.log > +FSFREEZE_D=$(dirname -- "$0")/fsfreeze-hook.d > + > +# Check whether file $1 is a backup or rpm-generated file and should be ignored > +is_ignored_file() { > + case "$1" in > + *~ | *.bak | *.orig | *.rpmnew | *.rpmorig | *.rpmsave | *.sample) > + return 0 ;; > + esac > + return 1 > +} > + > +# Iterate executables in directory "fsfreeze-hook.d" with the specified args > +[ ! -d "$FSFREEZE_D" ] && exit 1 > +for file in "$FSFREEZE_D"/* ; do > + is_ignored_file "$file" && continue > + [ -x "$file" ] || continue > + echo "$(date): execute $file $@" >>$LOGFILE > + "$file" "$@" >>$LOGFILE 2>&1 > + STATUS=$? > + echo "$(date): $file finished with status=$STATUS" >>$LOGFILE > +done > + > +exit 0 > diff --git a/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample b/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > new file mode 100755 > index 0000000..c69b8ad > --- /dev/null > +++ b/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > @@ -0,0 +1,55 @@ > +#!/bin/sh > + > +# Flush MySQL tables to the disk before the filesystem is freezed. > +# At the same time, this keeps a read lock in order to avoid write accesses > +# from the other clients until the filesystem is thawed. > + > +MYSQL="/usr/bin/mysql" > +MYSQL_OPTS="-uroot" #"-prootpassword" > +FIFO=/tmp/mysql-flush.fifo > +MYSQL_CMD="$MYSQL $MYSQL_OPTS" > + > +# Check mysql is installed and the server running > +[ -x $MYSQL ] && $MYSQL_CMD < /dev/null || exit 0 > + > +flush_and_wait() { > + printf "FLUSH TABLES WITH READ LOCK \\G\n" > + read < $FIFO > + printf "UNLOCK TABLES \\G\n" > +} > + > +case "$1" in > + freeze) > + mkfifo $FIFO || exit 1 > + flush_and_wait | $MYSQL_CMD & > + # wait until every block is flushed > + while [ "$(echo 'SHOW STATUS LIKE "Key_blocks_not_flushed"' |\ > + $MYSQL_CMD | tail -1 | cut -f 2)" -gt 0 ]; do > + sleep 1 > + done > + # for InnoDB, wait until every log is flushed > + INNODB_STATUS=$(mktemp /tmp/mysql-flush.XXXXXX) > + [ $? -ne 0 ] && exit 2 > + trap "rm -f $INNODB_STATUS" SIGINT > + while :; do > + printf "SHOW ENGINE INNODB STATUS \\G" | $MYSQL_CMD > $INNODB_STATUS > + LOG_CURRENT=$(grep 'Log sequence number' $INNODB_STATUS |\ > + tr -s ' ' | cut -d' ' -f4) > + LOG_FLUSHED=$(grep 'Log flushed up to' $INNODB_STATUS |\ > + tr -s ' ' | cut -d' ' -f5) > + [ "$LOG_CURRENT" = "$LOG_FLUSHED" ] && break > + sleep 1 > + done > + rm -f $INNODB_STATUS > + ;; > + > + thaw) > + [ ! -p $FIFO ] && exit 1 > + echo > $FIFO > + rm -f $FIFO > + ;; > + > + *) > + exit 1 > + ;; > +esac >
On Fri, Dec 07, 2012 at 11:55:16AM -0200, Luiz Capitulino wrote: > On Fri, 07 Dec 2012 17:39:32 +0900 > Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > > > Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. > > - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ > > - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot > > > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > > --- > > .gitignore | 1 > > Makefile | 2 - > > scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ > > .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ > > 4 files changed, 90 insertions(+), 1 deletion(-) > > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook > > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > > > > diff --git a/.gitignore b/.gitignore > > index bd6ba1c..286822d 100644 > > --- a/.gitignore > > +++ b/.gitignore > > @@ -93,3 +93,4 @@ cscope.* > > tags > > TAGS > > *~ > > +!scripts/qemu-guest-agent/fsfreeze-hook.d > > Why? Do we expect to have *~ files in there? I think default vim configurations will copy <file> to <file>~ prior to editing. > > > diff --git a/Makefile b/Makefile > > index 9ecbcbb..466dcd7 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -248,7 +248,7 @@ clean: > > # avoid old build problems by removing potentially incorrect old files > > rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h > > rm -f qemu-options.def > > - find . -name '*.[od]' -exec rm -f {} + > > + find . -name '*.[od]' -type f -exec rm -f {} + > > What does this change have to do with this patch? Looks like it'll try (and fail, due to lack of -r option to rm) to delete the fsfreeze-hook.d directory on `make clean`. For in-tree builds at least.
On 12/07/2012 06:55 AM, Luiz Capitulino wrote: > On Fri, 07 Dec 2012 17:39:32 +0900 > Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > >> Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. >> - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ >> - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot >> >> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> >> --- >> .gitignore | 1 >> Makefile | 2 - >> scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ >> .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ >> 4 files changed, 90 insertions(+), 1 deletion(-) >> create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook >> create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample >> >> diff --git a/.gitignore b/.gitignore >> index bd6ba1c..286822d 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -93,3 +93,4 @@ cscope.* >> tags >> TAGS >> *~ >> +!scripts/qemu-guest-agent/fsfreeze-hook.d > > Why? Do we expect to have *~ files in there? What does your question have to do with the patch, which isn't even touching the pre-existing *~ line? > >> diff --git a/Makefile b/Makefile >> index 9ecbcbb..466dcd7 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -248,7 +248,7 @@ clean: >> # avoid old build problems by removing potentially incorrect old files >> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h >> rm -f qemu-options.def >> - find . -name '*.[od]' -exec rm -f {} + >> + find . -name '*.[od]' -type f -exec rm -f {} + > > What does this change have to do with this patch? Because this patch introduces a directory scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample that belongs to the repo and therefore must not be cleaned.
On Fri, 7 Dec 2012 11:47:24 -0600 mdroth <mdroth@linux.vnet.ibm.com> wrote: > On Fri, Dec 07, 2012 at 11:55:16AM -0200, Luiz Capitulino wrote: > > On Fri, 07 Dec 2012 17:39:32 +0900 > > Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > > > > > Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. > > > - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ > > > - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot > > > > > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > > > --- > > > .gitignore | 1 > > > Makefile | 2 - > > > scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ > > > .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ > > > 4 files changed, 90 insertions(+), 1 deletion(-) > > > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook > > > create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > > > > > > diff --git a/.gitignore b/.gitignore > > > index bd6ba1c..286822d 100644 > > > --- a/.gitignore > > > +++ b/.gitignore > > > @@ -93,3 +93,4 @@ cscope.* > > > tags > > > TAGS > > > *~ > > > +!scripts/qemu-guest-agent/fsfreeze-hook.d > > > > Why? Do we expect to have *~ files in there? > > I think default vim configurations will copy <file> to <file>~ prior to > editing. But should that be tracked by git specifically in that dir? > > > diff --git a/Makefile b/Makefile > > > index 9ecbcbb..466dcd7 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -248,7 +248,7 @@ clean: > > > # avoid old build problems by removing potentially incorrect old files > > > rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h > > > rm -f qemu-options.def > > > - find . -name '*.[od]' -exec rm -f {} + > > > + find . -name '*.[od]' -type f -exec rm -f {} + > > > > What does this change have to do with this patch? > > Looks like it'll try (and fail, due to lack of -r option to rm) to > delete the fsfreeze-hook.d directory on `make clean`. For in-tree builds > at least. I can't see it, but well, I don't want to hold this patch for minor things either... It looks good for me apart from these two nitpicks.
On Fri, 07 Dec 2012 11:22:54 -0700 Eric Blake <eblake@redhat.com> wrote: > On 12/07/2012 06:55 AM, Luiz Capitulino wrote: > > On Fri, 07 Dec 2012 17:39:32 +0900 > > Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> wrote: > > > >> Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. > >> - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ > >> - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot > >> > >> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > >> --- > >> .gitignore | 1 > >> Makefile | 2 - > >> scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ > >> .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ > >> 4 files changed, 90 insertions(+), 1 deletion(-) > >> create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook > >> create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample > >> > >> diff --git a/.gitignore b/.gitignore > >> index bd6ba1c..286822d 100644 > >> --- a/.gitignore > >> +++ b/.gitignore > >> @@ -93,3 +93,4 @@ cscope.* > >> tags > >> TAGS > >> *~ > >> +!scripts/qemu-guest-agent/fsfreeze-hook.d > > > > Why? Do we expect to have *~ files in there? > > What does your question have to do with the patch, which isn't even > touching the pre-existing *~ line? As far I could understand the ! prefix in gitignore documention, it's changing the *~ meaning for the fsfreeze-hoo.d directory, and I wanted to understand why. > >> diff --git a/Makefile b/Makefile > >> index 9ecbcbb..466dcd7 100644 > >> --- a/Makefile > >> +++ b/Makefile > >> @@ -248,7 +248,7 @@ clean: > >> # avoid old build problems by removing potentially incorrect old files > >> rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h > >> rm -f qemu-options.def > >> - find . -name '*.[od]' -exec rm -f {} + > >> + find . -name '*.[od]' -type f -exec rm -f {} + > > > > What does this change have to do with this patch? > > Because this patch introduces a directory > scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample that > belongs to the repo and therefore must not be cleaned.
On 12/07/2012 01:39 AM, Tomoki Sekiyama wrote: > Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. > - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ > - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > --- > @@ -0,0 +1,33 @@ > +#!/bin/sh > + > +# This script is executed when a guest agent receives fsfreeze-freeze and > +# fsfreeze-thaw command, if it is specified in --fsfreeze-hook (-F) > +# option of qemu-ga or placed in default path (/etc/qemu/fsfreeze-hook). > +# When the agent receives fsfreeze-freeze request, this script is issued with > +# "freeze" argument before the filesystem is freezed. And for fsfreeze-thaw s/freezed/frozen/ > + > +# Iterate executables in directory "fsfreeze-hook.d" with the specified args > +[ ! -d "$FSFREEZE_D" ] && exit 1 Do you really want to fail the entire operation if the directory doesn't exist? Shouldn't you instead exit 0 because there is nothing to do? > +for file in "$FSFREEZE_D"/* ; do > + is_ignored_file "$file" && continue > + [ -x "$file" ] || continue > + echo "$(date): execute $file $@" >>$LOGFILE This is unsafe (although the worst that will happen is a poor message to the log file). $file might contain backslash, and echo cannot portably be mixed with backslash. Use printf(1) instead. > + "$file" "$@" >>$LOGFILE 2>&1 > + STATUS=$? > + echo "$(date): $file finished with status=$STATUS" >>$LOGFILE Again. > @@ -0,0 +1,55 @@ > +#!/bin/sh > + > +# Flush MySQL tables to the disk before the filesystem is freezed. s/freezed/frozen/ > +# At the same time, this keeps a read lock in order to avoid write accesses > +# from the other clients until the filesystem is thawed. > + > +MYSQL="/usr/bin/mysql" > +MYSQL_OPTS="-uroot" #"-prootpassword" > +FIFO=/tmp/mysql-flush.fifo > +MYSQL_CMD="$MYSQL $MYSQL_OPTS" > + > +# Check mysql is installed and the server running > +[ -x $MYSQL ] && $MYSQL_CMD < /dev/null || exit 0 Safe as written, since you just initialized $MYSQL above; but risky, since the mere use of MYSQL in the initialization might encourage someone to point to an alternate path that includes spaces. Then again, if they do that, then MYSQL_CMD is broken. It might be better to be explicit and write [ -x "$MYSQL" ] && "$MYSQL" $MYSQL_OPTS < /dev/null but I won't insist. > + # for InnoDB, wait until every log is flushed > + INNODB_STATUS=$(mktemp /tmp/mysql-flush.XXXXXX) > + [ $? -ne 0 ] && exit 2 > + trap "rm -f $INNODB_STATUS" SIGINT POSIX says that 'trap foo INT' is required to work, but 'trap foo SIGINT' is optional. Also, shouldn't you also worry about HUP, ALRM, and TERM?
On 12/07/2012 11:31 AM, Luiz Capitulino wrote: >>>> +++ b/.gitignore >>>> @@ -93,3 +93,4 @@ cscope.* >>>> tags >>>> TAGS >>>> *~ >>>> +!scripts/qemu-guest-agent/fsfreeze-hook.d >>> >>> Why? Do we expect to have *~ files in there? >> >> What does your question have to do with the patch, which isn't even >> touching the pre-existing *~ line? > > As far I could understand the ! prefix in gitignore documention, it's > changing the *~ meaning for the fsfreeze-hoo.d directory, and I wanted > to understand why. > No, it is changing a much earlier line: *.d to say that this _particular_ .d is allowed to be committed. It has nothing to do with *~. Still, I have to wonder if we really want to store these files in a .d in the repository itself, or if we should store them under some other file name and only at 'make install' time insert them into a .d at the install destination. It would clean up this confusion about the .gitignore as well as the change to the 'find' command during 'make clean'.
On Fri, 07 Dec 2012 11:37:44 -0700 Eric Blake <eblake@redhat.com> wrote: > On 12/07/2012 11:31 AM, Luiz Capitulino wrote: > >>>> +++ b/.gitignore > >>>> @@ -93,3 +93,4 @@ cscope.* > >>>> tags > >>>> TAGS > >>>> *~ > >>>> +!scripts/qemu-guest-agent/fsfreeze-hook.d > >>> > >>> Why? Do we expect to have *~ files in there? > >> > >> What does your question have to do with the patch, which isn't even > >> touching the pre-existing *~ line? > > > > As far I could understand the ! prefix in gitignore documention, it's > > changing the *~ meaning for the fsfreeze-hoo.d directory, and I wanted > > to understand why. > > > > No, it is changing a much earlier line: > > *.d > > to say that this _particular_ .d is allowed to be committed. It has > nothing to do with *~. Ah, now it makes a lot of sense, thanks Eric. It would be nice to move it right below *.d, to avoid stupid comments :) > Still, I have to wonder if we really want to store these files in a .d > in the repository itself, or if we should store them under some other > file name and only at 'make install' time insert them into a .d at the > install destination. It would clean up this confusion about the > .gitignore as well as the change to the 'find' command during 'make clean'. Well, now that I understand it both ways are fine with me (ie. what you suggest and what's been implemented in this patch).
Hi, sorry for my late reply. On 2012/12/08 3:57, Luiz Capitulino wrote: > On Fri, 07 Dec 2012 11:37:44 -0700 > Eric Blake <eblake@redhat.com> wrote: > >> On 12/07/2012 11:31 AM, Luiz Capitulino wrote: >>>>>> +++ b/.gitignore >>>>>> @@ -93,3 +93,4 @@ cscope.* >>>>>> tags >>>>>> TAGS >>>>>> *~ >>>>>> +!scripts/qemu-guest-agent/fsfreeze-hook.d >>>>> >>>>> Why? Do we expect to have *~ files in there? >>>> >>>> What does your question have to do with the patch, which isn't even >>>> touching the pre-existing *~ line? >>> >>> As far I could understand the ! prefix in gitignore documention, it's >>> changing the *~ meaning for the fsfreeze-hoo.d directory, and I wanted >>> to understand why. >>> >> >> No, it is changing a much earlier line: >> >> *.d >> >> to say that this _particular_ .d is allowed to be committed. It has >> nothing to do with *~. Yes, it's for adding fsfreeze-hook.d into the repo. > Ah, now it makes a lot of sense, thanks Eric. > > It would be nice to move it right below *.d, to avoid stupid comments :) And OK, I will move this there. >> Still, I have to wonder if we really want to store these files in a .d >> in the repository itself, or if we should store them under some other >> file name and only at 'make install' time insert them into a .d at the >> install destination. It would clean up this confusion about the >> .gitignore as well as the change to the 'find' command during 'make clean'. > > Well, now that I understand it both ways are fine with me (ie. what you > suggest and what's been implemented in this patch). Then I'd like to keep current implementation. Thanks,
Hi Eric, Thanks for your review. On 2012/12/08 3:34, Eric Blake wrote: > On 12/07/2012 01:39 AM, Tomoki Sekiyama wrote: >> Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. >> - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ >> - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot >> >> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> >> --- > >> @@ -0,0 +1,33 @@ >> +#!/bin/sh >> + >> +# This script is executed when a guest agent receives fsfreeze-freeze and >> +# fsfreeze-thaw command, if it is specified in --fsfreeze-hook (-F) >> +# option of qemu-ga or placed in default path (/etc/qemu/fsfreeze-hook). >> +# When the agent receives fsfreeze-freeze request, this script is issued with >> +# "freeze" argument before the filesystem is freezed. And for fsfreeze-thaw > > s/freezed/frozen/ Oops... >> + >> +# Iterate executables in directory "fsfreeze-hook.d" with the specified args >> +[ ! -d "$FSFREEZE_D" ] && exit 1 > > Do you really want to fail the entire operation if the directory doesn't > exist? Shouldn't you instead exit 0 because there is nothing to do? I thought that was installation failure, but this might be too cautious. Exiting 0 is also reasonable, so I will change this. >> +for file in "$FSFREEZE_D"/* ; do >> + is_ignored_file "$file" && continue >> + [ -x "$file" ] || continue >> + echo "$(date): execute $file $@" >>$LOGFILE > > This is unsafe (although the worst that will happen is a poor message to > the log file). $file might contain backslash, and echo cannot portably > be mixed with backslash. Use printf(1) instead. OK, I will use printf here, and >> + "$file" "$@" >>$LOGFILE 2>&1 >> + STATUS=$? >> + echo "$(date): $file finished with status=$STATUS" >>$LOGFILE > > Again. here too. >> @@ -0,0 +1,55 @@ >> +#!/bin/sh >> + >> +# Flush MySQL tables to the disk before the filesystem is freezed. > > s/freezed/frozen/ > >> +# At the same time, this keeps a read lock in order to avoid write accesses >> +# from the other clients until the filesystem is thawed. >> + >> +MYSQL="/usr/bin/mysql" >> +MYSQL_OPTS="-uroot" #"-prootpassword" >> +FIFO=/tmp/mysql-flush.fifo >> +MYSQL_CMD="$MYSQL $MYSQL_OPTS" >> + >> +# Check mysql is installed and the server running >> +[ -x $MYSQL ] && $MYSQL_CMD < /dev/null || exit 0 > > Safe as written, since you just initialized $MYSQL above; but risky, > since the mere use of MYSQL in the initialization might encourage > someone to point to an alternate path that includes spaces. Then again, > if they do that, then MYSQL_CMD is broken. It might be better to be > explicit and write > [ -x "$MYSQL" ] && "$MYSQL" $MYSQL_OPTS < /dev/null > but I won't insist. OK, I will replace $MYSQL_CMD with "$MYSQL" $MYSQL_OPTS. >> + # for InnoDB, wait until every log is flushed >> + INNODB_STATUS=$(mktemp /tmp/mysql-flush.XXXXXX) >> + [ $? -ne 0 ] && exit 2 >> + trap "rm -f $INNODB_STATUS" SIGINT > > POSIX says that 'trap foo INT' is required to work, but 'trap foo > SIGINT' is optional. Also, shouldn't you also worry about HUP, ALRM, > and TERM? I will fix this to trap "..." HUP INT QUIT ALRM TERM Thanks,
diff --git a/.gitignore b/.gitignore index bd6ba1c..286822d 100644 --- a/.gitignore +++ b/.gitignore @@ -93,3 +93,4 @@ cscope.* tags TAGS *~ +!scripts/qemu-guest-agent/fsfreeze-hook.d diff --git a/Makefile b/Makefile index 9ecbcbb..466dcd7 100644 --- a/Makefile +++ b/Makefile @@ -248,7 +248,7 @@ clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h rm -f qemu-options.def - find . -name '*.[od]' -exec rm -f {} + + find . -name '*.[od]' -type f -exec rm -f {} + rm -f *.a *.lo $(TOOLS) $(HELPERS-y) qemu-ga TAGS cscope.* *.pod *~ */*~ rm -Rf .libs rm -f qemu-img-cmds.h diff --git a/scripts/qemu-guest-agent/fsfreeze-hook b/scripts/qemu-guest-agent/fsfreeze-hook new file mode 100755 index 0000000..4f7ff15 --- /dev/null +++ b/scripts/qemu-guest-agent/fsfreeze-hook @@ -0,0 +1,33 @@ +#!/bin/sh + +# This script is executed when a guest agent receives fsfreeze-freeze and +# fsfreeze-thaw command, if it is specified in --fsfreeze-hook (-F) +# option of qemu-ga or placed in default path (/etc/qemu/fsfreeze-hook). +# When the agent receives fsfreeze-freeze request, this script is issued with +# "freeze" argument before the filesystem is freezed. And for fsfreeze-thaw +# request, it is issued with "thaw" argument after filesystem is thawed. + +LOGFILE=/var/log/qga-fsfreeze-hook.log +FSFREEZE_D=$(dirname -- "$0")/fsfreeze-hook.d + +# Check whether file $1 is a backup or rpm-generated file and should be ignored +is_ignored_file() { + case "$1" in + *~ | *.bak | *.orig | *.rpmnew | *.rpmorig | *.rpmsave | *.sample) + return 0 ;; + esac + return 1 +} + +# Iterate executables in directory "fsfreeze-hook.d" with the specified args +[ ! -d "$FSFREEZE_D" ] && exit 1 +for file in "$FSFREEZE_D"/* ; do + is_ignored_file "$file" && continue + [ -x "$file" ] || continue + echo "$(date): execute $file $@" >>$LOGFILE + "$file" "$@" >>$LOGFILE 2>&1 + STATUS=$? + echo "$(date): $file finished with status=$STATUS" >>$LOGFILE +done + +exit 0 diff --git a/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample b/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample new file mode 100755 index 0000000..c69b8ad --- /dev/null +++ b/scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample @@ -0,0 +1,55 @@ +#!/bin/sh + +# Flush MySQL tables to the disk before the filesystem is freezed. +# At the same time, this keeps a read lock in order to avoid write accesses +# from the other clients until the filesystem is thawed. + +MYSQL="/usr/bin/mysql" +MYSQL_OPTS="-uroot" #"-prootpassword" +FIFO=/tmp/mysql-flush.fifo +MYSQL_CMD="$MYSQL $MYSQL_OPTS" + +# Check mysql is installed and the server running +[ -x $MYSQL ] && $MYSQL_CMD < /dev/null || exit 0 + +flush_and_wait() { + printf "FLUSH TABLES WITH READ LOCK \\G\n" + read < $FIFO + printf "UNLOCK TABLES \\G\n" +} + +case "$1" in + freeze) + mkfifo $FIFO || exit 1 + flush_and_wait | $MYSQL_CMD & + # wait until every block is flushed + while [ "$(echo 'SHOW STATUS LIKE "Key_blocks_not_flushed"' |\ + $MYSQL_CMD | tail -1 | cut -f 2)" -gt 0 ]; do + sleep 1 + done + # for InnoDB, wait until every log is flushed + INNODB_STATUS=$(mktemp /tmp/mysql-flush.XXXXXX) + [ $? -ne 0 ] && exit 2 + trap "rm -f $INNODB_STATUS" SIGINT + while :; do + printf "SHOW ENGINE INNODB STATUS \\G" | $MYSQL_CMD > $INNODB_STATUS + LOG_CURRENT=$(grep 'Log sequence number' $INNODB_STATUS |\ + tr -s ' ' | cut -d' ' -f4) + LOG_FLUSHED=$(grep 'Log flushed up to' $INNODB_STATUS |\ + tr -s ' ' | cut -d' ' -f5) + [ "$LOG_CURRENT" = "$LOG_FLUSHED" ] && break + sleep 1 + done + rm -f $INNODB_STATUS + ;; + + thaw) + [ ! -p $FIFO ] && exit 1 + echo > $FIFO + rm -f $FIFO + ;; + + *) + exit 1 + ;; +esac
Adds sample hook scripts for --fsfreeze-hook option of qemu-ga. - fsfreeze-hook : execute scripts in fsfreeze-hook.d/ - fsfreeze-hook.d/mysql-flush.sh.sample : quiesce MySQL before snapshot Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> --- .gitignore | 1 Makefile | 2 - scripts/qemu-guest-agent/fsfreeze-hook | 33 ++++++++++++ .../fsfreeze-hook.d/mysql-flush.sh.sample | 55 ++++++++++++++++++++ 4 files changed, 90 insertions(+), 1 deletion(-) create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook create mode 100755 scripts/qemu-guest-agent/fsfreeze-hook.d/mysql-flush.sh.sample