[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/7] block/ssh: don't call libss
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/7] block/ssh: don't call libssh2_init() in block_init() |
Date: |
Wed, 30 Aug 2017 14:40:16 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 08/30/2017 11:56 AM, 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 | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 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);
> + return ret;
Do we know if this number is always positive or negative?
> @@ -772,8 +788,13 @@ static int ssh_file_open(BlockDriverState *bs, QDict
> *options, int bdrv_flags,
> BDRVSSHState *s = bs->opaque;
> int ret;
> int ssh_flags;
> + 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;
Is returning 'ret' from libssh2_init() wise?
If 'ret' is not important, you could simplify this:
if (ssh_state_init(s, errp)) {
return -1;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature