[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 2/3] block: ssh: Use libssh2_sftp_fsync (if s
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v8 2/3] block: ssh: Use libssh2_sftp_fsync (if supported by libssh2) to flush to disk. |
Date: |
Tue, 9 Apr 2013 14:47:14 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Tue, Apr 09, 2013 at 10:56:32AM +0100, Richard W.M. Jones wrote:
These are both not worth respinning for, but just in case you send
another version.
> +static coroutine_fn int ssh_flush(BDRVSSHState *s)
> +{
> + int r;
> + static int warned = 0;
> +
> + 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) {
> + if (!warned) {
> + error_report("warning: remote ssh server does not support
> fsync");
Perhaps not worth fixing but this error message has two properties which
could be improved:
1. When multiple -drive file=ssh:// are used it is unclear which SSH
server lacks fsync support since the error message does not say.
2. When multiple -drive file=ssh:// are used only the first server
lacking support will print a warning since warned is static.
> diff --git a/qemu-doc.texi b/qemu-doc.texi
> index 7a0f373..9e30667 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -1072,12 +1072,6 @@ the standard ssh port (22) is used.
> 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.
We can expect many SSH servers not to support the fsync extension. It
would be best to include a note explaining that fsync is not available
on older ssh servers and therefore data integrity cannot be guaranteed
in those cases.
Stefan