| Message ID | 20130322130455.GI1504@rhmail.home.annexia.org |
|---|---|
| State | New |
| Headers | show |
On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote: > > I got it working with Curl, patch attached. > > However there are multiple issues (these are mainly notes for myself): > > (1) libcurl cannot read the size of the file. I had to hard-code > this. This is probably just a shortcoming of libcurl (libssh2/sftp > itself can read the size of files). Will try to work on a patch for > upstream. > > (2) Fedora's curl (which is heavily patched) is broken in some way and > deadlocks itself. Upstream curl from git works better. I haven't yet > identified which patch/commit is responsible. > > (3) ssh-agent authentication doesn't work. It appears that either > ssh-agent itself doesn't like multiple connections from a single > process (qemu), or libcurl/libssh2 is having a problem with making > multiple connections out to ssh-agent. If I disable ssh-agent auth, > it works. Still investigating this. > > (4) You must specify a user@ in the URL, else libcurl tries to > authenticate as user "". I will see if I can send a fix for this > upstream. > > (5) Although it gets much of the way through a boot of a guest, it > eventually segfaults. Still investigating this. > > (6) There are several more issues marked by XXX's in the code. Thank you for improving libcurl! You're making it better for everybody. A lot of people go back to NIH when they hit limitations in existing software. Stefan
On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote: > I got it working with Curl, patch attached. > > However there are multiple issues (these are mainly notes for myself): > > (1) libcurl cannot read the size of the file. I had to hard-code > this. This is probably just a shortcoming of libcurl (libssh2/sftp > itself can read the size of files). Will try to work on a patch for > upstream. After my holiday and in the cold of day I've had a long look at the SFTP implementation in libcurl. https://github.com/bagder/curl/blob/master/lib/ssh.c It's implemented as a huge state machine and simply implementing (1) above is problematic (I believe we would have to reopen the connection after our call to curl_easy_perform). The larger issue is that the qemu curl block driver doesn't support writes. Now these could in theory be added. Indeed curl does support "random access" writes, although AFAICT you have to open a new connection each time you want to seek backwards, and you can't read and write over the same connection (so you'd need >= 1 connections for reading and another >= 1 connections for writing). I think I will continue with the pure libssh2-based block driver, making it support all the missing features discussed earlier: http://www.mail-archive.com/qemu-devel@nongnu.org/msg161997.html plus of course non-blocking AIO. I think this way we'll end up with a much more robust, reliable and easier to debug ssh implementation in qemu. Rich.
On Mon, Mar 25, 2013 at 1:32 PM, Richard W.M. Jones <rjones@redhat.com> wrote: > On Fri, Mar 22, 2013 at 01:04:55PM +0000, Richard W.M. Jones wrote: >> I got it working with Curl, patch attached. >> >> However there are multiple issues (these are mainly notes for myself): >> >> (1) libcurl cannot read the size of the file. I had to hard-code >> this. This is probably just a shortcoming of libcurl (libssh2/sftp >> itself can read the size of files). Will try to work on a patch for >> upstream. > > After my holiday and in the cold of day I've had a long look at the > SFTP implementation in libcurl. > > https://github.com/bagder/curl/blob/master/lib/ssh.c > > It's implemented as a huge state machine and simply implementing (1) > above is problematic (I believe we would have to reopen the connection > after our call to curl_easy_perform). > > The larger issue is that the qemu curl block driver doesn't support > writes. Now these could in theory be added. Indeed curl does support > "random access" writes, although AFAICT you have to open a new > connection each time you want to seek backwards, and you can't read > and write over the same connection (so you'd need >= 1 connections for > reading and another >= 1 connections for writing). > > I think I will continue with the pure libssh2-based block driver, > making it support all the missing features discussed earlier: > > http://www.mail-archive.com/qemu-devel@nongnu.org/msg161997.html > > plus of course non-blocking AIO. > > I think this way we'll end up with a much more robust, reliable and > easier to debug ssh implementation in qemu. Fair enough. Stefan
diff --git a/block/curl.c b/block/curl.c index 98947da..b236f7f 100644 --- a/block/curl.c +++ b/block/curl.c @@ -36,7 +36,7 @@ #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \ CURLPROTO_FTP | CURLPROTO_FTPS | \ - CURLPROTO_TFTP) + CURLPROTO_TFTP | CURLPROTO_SFTP) #define CURL_NUM_STATES 8 #define CURL_NUM_ACB 8 @@ -305,6 +305,17 @@ static CURLState *curl_init_state(BDRVCURLState *s) curl_easy_setopt(state->curl, CURLOPT_NOSIGNAL, 1); curl_easy_setopt(state->curl, CURLOPT_ERRORBUFFER, state->errmsg); curl_easy_setopt(state->curl, CURLOPT_FAILONERROR, 1); +#if defined(CURLSSH_AUTH_ANY) + curl_easy_setopt(state->curl, CURLOPT_SSH_AUTH_TYPES, CURLSSH_AUTH_ANY); +#endif + + /* XXX */ + curl_easy_setopt (state->curl, CURLOPT_SSH_PRIVATE_KEYFILE, + "/home/rjones/.ssh/id_rsa"); /* XXX */ + curl_easy_setopt (state->curl, CURLOPT_SSH_PUBLIC_KEYFILE, + "/home/rjones/.ssh/id_rsa.pub"); /* XXX */ + curl_easy_setopt (state->curl, CURLOPT_KEYPASSWD, + "XXX PUT YOUR PASSPHRASE HERE XXX"); /* XXX */ /* Restrict supported protocols to avoid security issues in the more * obscure protocols. For example, do not allow POP3/SMTP/IMAP see @@ -406,8 +417,12 @@ static int curl_open(BlockDriverState *bs, const char *filename, int flags) curl_easy_setopt(state->curl, CURLOPT_NOBODY, 0); if (d) s->len = (size_t)d; +#if 0 else if(!s->len) goto out; +#else + s->len = 8589934592; /* XXX */ +#endif DPRINTF("CURL: Size = %zd\n", s->len); curl_clean_state(state); @@ -626,6 +641,18 @@ static BlockDriver bdrv_tftp = { .bdrv_aio_readv = curl_aio_readv, }; +static BlockDriver bdrv_sftp = { + .format_name = "sftp", + .protocol_name = "sftp", + + .instance_size = sizeof(BDRVCURLState), + .bdrv_file_open = curl_open, + .bdrv_close = curl_close, + .bdrv_getlength = curl_getlength, + + .bdrv_aio_readv = curl_aio_readv, +}; + static void curl_block_init(void) { bdrv_register(&bdrv_http); @@ -633,6 +660,7 @@ static void curl_block_init(void) bdrv_register(&bdrv_ftp); bdrv_register(&bdrv_ftps); bdrv_register(&bdrv_tftp); + bdrv_register(&bdrv_sftp); } block_init(curl_block_init);