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

* 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,