[PATCH 0/3] fsfreeze-hook: fix bashisms, syslog fallback logic

[PATCH 0/3] fsfreeze-hook: fix bashisms, syslog fallback logic

[Peter Maydell]

This patchset fixes some issues with the fsfreeze-hook guest-agent
script reported in https://gitlab.com/qemu-project/qemu/-/work_items/3339

 * the script is a #!/bin/sh one, but it uses various bash-isms.
   Most are easy fixes, but getting rid of the PIPESTATUS usage
   is a little less obvious, so that gets its own patch
 * a bug in the logic determining whether to log to file or syslog
   means that if the logfile doesn't exist on first run, the script
   will create the logfile but log to syslog (and then log to file
   on second and subsequent runs)

NB: I have tested this by setting up the script and running it
manually in a temp directory, but not as a "real world" fsfreeze.

thanks
-- PMM

Peter Maydell (3):
  scripts/qemu-guest-agent/fsfreeze-hook: Avoid bash-isms
  scripts/qemu-guest-agent/fsfreeze-hook: Avoid use of PIPESTATUS
  scripts/qemu-guest-agent/fsfreeze-hook: Fix syslog-fallback logic

 scripts/qemu-guest-agent/fsfreeze-hook | 34 ++++++++++++++++++--------
 1 file changed, 24 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH 1/3] scripts/qemu-guest-agent/fsfreeze-hook: Avoid bash-isms

[Peter Maydell]

The fsfreeze-hook script starts with #!/bin/sh, but it uses
several bash-specific constructs, resulting in misbehaviour
on guest systems where /bin/sh is some other POSIX shell.

Fix the simple ones reported by shellcheck:

In scripts/qemu-guest-agent/fsfreeze-hook line 27:
touch "$LOGFILE" &>/dev/null || USE_SYSLOG=1
                 ^---------^ SC3020 (warning): In POSIX sh, &> is undefined.

In scripts/qemu-guest-agent/fsfreeze-hook line 31:
    local message="$1"
    ^-----------^ SC3043 (warning): In POSIX sh, 'local' is undefined.

In scripts/qemu-guest-agent/fsfreeze-hook line 46:
    log_message "Executing $file $@"
                                 ^-- SC2145 (error): Argument mixes string and array. Use * or separate argument.

In scripts/qemu-guest-agent/fsfreeze-hook line 55:
    if [ $STATUS -ne 0 ]; then
         ^-----^ SC2086 (info): Double quote to prevent globbing and word splitting.

There is also a use of PIPESTATUS that is more complex to fix;
that will be dealt with in a separate commit.

