[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 02/15] block/ssh: Do not report rea
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 02/15] block/ssh: Do not report read/write/flush errors to the user |
Date: |
Mon, 8 Apr 2019 14:13:08 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 |
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" <address@hidden>
> Cc: Kevin Wolf <address@hidden>
> Cc: Max Reitz <address@hidden>
> Cc: address@hidden
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> 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.
> - /* 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.
Indentation fixup can be done by maintainer,
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature