[libvirt] [PATCH v1] Loop through all resolved addresses in virNetSocketNewListenTCP

[John Ferlan]

$SUBJ:

util: Fix addres
On 05/15/2018 04:20 AM, Olaf Hering wrote:
> Currently virNetSocketNewListenTCP bails out early under the following
> conditions:
> - the hostname resolves to at least one IPv4 and at least one IPv6
>   address

which produces what error?

> - the local interfaces have that one IPv4 address assigned, but not any
>   of the IPv6 addresses

same question

> - the local interfaces have just IPv6 link-local addresses

same question

> 
> In this case the resolver returns not only the IPv4 addreses but also

s/addreses/addresses

> IPv6. Binding the IPv6 address will obviously fail. But this terminates
> the entire loop, even if binding to IPv4 succeeded.
> 
> To fix this error, just keep going and loop through all returned
> addresses. In case none of the attempts to bind to some address
> succeeded, try to report the appropriate error.
> 

A bit tough to read for this English as my first language, but I think
it can be word-smithed a bit...  once I understand more about what's
being done.

> Signed-off-by: Olaf Hering <olaf at aepfle.de>
> ---
>  src/rpc/virnetsocket.c | 23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 

git am on the patch produces:

Applying: Loop through all resolved addresses in virNetSocketNewListenTCP
.git/rebase-apply/patch:40: trailing whitespace.
      else
warning: 1 line adds whitespace errors.

which means syntax-check fails:


4.51 prohibit_empty_first_line
src/rpc/virnetsocket.c:414:      else
maint.mk: found trailing blank(s)
4.51 prohibit_fork_wrappers
make: *** [maint.mk:750: sc_trailing_blank] Error 1
make: *** Waiting for unfinished jobs....

but it also fails for:

  GEN      public-submodule-commit
  GEN      spacing-check
No whitespace after keyword:
src/rpc/virnetsocket.c:412:       else if(addrInUse)
maint.mk: incorrect formatting
make: *** [cfg.mk:1117: spacing-check] Error 1

Before posting please be sure that:

make check syntax-check

passes.

> diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> index 7087abec9c..0ee5e3604f 100644
> --- a/src/rpc/virnetsocket.c
> +++ b/src/rpc/virnetsocket.c
> @@ -382,11 +382,8 @@ int virNetSocketNewListenTCP(const char *nodename,
>  #endif
>  
>          if (bind(fd, runp->ai_addr, runp->ai_addrlen) < 0) {
> -            if (errno != EADDRINUSE) {
> -                virReportSystemError(errno, "%s", _("Unable to bind to port"));
> -                goto error;
> -            }
> -            addrInUse = true;
> +            if (errno == EADDRINUSE)
> +                addrInUse = true;

So this to me reads as if we're just ignoring all errors, but we do care
if the address was in use already and we'll save that bit of information
for later.

Does that mean there is some "other" sub class of errors for which it's
desired to retry?  Is there a finite list?

It doesn't imply to me what was stated in the commit message, but I'm
not deep in the throes of debugging a problem here either!

>              VIR_FORCE_CLOSE(fd);
>              runp = runp->ai_next;
>              continue;
> @@ -409,14 +406,14 @@ int virNetSocketNewListenTCP(const char *nodename,
>          fd = -1;
>      }
>  
> -    if (nsocks == 0 && familyNotSupported) {
> -        virReportSystemError(EAFNOSUPPORT, "%s", _("Unable to bind to port"));
> -        goto error;
> -    }
> -
> -    if (nsocks == 0 &&
> -        addrInUse) {
> -        virReportSystemError(EADDRINUSE, "%s", _("Unable to bind to port"));
> +    if (nsocks == 0) {
> +      if (familyNotSupported)
> +        errno = EAFNOSUPPORT;
> +      else if(addrInUse)
> +        errno = EADDRINUSE;
> +      else 
> +        errno = EDESTADDRREQ;

bind says this is returned when "The address argument is a null
pointer." - so essentially we're making up a failure?

> +        virReportSystemError(errno, "%s", _("Unable to bind to port"));

This is all not formatted properly

Indents should be 4 spaces and this last virReportError probably
shouldn't be indented.

Please rework and repost.

Tks,

John
>          goto error;
>      }
>  
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>