Cc: qemu-stable@nongnu.org
Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of logging error")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/qemu-guest-agent/fsfreeze-hook | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/scripts/qemu-guest-agent/fsfreeze-hook b/scripts/qemu-guest-agent/fsfreeze-hook
index 5b915af017..6e2d7588af 100755
--- a/scripts/qemu-guest-agent/fsfreeze-hook
+++ b/scripts/qemu-guest-agent/fsfreeze-hook
@@ -24,15 +24,14 @@ USE_SYSLOG=0
 # if log file is not writable, fallback to syslog
 [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
 # try to update log file and fallback to syslog if it fails
-touch "$LOGFILE" &>/dev/null || USE_SYSLOG=1
+touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1
 
 # Ensure the log file is writable, fallback to syslog if not
 log_message() {
-    local message="$1"
     if [ "$USE_SYSLOG" -eq 0 ]; then
-        printf "%s: %s\n" "$(date)" "$message" >>"$LOGFILE"
+        printf "%s: %s\n" "$(date)" "$1" >>"$LOGFILE"
     else
-        logger -t qemu-ga-freeze-hook "$message"
+        logger -t qemu-ga-freeze-hook "$1"
     fi
 }
 
@@ -43,7 +42,7 @@ for file in "$FSFREEZE_D"/* ; do
     is_ignored_file "$file" && continue
     [ -x "$file" ] || continue
 
-    log_message "Executing $file $@"
+    log_message "Executing $file $*"
     if [ "$USE_SYSLOG" -eq 0 ]; then
         "$file" "$@" >>"$LOGFILE" 2>&1
         STATUS=$?
@@ -52,7 +51,7 @@ for file in "$FSFREEZE_D"/* ; do
         STATUS=${PIPESTATUS[0]}
     fi
 
-    if [ $STATUS -ne 0 ]; then
+    if [ "$STATUS" -ne 0 ]; then
         log_message "Error: $file finished with status=$STATUS"
     else
         log_message "$file finished successfully"
-- 
2.43.0

[PATCH 2/3] scripts/qemu-guest-agent/fsfreeze-hook: Avoid use of PIPESTATUS

[Peter Maydell]

PIPESTATUS is a bash-specific construct, and this script is supposed
to be POSIX shell. We only use it in one place, to capture the exit
status of a command whose output we are piping to 'logger'.

Replace the PIPESTATUS usage with the trick described in
https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
which uses a command-group to capture the status of the
first process in the pipeline.

Cc: qemu-stable@nongnu.org
Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of logging error")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3339
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/qemu-guest-agent/fsfreeze-hook | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu-guest-agent/fsfreeze-hook b/scripts/qemu-guest-agent/fsfreeze-hook
index 6e2d7588af..21eb5c5145 100755
--- a/scripts/qemu-guest-agent/fsfreeze-hook
+++ b/scripts/qemu-guest-agent/fsfreeze-hook
@@ -47,8 +47,23 @@ for file in "$FSFREEZE_D"/* ; do
         "$file" "$@" >>"$LOGFILE" 2>&1
         STATUS=$?
     else
-        "$file" "$@" 2>&1 | logger -t qemu-ga-freeze-hook
-        STATUS=${PIPESTATUS[0]}
+        # We want to pipe the output of $file through 'logger' and also
+        # capture its exit status. Since we are a POSIX script we can't
+        # use PIPESTATUS, so instead this is a trick borrowed from
+        # https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
+        # which uses command-groups and redirection to get the exit status.
+        # This is equivalent to
+        #   "$file" "$@" 2>&1 | logger -t qemu-ga-freeze-hook
+        # plus setting the exit status of the pipe to the exit
+        # status of the first command rather than the last one.
+        { { { {
+                "$file" "$@" 2>&1 3>&- 4>&-
+                echo $? >&3
+            } | logger -t qemu-ga-freeze-hook >&4
+            } 3>&1
+          } | { read -r xs ; exit "$xs"; }
+        } 4>&1
+        STATUS=$?
     fi
 
     if [ "$STATUS" -ne 0 ]; then
-- 
2.43.0

[PATCH 3/3] scripts/qemu-guest-agent/fsfreeze-hook: Fix syslog-fallback logic

[Peter Maydell]

In the fsfreeze script we attempt to implement "log to a file if we
can, and fall back to syslog if we cannot".  We do this with:

  [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
  touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1

This has a weird behaviour if it is run in a setup where we have
permissions that would allow us to write to $LOGFILE but it does not
currently exist.  On the first execution, the '-w' fails and so we
set USE_SYSLOG=1.  But since we also do the "touch $LOGFILE" step we
create an empty logfile.  Then on the second time the script is
executed, we see a writeable logfile and will use it.  The effect is
"log to syslog once, then to the logfile thereafter", which is not
likely to be what anybody wants.

Update the condition of the first check to only pick syslog if
the logfile exists but is not writable. This means that:
 * if the logfile doesn't exist but we are able to create it,
   we will create it and use it
 * if the logfile already exists and we can write to it,
   we will use it
 * if the logfile already exists but we can't write to it,
   we will fall back to syslog
 * if the logfile doesn't exist and we can't create it,
   we will fall back to syslog

Cc: qemu-stable@nongnu.org
Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of logging error")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/qemu-guest-agent/fsfreeze-hook | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qemu-guest-agent/fsfreeze-hook b/scripts/qemu-guest-agent/fsfreeze-hook
index 21eb5c5145..76669f5caf 100755
--- a/scripts/qemu-guest-agent/fsfreeze-hook
+++ b/scripts/qemu-guest-agent/fsfreeze-hook
@@ -21,8 +21,8 @@ is_ignored_file() {
 }
 
 USE_SYSLOG=0
-# if log file is not writable, fallback to syslog
-[ ! -w "$LOGFILE" ] && USE_SYSLOG=1
+# if log file exists but is not writable, fallback to syslog
+[ -e "$LOGFILE" ] && [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
 # try to update log file and fallback to syslog if it fails
 touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1
 
-- 
2.43.0

Re: [PATCH 3/3] scripts/qemu-guest-agent/fsfreeze-hook: Fix syslog-fallback logic

[Philippe Mathieu-Daudé]

On 17/3/26 10:48, Peter Maydell wrote:
> In the fsfreeze script we attempt to implement "log to a file if we
> can, and fall back to syslog if we cannot".  We do this with:
> 
>    [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
>    touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1
> 
> This has a weird behaviour if it is run in a setup where we have
> permissions that would allow us to write to $LOGFILE but it does not
> currently exist.  On the first execution, the '-w' fails and so we
> set USE_SYSLOG=1.  But since we also do the "touch $LOGFILE" step we
> create an empty logfile.  Then on the second time the script is
> executed, we see a writeable logfile and will use it.  The effect is
> "log to syslog once, then to the logfile thereafter", which is not
> likely to be what anybody wants.
> 
> Update the condition of the first check to only pick syslog if
> the logfile exists but is not writable. This means that:
>   * if the logfile doesn't exist but we are able to create it,
>     we will create it and use it
>   * if the logfile already exists and we can write to it,
>     we will use it
>   * if the logfile already exists but we can't write to it,
>     we will fall back to syslog
>   * if the logfile doesn't exist and we can't create it,
>     we will fall back to syslog
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of logging error")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   scripts/qemu-guest-agent/fsfreeze-hook | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/3] scripts/qemu-guest-agent/fsfreeze-hook: Avoid bash-isms

[Kostiantyn Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 3148 bytes --]

Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>

On Tue, Mar 17, 2026 at 11:48 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> The fsfreeze-hook script starts with #!/bin/sh, but it uses
> several bash-specific constructs, resulting in misbehaviour
> on guest systems where /bin/sh is some other POSIX shell.
>
> Fix the simple ones reported by shellcheck:
>
> In scripts/qemu-guest-agent/fsfreeze-hook line 27:
> touch "$LOGFILE" &>/dev/null || USE_SYSLOG=1
>                  ^---------^ SC3020 (warning): In POSIX sh, &> is
> undefined.
>
> In scripts/qemu-guest-agent/fsfreeze-hook line 31:
>     local message="$1"
>     ^-----------^ SC3043 (warning): In POSIX sh, 'local' is undefined.
>
> In scripts/qemu-guest-agent/fsfreeze-hook line 46:
>     log_message "Executing $file $@"
>                                  ^-- SC2145 (error): Argument mixes string
> and array. Use * or separate argument.
>
> In scripts/qemu-guest-agent/fsfreeze-hook line 55:
>     if [ $STATUS -ne 0 ]; then
>          ^-----^ SC2086 (info): Double quote to prevent globbing and word
> splitting.
>
> There is also a use of PIPESTATUS that is more complex to fix;
> that will be dealt with in a separate commit.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of
> logging error")
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/qemu-guest-agent/fsfreeze-hook | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qemu-guest-agent/fsfreeze-hook
> b/scripts/qemu-guest-agent/fsfreeze-hook
> index 5b915af017..6e2d7588af 100755
> --- a/scripts/qemu-guest-agent/fsfreeze-hook
> +++ b/scripts/qemu-guest-agent/fsfreeze-hook
> @@ -24,15 +24,14 @@ USE_SYSLOG=0
>  # if log file is not writable, fallback to syslog
>  [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
>  # try to update log file and fallback to syslog if it fails
> -touch "$LOGFILE" &>/dev/null || USE_SYSLOG=1
> +touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1
>
>  # Ensure the log file is writable, fallback to syslog if not
>  log_message() {
> -    local message="$1"
>      if [ "$USE_SYSLOG" -eq 0 ]; then
> -        printf "%s: %s\n" "$(date)" "$message" >>"$LOGFILE"
> +        printf "%s: %s\n" "$(date)" "$1" >>"$LOGFILE"
>      else
> -        logger -t qemu-ga-freeze-hook "$message"
> +        logger -t qemu-ga-freeze-hook "$1"
>      fi
>  }
>
> @@ -43,7 +42,7 @@ for file in "$FSFREEZE_D"/* ; do
>      is_ignored_file "$file" && continue
>      [ -x "$file" ] || continue
>
> -    log_message "Executing $file $@"
> +    log_message "Executing $file $*"
>      if [ "$USE_SYSLOG" -eq 0 ]; then
>          "$file" "$@" >>"$LOGFILE" 2>&1
>          STATUS=$?
> @@ -52,7 +51,7 @@ for file in "$FSFREEZE_D"/* ; do
>          STATUS=${PIPESTATUS[0]}
>      fi
>
> -    if [ $STATUS -ne 0 ]; then
> +    if [ "$STATUS" -ne 0 ]; then
>          log_message "Error: $file finished with status=$STATUS"
>      else
>          log_message "$file finished successfully"
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 4310 bytes --]

Re: [PATCH 2/3] scripts/qemu-guest-agent/fsfreeze-hook: Avoid use of PIPESTATUS

[Kostiantyn Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 2491 bytes --]

Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>

Cool magic

On Tue, Mar 17, 2026 at 11:48 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> PIPESTATUS is a bash-specific construct, and this script is supposed
> to be POSIX shell. We only use it in one place, to capture the exit
> status of a command whose output we are piping to 'logger'.
>
> Replace the PIPESTATUS usage with the trick described in
>
> https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
> which uses a command-group to capture the status of the
> first process in the pipeline.
>
> Cc: qemu-stable@nongnu.org
> Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of
> logging error")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3339
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/qemu-guest-agent/fsfreeze-hook | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qemu-guest-agent/fsfreeze-hook
> b/scripts/qemu-guest-agent/fsfreeze-hook
> index 6e2d7588af..21eb5c5145 100755
> --- a/scripts/qemu-guest-agent/fsfreeze-hook
> +++ b/scripts/qemu-guest-agent/fsfreeze-hook
> @@ -47,8 +47,23 @@ for file in "$FSFREEZE_D"/* ; do
>          "$file" "$@" >>"$LOGFILE" 2>&1
>          STATUS=$?
>      else
> -        "$file" "$@" 2>&1 | logger -t qemu-ga-freeze-hook
> -        STATUS=${PIPESTATUS[0]}
> +        # We want to pipe the output of $file through 'logger' and also
> +        # capture its exit status. Since we are a POSIX script we can't
> +        # use PIPESTATUS, so instead this is a trick borrowed from
> +        #
> https://unix.stackexchange.com/questions/14270/get-exit-status-of-process-thats-piped-to-another/70675#70675
> +        # which uses command-groups and redirection to get the exit
> status.
> +        # This is equivalent to
> +        #   "$file" "$@" 2>&1 | logger -t qemu-ga-freeze-hook
> +        # plus setting the exit status of the pipe to the exit
> +        # status of the first command rather than the last one.
> +        { { { {
> +                "$file" "$@" 2>&1 3>&- 4>&-
> +                echo $? >&3
> +            } | logger -t qemu-ga-freeze-hook >&4
> +            } 3>&1
> +          } | { read -r xs ; exit "$xs"; }
> +        } 4>&1
> +        STATUS=$?
>      fi
>
>      if [ "$STATUS" -ne 0 ]; then
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 3838 bytes --]

Re: [PATCH 3/3] scripts/qemu-guest-agent/fsfreeze-hook: Fix syslog-fallback logic

[Kostiantyn Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

Reviewed-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>

On Tue, Mar 17, 2026 at 11:54 AM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> On 17/3/26 10:48, Peter Maydell wrote:
> > In the fsfreeze script we attempt to implement "log to a file if we
> > can, and fall back to syslog if we cannot".  We do this with:
> >
> >    [ ! -w "$LOGFILE" ] && USE_SYSLOG=1
> >    touch "$LOGFILE" >/dev/null 2>&1 || USE_SYSLOG=1
> >
> > This has a weird behaviour if it is run in a setup where we have
> > permissions that would allow us to write to $LOGFILE but it does not
> > currently exist.  On the first execution, the '-w' fails and so we
> > set USE_SYSLOG=1.  But since we also do the "touch $LOGFILE" step we
> > create an empty logfile.  Then on the second time the script is
> > executed, we see a writeable logfile and will use it.  The effect is
> > "log to syslog once, then to the logfile thereafter", which is not
> > likely to be what anybody wants.
> >
> > Update the condition of the first check to only pick syslog if
> > the logfile exists but is not writable. This means that:
> >   * if the logfile doesn't exist but we are able to create it,
> >     we will create it and use it
> >   * if the logfile already exists and we can write to it,
> >     we will use it
> >   * if the logfile already exists but we can't write to it,
> >     we will fall back to syslog
> >   * if the logfile doesn't exist and we can't create it,
> >     we will fall back to syslog
> >
> > Cc: qemu-stable@nongnu.org
> > Fixes: 85978dfb6b1c133 ("qemu-ga: Optimize freeze-hook script logic of
> logging error")
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   scripts/qemu-guest-agent/fsfreeze-hook | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
>

[-- Attachment #2: Type: text/html, Size: 2742 bytes --]