qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Anthony Liguori <aliguori@us.ibm.com>
Subject: [Qemu-devel] [PATCH 09/11] block: ssh: Use libssh2_sftp_fsync (if supported by libssh2) to flush to disk.
Date: Mon, 15 Apr 2013 10:22:42 +0200	[thread overview]
Message-ID: <1366014164-17557-10-git-send-email-stefanha@redhat.com> (raw)
In-Reply-To: <1366014164-17557-1-git-send-email-stefanha@redhat.com>

From: "Richard W.M. Jones" <rjones@redhat.com>

libssh2_sftp_fsync is an extension to libssh2 to support fsync(2) over
sftp, which is itself an extension of OpenSSH.

If both libssh2 and the ssh daemon support it, this will allow
bdrv_flush_to_disk to commit changes through to disk on the remote
server.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/ssh.c   | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 configure     | 25 +++++++++++++++++++
 qemu-doc.texi | 15 ++++++++----
 3 files changed, 108 insertions(+), 10 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 89a9017..8f78e2e 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -72,6 +72,10 @@ typedef struct BDRVSSHState {
      * updated if it changes (eg by writing at the end of the file).
      */
     LIBSSH2_SFTP_ATTRIBUTES attrs;
+
+    /* Used to warn if 'flush' is not supported. */
+    char *hostport;
+    bool unsafe_flush_warning;
 } BDRVSSHState;
 
 static void ssh_state_init(BDRVSSHState *s)
@@ -84,6 +88,7 @@ static void ssh_state_init(BDRVSSHState *s)
 
 static void ssh_state_free(BDRVSSHState *s)
 {
+    g_free(s->hostport);
     if (s->sftp_handle) {
         libssh2_sftp_close(s->sftp_handle);
     }
@@ -479,7 +484,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
     Error *err = NULL;
     const char *host, *user, *path, *host_key_check;
     int port;
-    char *hostport = NULL;
 
     host = qdict_get_str(options, "host");
 
@@ -507,9 +511,12 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
         host_key_check = "yes";
     }
 
+    /* Construct the host:port name for inet_connect. */
+    g_free(s->hostport);
+    s->hostport = g_strdup_printf("%s:%d", host, port);
+
     /* Open the socket and connect. */
-    hostport = g_strdup_printf("%s:%d", host, port);
-    s->sock = inet_connect(hostport, &err);
+    s->sock = inet_connect(s->hostport, &err);
     if (err != NULL) {
         ret = -errno;
         qerror_report_err(err);
@@ -581,7 +588,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
     qdict_del(options, "path");
     qdict_del(options, "host_key_check");
 
-    g_free(hostport);
     return 0;
 
  err:
@@ -600,7 +606,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
         libssh2_session_free(s->session);
     }
     s->session = NULL;
-    g_free(hostport);
 
     return ret;
 }
@@ -953,6 +958,68 @@ static coroutine_fn int ssh_co_writev(BlockDriverState *bs,
     return ret;
 }
 
