qemu-devel
[Top][All Lists]
Advanced

[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/



reply via email to

[Prev in Thread] Current Thread [Next in Thread]