[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: |
Richard W.M. Jones |
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:04:19 +0100 |
User-agent: |
Mutt/1.5.20 (2009-12-10) |
On Tue, Apr 09, 2013 at 02:47:14PM +0200, Stefan Hajnoczi wrote:
> 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.
Thanks -- will fix both in the next version.
Also I have (in my copy) implemented host_key_check parameter.
Fun fact: OpenSSH uses MD5 to print SSH host key fingerprints.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/