qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>,
	pbonzini@redhat.com, jasowang@redhat.com, qemu-devel@nongnu.org,
	armbru@redhat.com, kraxel@redhat.com
Subject: Re: [Qemu-devel] [PATCH v6 3/4] net/net: Convert parse_host_port() to Error
Date: Wed, 28 Jun 2017 15:28:51 +0100	[thread overview]
Message-ID: <20170628142851.GO29134@redhat.com> (raw)
In-Reply-To: <c25da728-d3cf-2b24-19e8-0a67e69fd65c@redhat.com>

On Wed, Jun 28, 2017 at 09:24:58AM -0500, Eric Blake wrote:
> On 06/28/2017 08:23 AM, Daniel P. Berrange wrote:
> > On Wed, Jun 28, 2017 at 09:08:49PM +0800, Mao Zhongyi wrote:
> >> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> >> index 5c326db..78e2b30 100644
> >> --- a/include/qemu/sockets.h
> >> +++ b/include/qemu/sockets.h
> > 
> >>          if (qemu_isdigit(buf[0])) {
> >> -            if (!inet_aton(buf, &saddr->sin_addr))
> >> +            if (!inet_aton(buf, &saddr->sin_addr)) {
> >> +                error_setg(errp, "host address '%s' is not a valid "
> >> +                           "IPv4 address", buf);
> >>                  return -1;
> >> +            }
> >>          } else {
> >> -            if ((he = gethostbyname(buf)) == NULL)
> >> +            he = gethostbyname(buf);
> >> +            if (he == NULL) {
> >> +                error_setg(errp, "can't resolve host address '%s': "
> >> +                           "unknown host", buf);
> >>                  return - 1;
> >> +            }
> > 
> > gethostbyname sets  'h_errno' on failure, so you should pass that
> > into error_setg_errno, instead of hardcoding 'unknown host' as a
> > message
> 
> 'man gethostbyname' says it is deprecated, and that applications should
> use getaddrinfo/getnameinfo instead.  What's our story here?

The real story is to get net/socket.c converted to QIOChannelSocket
and kill this parse_host_port() method in sockets.c It is already
broken by design since it takes a 'struct sockdddr_in' and thus
can't do IPv6.

This patch doesn't make the existing situation worse, so I think
its fine to add this error reporting cleanup now, and not force
immediate conversion to QIOChannelSocket today.  The net/sockets.c
code needs a further refactor before that conversion can be done
in the right way - we've already reverted the wrong way twice ;-)

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  reply	other threads:[~2017-06-28 14:29 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-28 13:08 [Qemu-devel] [PATCH v6 0/4] Improve error reporting Mao Zhongyi
2017-06-28 13:08 ` [Qemu-devel] [PATCH v6 1/4] net/socket: Don't treat odd socket type as SOCK_STREAM Mao Zhongyi
2017-06-28 13:18   ` Daniel P. Berrange
2017-06-28 14:23     ` Eric Blake
2017-06-29  3:24       ` Mao Zhongyi
2017-06-28 13:08 ` [Qemu-devel] [PATCH v6 2/4] net/socket: Convert several helper functions to Error Mao Zhongyi
2017-06-28 13:21   ` Daniel P. Berrange
2017-06-28 13:08 ` [Qemu-devel] [PATCH v6 3/4] net/net: Convert parse_host_port() " Mao Zhongyi
2017-06-28 13:23   ` Daniel P. Berrange
2017-06-28 14:24     ` Eric Blake
2017-06-28 14:28       ` Daniel P. Berrange [this message]
2017-06-29  7:29         ` Markus Armbruster
2017-06-29  3:01     ` Mao Zhongyi
2017-06-28 13:08 ` [Qemu-devel] [PATCH v6 4/4] net/socket: Improve -net socket error reporting Mao Zhongyi
2017-06-28 13:27   ` Daniel P. Berrange
2017-06-29  3:08     ` Mao Zhongyi
2017-06-29  7:31       ` Markus Armbruster

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=20170628142851.GO29134@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=eblake@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=maozy.fnst@cn.fujitsu.com \
    --cc=pbonzini@redhat.com \
    --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).