[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]