+static void unsafe_flush_warning(BDRVSSHState *s, const char *what)
+{
+    if (!s->unsafe_flush_warning) {
+        error_report("warning: ssh server %s does not support fsync",
+                     s->hostport);
+        if (what) {
+            error_report("to support fsync, you need %s", what);
+        }
+        s->unsafe_flush_warning = true;
+    }
+}
+
+#ifdef HAS_LIBSSH2_SFTP_FSYNC
+
+static coroutine_fn int ssh_flush(BDRVSSHState *s)
+{
+    int r;
+
+    DPRINTF("fsync");
+ again:
+    r = libssh2_sftp_fsync(s->sftp_handle);
+    if (r == LIBSSH2_ERROR_EAGAIN || r == LIBSSH2_ERROR_TIMEOUT) {
+        co_yield(s);
+        goto again;
+    }
+    if (r == LIBSSH2_ERROR_SFTP_PROTOCOL &&
+        libssh2_sftp_last_error(s->sftp) == LIBSSH2_FX_OP_UNSUPPORTED) {
+        unsafe_flush_warning(s, "OpenSSH >= 6.3");
+        return 0;
+    }
+    if (r < 0) {
+        sftp_error_report(s, "fsync failed");
+        return -EIO;
+    }
+
+    return 0;
+}
+
+static coroutine_fn int ssh_co_flush(BlockDriverState *bs)
+{
+    BDRVSSHState *s = bs->opaque;
+    int ret;
+
+    qemu_co_mutex_lock(&s->lock);
+    ret = ssh_flush(s);
+    qemu_co_mutex_unlock(&s->lock);
+
+    return ret;
+}
+
+#else /* !HAS_LIBSSH2_SFTP_FSYNC */
+
+static coroutine_fn int ssh_co_flush(BlockDriverState *bs)
+{
+    BDRVSSHState *s = bs->opaque;
+
+    unsafe_flush_warning(s, "libssh2 >= 1.4.4");
+    return 0;
+}
+
+#endif /* !HAS_LIBSSH2_SFTP_FSYNC */
+
 static int64_t ssh_getlength(BlockDriverState *bs)
 {
     BDRVSSHState *s = bs->opaque;
@@ -976,6 +1043,7 @@ static BlockDriver bdrv_ssh = {
     .bdrv_co_readv                = ssh_co_readv,
     .bdrv_co_writev               = ssh_co_writev,
     .bdrv_getlength               = ssh_getlength,
+    .bdrv_co_flush_to_disk        = ssh_co_flush,
     .create_options               = ssh_create_options,
 };
 
diff --git a/configure b/configure
index e6c5d2c..a97bf31 100755
--- a/configure
+++ b/configure
@@ -2357,6 +2357,31 @@ EOF
 fi
 
 ##########################################
+# libssh2_sftp_fsync probe
+
+if test "$libssh2" = "yes"; then
+  cat > $TMPC <<EOF
+#include <stdio.h>
+#include <libssh2.h>
+#include <libssh2_sftp.h>
+int main(void) {
+    LIBSSH2_SESSION *session;
+    LIBSSH2_SFTP *sftp;
+    LIBSSH2_SFTP_HANDLE *sftp_handle;
+    session = libssh2_session_init ();
+    sftp = libssh2_sftp_init (session);
+    sftp_handle = libssh2_sftp_open (sftp, "/", 0, 0);
+    libssh2_sftp_fsync (sftp_handle);
+    return 0;
+}
+EOF
+  # libssh2_cflags/libssh2_libs defined in previous test.
+  if compile_prog "$libssh2_cflags" "$libssh2_libs" ; then
+    QEMU_CFLAGS="-DHAS_LIBSSH2_SFTP_FSYNC $QEMU_CFLAGS"
+  fi
+fi
+
+##########################################
 # linux-aio probe
 
 if test "$linux_aio" != "no" ; then
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 5b36004..dfea4d3 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1081,11 +1081,16 @@ tools only use MD5 to print fingerprints).
 Currently authentication must be done using ssh-agent.  Other
 authentication methods may be supported in future.
 
-Note: The ssh driver does not obey disk flush requests (ie. to commit
-data to the backing disk when the guest requests it).  This is because
-the underlying protocol (SFTP) does not support this.  Thus there is a
-risk of guest disk corruption if the remote server or network goes
-down during writes.
+Note: Many ssh servers do not support an @code{fsync}-style operation.
+The ssh driver cannot guarantee that disk flush requests are
+obeyed, and this causes a risk of disk corruption if the remote
+server or network goes down during writes.  The driver will
+print a warning when @code{fsync} is not supported:
+
+warning: ssh server @code{ssh.example.com:22} does not support fsync
+
+With sufficiently new versions of libssh2 and OpenSSH, @code{fsync} is
+supported.
 
 @node pcsys_network
 @section Network emulation
-- 
1.8.1.4

  parent reply	other threads:[~2013-04-15  8:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-15  8:22 [Qemu-devel] [PULL 00/11] Block patches Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 01/11] block: Introduce bdrv_writev_vmstate Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 02/11] savevm: Implement block_writev_buffer() Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 03/11] block: Introduce bdrv_pwritev() for qcow2_save_vmstate Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 04/11] qemu-iotests: A few more bdrv_pread/pwrite tests Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 05/11] qemu-iotests: Add test for -drive options Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 06/11] qemu-iotests: filter QEMU_PROG in 051.out Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 07/11] ide: refuse WIN_READ_NATIVE_MAX on empty device Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 08/11] block: Add support for Secure Shell (ssh) block device Stefan Hajnoczi
2013-04-15  8:22 ` Stefan Hajnoczi [this message]
2013-04-15  8:22 ` [Qemu-devel] [PATCH 10/11] iotests: Add 'check -ssh' option to test Secure Shell " Stefan Hajnoczi
2013-04-15  8:22 ` [Qemu-devel] [PATCH 11/11] rbd: add an asynchronous flush Stefan Hajnoczi
  -- strict thread matches above, loose matches on Subject: below --
2013-04-26 11:44 [Qemu-devel] [PULL 00/11] Block patches Stefan Hajnoczi
2013-04-26 11:44 ` [Qemu-devel] [PATCH 09/11] block: ssh: Use libssh2_sftp_fsync (if supported by libssh2) to flush to disk Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1366014164-17557-10-git-send-email-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).