qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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

  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).