From: "Richard W.M. Jones" <rjones@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Jeff Cody <jcody@redhat.com>,
qemu-devel@nongnu.org, mitake.hitoshi@lab.ntt.co.jp,
namei.unix@gmail.com, kwolf@redhat.com, qemu-block@nongnu.org,
ptoscano@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 1/7] block/ssh: don't call libssh2_init() in block_init()
Date: Thu, 31 Aug 2017 08:46:20 +0100 [thread overview]
Message-ID: <20170831074620.GR20914@redhat.com> (raw)
In-Reply-To: <94eb9a23-1cb7-a139-039e-4d0f138a6244@redhat.com>
On Wed, Aug 30, 2017 at 02:40:16PM -0500, Eric Blake wrote:
> 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 <jcody@redhat.com>
> > ---
> > 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?
FWIW documentation says:
"Returns 0 if succeeded, or a negative value for error."
I was holding off on reviewing this patch in general since there's a
libssh-based alternative driver under development, which should be
better.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
next prev parent reply other threads:[~2017-08-31 7:46 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-30 16:56 [Qemu-devel] [PATCH v2 0/7] Code cleanup and minor fixes Jeff Cody
2017-08-30 16:56 ` [Qemu-devel] [PATCH v2 1/7] block/ssh: don't call libssh2_init() in block_init() Jeff Cody
2017-08-30 19:40 ` Eric Blake
2017-08-30 20:11 ` Jeff Cody
2017-08-30 20:12 ` Eric Blake
2017-08-31 7:46 ` Richard W.M. Jones [this message]
2017-08-30 16:56 ` [Qemu-devel] [PATCH v2 2/7] block/ssh: make compliant with coding guidelines Jeff Cody
2017-08-30 19:41 ` Eric Blake
2017-08-30 16:57 ` [Qemu-devel] [PATCH v2 3/7] block/sheepdog: remove spurious NULL check Jeff Cody
2017-08-30 17:37 ` Philippe Mathieu-Daudé
2017-08-30 19:42 ` Eric Blake
2017-08-30 16:57 ` [Qemu-devel] [PATCH v2 4/7] block/sheepdog: code beautification Jeff Cody
2017-08-30 19:48 ` Eric Blake
2017-08-30 16:57 ` [Qemu-devel] [PATCH v2 5/7] block/curl: check error return of curl_global_init() Jeff Cody
2017-08-30 19:49 ` Eric Blake
2017-08-30 16:57 ` [Qemu-devel] [PATCH v2 6/7] block/curl: fix minor memory leaks Jeff Cody
2017-08-30 19:51 ` Eric Blake
2017-08-30 16:57 ` [Qemu-devel] [PATCH v2 7/7] block/curl: code cleanup to comply with coding style Jeff Cody
2017-08-30 19:53 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170831074620.GR20914@redhat.com \
--to=rjones@redhat.com \
--cc=eblake@redhat.com \
--cc=jcody@redhat.com \
--cc=kwolf@redhat.com \
--cc=mitake.hitoshi@lab.ntt.co.jp \
--cc=namei.unix@gmail.com \
--cc=ptoscano@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).