Eric Blake <ebl...@redhat.com> writes:
> On 4/8/19 3:36 AM, Markus Armbruster wrote:
>> Callbacks ssh_co_readv(), ssh_co_writev(), ssh_co_flush() report
>> errors to the user with error_printf(). They shouldn't, it's their
>> caller's job. Replace by a suitable trace point.
>>
>> Perhaps we should convert this part of the block driver interface to
>> Error, so block drivers can pass more detail to their callers. Not
>> today.
>>
>> Cc: "Richard W.M. Jones" <rjo...@redhat.com>
>> Cc: Kevin Wolf <kw...@redhat.com>
>> Cc: Max Reitz <mre...@redhat.com>
>> Cc: qemu-bl...@nongnu.org
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> block/ssh.c | 36 ++++++++++++------------------------
>> block/trace-events | 3 +++
>> 2 files changed, 15 insertions(+), 24 deletions(-)
>>
>
>> -static void GCC_FMT_ATTR(2, 3)
>> -sftp_error_report(BDRVSSHState *s, const char *fs, ...)
>> +static void sftp_error_trace(BDRVSSHState *s, const char *op)
>> {
>> - va_list args;
>> + char *ssh_err;
>> + int ssh_err_code;
>> + unsigned long sftp_err_code;
>>
>> - va_start(args, fs);
>> - error_vprintf(fs, args);
>> -
>> - if ((s)->sftp) {
>
> The old code was conditional,
>
>> - char *ssh_err;
>> - int ssh_err_code;
>> - unsigned long sftp_err_code;
>> -
>> - /* This is not an errno. See <libssh2.h>. */
>> - ssh_err_code = libssh2_session_last_error(s->session,
>> + /* This is not an errno. See <libssh2.h>. */
>> + ssh_err_code = libssh2_session_last_error(s->session,
>> &ssh_err, NULL, 0);
>
> Indentation looks off now.
Will fix.
>> - /* See <libssh2_sftp.h>. */
>> - sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>> + /* See <libssh2_sftp.h>. */
>> + sftp_err_code = libssh2_sftp_last_error((s)->sftp);
>
> but it appears the new code can call libssh2_sftp_last_error(NULL). Am I
> missing something, or could that be a problem?
>
> /me rescans the full file...
>
> Okay, connect_to_ssh() won't succeed unless s->sftp is set, and it
> appears that all callers to the trace function can only be reached if
> connect succeeded.
I can add to the commit message
While there, drop the unreachable !s->sftp case.
> Indentation fixup can be done by maintainer,
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
