[libvirt] [PATCH] qemu: work around dash 0.5.5 bug in managed save

[Stefan Berger]

  On 10/22/2010 07:29 PM, Eric Blake wrote:
> * configure.ac (VIR_WRAPPER_SHELL): Define to a replacement shell,
> if /bin/sh is broken on<>  redirection.
> * src/qemu/qemu_monitor.h (VIR_WRAPPER_SHELL_PREFIX)
> (VIR_WRAPPER_SHELL_SUFFIX): New macros.
> * src/qemu/qemu_monitor_text.c (qemuMonitorTextMigrateToFile): Use
> them.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMigrateToFile):
> Likewise.
> ---
>
> Took longer than I thought, but this should do the trick with no
> overhead on decent systems, and while still avoiding buggy dash ==
> /bin/sh with something that works elsewhere.
>
> This passes for me on Fedora, where /bin/sh is bash; testing on
> Ubuntu or other debian-based system where /bin/sh is dash 0.5.5
> would be appreciated.
>
>   configure.ac                 |   50 ++++++++++++++++++++++++++++++++++++++++++
>   src/qemu/qemu_monitor.h      |   13 +++++++++++
>   src/qemu/qemu_monitor_json.c |    5 ++-
>   src/qemu/qemu_monitor_text.c |    5 ++-
>   4 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index e41f2b5..6aa09f7 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -583,6 +583,56 @@ if test "$with_lxc" = "yes" ; then
>   fi
>   AM_CONDITIONAL([WITH_LXC], [test "$with_lxc" = "yes"])
>
> +dnl
> +dnl check for shell that understands<>  redirection without truncation,
> +dnl needed by src/qemu/qemu_monitor_{text,json}.c.
> +dnl
> +if test "$with_qemu" = yes; then
> +  lv_wrapper_shell=
> +  AC_CACHE_CHECK([for shell that supports<>  redirection],
> +    [lv_cv_wrapper_shell],
> +    [
> +    # If cross-compiling, guess that /bin/sh is good enough except for
> +    # Linux, where it might be dash 0.5.5 which is known broken; and on
> +    # Linux, we have a good chance that /bin/bash will exist.
> +    # If we guess wrong, a user can override the cache variable.
> +    # Going through /bin/bash is a slight slowdown if /bin/sh works.
> +    if test "$cross_compiling" = yes; then
> +      case $host_os in
> +        linux*) lv_cv_wrapper_shell=/bin/bash ;;
> +        *) lv_cv_wrapper_shell=/bin/sh ;;
> +      esac
> +    else
> +      for lv_cv_wrapper_shell in /bin/sh bash ksh zsh none; do
> +        test $lv_cv_wrapper_shell = none&&
> +          AC_MSG_ERROR([could not find decent shell])
> +        echo a>  conftest.a
> +        $lv_cv_wrapper_shell -c ': 1<>conftest.a'
> +        case `cat conftest.a`.$lv_cv_wrapper_shell in
> +          a./*) break;; dnl /bin/sh is good enough
> +          a.*) dnl bash, ksh, and zsh all understand 'command', use that
> +               dnl to determine the absolute path of the shell
> +            lv_cv_wrapper_shell=`$lv_cv_wrapper_shell -c \
> +              'command -v $lv_cv_wrapper_shell'`
> +            case $lv_cv_wrapper_shell in
> +              /*) break;;
> +            esac
> +            ;;
> +        esac
> +      done
> +      rm -f conftest.a
> +    fi
> +  ])
> +  if test "x$lv_cv_wrapper_shell" != x/bin/sh; then
> +    lv_wrapper_shell=$lv_cv_wrapper_shell
> +  fi
> +  if test "x$lv_wrapper_shell" != x; then
> +    AC_DEFINE_UNQUOTED([VIR_WRAPPER_SHELL], [$lv_wrapper_shell],
> +      [Define to the absolute path of a shell that does not truncate on
> +<>  redirection, if /bin/sh does not fit the bill])
> +  fi
> +fi
> +
>
>   dnl
>   dnl check for kernel headers required by src/bridge.c
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 48f4c20..7d09145 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -391,4 +391,17 @@ int qemuMonitorDeleteSnapshot(qemuMonitorPtr mon, const char *name);
>
>   int qemuMonitorArbitraryCommand(qemuMonitorPtr mon, const char *cmd, char **reply);
>
> +/**
> + * When running two dd process and using<>  redirection, we need a
> + * shell that will not truncate files.  These two strings serve that
> + * purpose.
> + */
> +# ifdef VIR_WRAPPER_SHELL
> +#  define VIR_WRAPPER_SHELL_PREFIX VIR_WRAPPER_SHELL " -c '"
> +#  define VIR_WRAPPER_SHELL_SUFFIX "'"
> +# else
> +#  define VIR_WRAPPER_SHELL_PREFIX /* nothing */
> +#  define VIR_WRAPPER_SHELL_SUFFIX /* nothing */
> +# endif
> +
>   #endif /* QEMU_MONITOR_H */
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index d3ab25f..d2c6f0a 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -1698,8 +1698,9 @@ int qemuMonitorJSONMigrateToFile(qemuMonitorPtr mon,
>        * allow starting at an alignment of 512, but without wasting
>        * padding to get to the larger alignment useful for speed.  Use
>        *<>  redirection to avoid truncating a regular file.  */
> -    if (virAsprintf(&dest, "exec:%s | { dd bs=%llu seek=%llu if=/dev/null&&  "
> -                    "dd bs=%llu; } 1<>%s",
> +    if (virAsprintf(&dest, "exec:" VIR_WRAPPER_SHELL_PREFIX "%s | "
> +                    "{ dd bs=%llu seek=%llu if=/dev/null&&  "
> +                    "dd bs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX,
>                       argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS,
>                       offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS,
>                       QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE,
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 69971a6..d7e128c 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -1277,8 +1277,9 @@ int qemuMonitorTextMigrateToFile(qemuMonitorPtr mon,
>        * allow starting at an alignment of 512, but without wasting
>        * padding to get to the larger alignment useful for speed.  Use
>        *<>  redirection to avoid truncating a regular file.  */
> -    if (virAsprintf(&dest, "exec:%s | { dd bs=%llu seek=%llu if=/dev/null&&  "
> -                    "dd bs=%llu; } 1<>%s",
> +    if (virAsprintf(&dest, "exec:" VIR_WRAPPER_SHELL_PREFIX "%s | "
> +                    "{ dd bs=%llu seek=%llu if=/dev/null&&  "
> +                    "dd bs=%llu; } 1<>%s" VIR_WRAPPER_SHELL_SUFFIX,
>                       argstr, QEMU_MONITOR_MIGRATE_TO_FILE_BS,
>                       offset / QEMU_MONITOR_MIGRATE_TO_FILE_BS,
>                       QEMU_MONITOR_MIGRATE_TO_FILE_TRANSFER_SIZE,

Cannot test it, but looks good. ACK.

     Stefan