Re: [Qemu-devel] [PATCH v3 1/7] block/ssh: don't call libssh2_init() in block_init()

[Eric Blake]

On 11/07/2017 04:27 PM, Jeff Cody wrote:
> We don't need libssh2 failure to be fatal (we could just opt to not
> register the driver on failure). But, it is probably a good idea to
> avoid external library calls during the block_init(), and call the
> libssh2 global init function on the first usage, returning any errors.
> 
> Signed-off-by: Jeff Cody <address@hidden>
> ---
>  block/ssh.c | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
> 
> +static int ssh_state_init(BDRVSSHState *s, Error **errp)
>  {
> +    int ret;
> +
> +    if (!ssh_libinit_called) {
> +        ret = libssh2_init(0);
> +        if (ret) {
> +            error_setg(errp, "libssh2 initialization failed with %d", ret);

Maybe s/with %d/with status %d/

> +            return ret;

This is returning a non-zero value, but not necessarily a negative errno...

> @@ -821,8 +839,13 @@ static int ssh_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>      BDRVSSHState s;
>      ssize_t r2;
>      char c[1] = { '\0' };
> +    Error *local_err = NULL;
>  
> -    ssh_state_init(&s);
> +    ret = ssh_state_init(&s, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return ret;

...but this function wants to return a negative errno.  I think you can
rewrite this to:

if (ssh_state_init(&s, errp)) {
    return -EIO;
}

and skip out on local_err.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature