[PATCH ssh v2] ssh: Do not switch session to non-blocking mode

Richard W.M. Jones posted 1 patch 1 year, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241113125526.2495731-1-rjones@redhat.com
Maintainers: "Richard W.M. Jones" <rjones@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
block/ssh.c | 3 ---
1 file changed, 3 deletions(-)
[PATCH ssh v2] ssh: Do not switch session to non-blocking mode
Posted by Richard W.M. Jones 1 year, 2 months ago
From: Jakub Jelen <jjelen@redhat.com>

The libssh does not handle non-blocking mode in SFTP correctly. The
driver code already changes the mode to blocking for the SFTP
initialization, but for some reason changes to non-blocking mode.
This used to work accidentally until libssh in 0.11 branch merged
the patch to avoid infinite looping in case of network errors:

https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498

Since then, the ssh driver in qemu fails to read files over SFTP
as the first SFTP messages exchanged after switching the session
to non-blocking mode return SSH_AGAIN, but that message is lost
int the SFTP internals and interpretted as SSH_ERROR, which is
returned to the caller:

https://gitlab.com/libssh/libssh-mirror/-/issues/280

This is indeed an issue in libssh that we should address in the
long term, but it will require more work on the internals. For
now, the SFTP is not supported in non-blocking mode.

Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
---
 block/ssh.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/block/ssh.c b/block/ssh.c
index 9f8140bcb6..b9f33ec739 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -866,9 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
         goto err;
     }
 
-    /* Go non-blocking. */
-    ssh_set_blocking(s->session, 0);
-
     if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) {
         bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
     }
-- 
2.46.0
Re: [PATCH ssh v2] ssh: Do not switch session to non-blocking mode
Posted by Michael Tokarev 1 year, 2 months ago
13.11.2024 15:55, Richard W.M. Jones wrote:
> From: Jakub Jelen <jjelen@redhat.com>
> 
> The libssh does not handle non-blocking mode in SFTP correctly. The
> driver code already changes the mode to blocking for the SFTP
> initialization, but for some reason changes to non-blocking mode.
> This used to work accidentally until libssh in 0.11 branch merged
> the patch to avoid infinite looping in case of network errors:
> 
> https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498
> 
> Since then, the ssh driver in qemu fails to read files over SFTP
> as the first SFTP messages exchanged after switching the session
> to non-blocking mode return SSH_AGAIN, but that message is lost
> int the SFTP internals and interpretted as SSH_ERROR, which is
> returned to the caller:
> 
> https://gitlab.com/libssh/libssh-mirror/-/issues/280
> 
> This is indeed an issue in libssh that we should address in the
> long term, but it will require more work on the internals. For
> now, the SFTP is not supported in non-blocking mode.
> 
> Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280
> Signed-off-by: Jakub Jelen <jjelen@redhat.com>
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>
> ---
>   block/ssh.c | 3 ---
>   1 file changed, 3 deletions(-)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index 9f8140bcb6..b9f33ec739 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -866,9 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags,
>           goto err;
>       }
>   
> -    /* Go non-blocking. */
> -    ssh_set_blocking(s->session, 0);
> -
>       if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) {
>           bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE;
>       }

I'm picking this up for the next stable series, since new libssh
is being released and used.  Hopefully the resulting code wont
break with old libssh.

Thanks,

/mjt
Re: [PATCH ssh v2] ssh: Do not switch session to non-blocking mode
Posted by Kevin Wolf 1 year, 2 months ago
Am 13.11.2024 um 13:55 hat Richard W.M. Jones geschrieben:
> From: Jakub Jelen <jjelen@redhat.com>
> 
> The libssh does not handle non-blocking mode in SFTP correctly. The
> driver code already changes the mode to blocking for the SFTP
> initialization, but for some reason changes to non-blocking mode.
> This used to work accidentally until libssh in 0.11 branch merged
> the patch to avoid infinite looping in case of network errors:
> 
> https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498
> 
> Since then, the ssh driver in qemu fails to read files over SFTP
> as the first SFTP messages exchanged after switching the session
> to non-blocking mode return SSH_AGAIN, but that message is lost
> int the SFTP internals and interpretted as SSH_ERROR, which is
> returned to the caller:
> 
> https://gitlab.com/libssh/libssh-mirror/-/issues/280
> 
> This is indeed an issue in libssh that we should address in the
> long term, but it will require more work on the internals. For
> now, the SFTP is not supported in non-blocking mode.
> 
> Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280
> Signed-off-by: Jakub Jelen <jjelen@redhat.com>
> Signed-off-by: Richard W.M. Jones <rjones@redhat.com>

Thanks, applied to the block branch.

Kevin