Hello, comments inline below. On Thu, Nov 14, 2024 at 4:21 PM Kevin Wolf <[email protected]> wrote: > [...] > > > I'll just note that I'm only forwarding on the patch from Jakub. > > I didn't write it. > > > > I did lightly test it, and it seems to work. However it seems also > > likely that it is causing qemu to block internally. Probably not > > noticable for light use, but not something that we'd want for serious > > use. However if libssh doesn't support non-blocking SFTP there's not > > much we can do about that in qemu. > > ...just making it blocking is not acceptable. It will potentially make > the guest hang while we're waiting for sftp responses.
This is the limitation of the SFTP API we have (and a reason why we have a new API below, but only in new 0.11.0 release so not solution for older systems that wont get new libssh). > I see that there is an sftp_aio_*() API, but it looks weird. Instead of > allowing you to just poll the next request that is ready, you have to > call a (blocking) wait on a specific request. Its more "streaming" API allowing the request and response overlap in time allowing better throughput on networks with large latency. To support fully non blocking API in SFTP, there is still way to go, but this api should be more ready for that than the old one. > co_yield(), which is currently used when sftp_read() returns SSH_AGAIN, > makes sure that we poll the socket fd, so we can know that _something_ > new has arrived. However it's unclear to me how to know _which_ request > received a reply and can be completed now. It seems you have to call > sftp_aio_wait_*() in non-blocking mode on all requests to do that, which > probably is affected by the libssh bug, too. Are you sure that with the old libssh versions you were getting SSH_AGAIN in non-blocking mode? Michael in the following comment found the change where the issue started to demonstrate: https://gitlab.com/libssh/libssh-mirror/-/issues/280#note_2204139954 https://gitlab.com/libssh/libssh-mirror/-/commit/2d3b7e07af3675b9a0326bc5c6253a0bbbda567b And from what I read, it was just silently behaving as blocking (potentially infinitely) instead of returning SSH_AGAIN deep in libssh code. > So I'm not sure if sftp_aio_*() can be combined with something else into > a working solution, and I also don't know if it's affected by the same > libssh bug. Right now, we do not have a full solution. But having SFTP working completely in nonoblocking mode is one of the things we would like to have in the long term. > Jakub, can you help with that? > > [...] > > As far as I can see, libssh sessions aren't thread safe, so we'll have > to make sure to have only one request going at the same time, but I > assume that calling ssh_read/write() from different threads sequentially > isn't a problem? My understanding is that the thread safety of libssh is limited to not sharing session between threads -- there is no synchronization if two threads would send packets at the same time: https://api.libssh.org/master/ If you will make sure you will not call sftp_read()/sftp_write() at the same time from different threads, it might work, but it is untested. (joined the ML so I hope this mail will make it there) Jakub
