On Tue, May 28, 2013 at 06:18:13PM +0200, Ján Tomko wrote:
> Instead of resolving the host as IPv4 and only using the first result,
> resolve it as IPv6 as well and use the first address we've succesfully
> connected to as the portal name.
> ---
> src/libvirt_private.syms | 1 +
> src/storage/storage_backend_iscsi.c | 64 +++++----------------------
> src/util/virutil.c | 87 +++++++++++++++++++++++++++++++++++++
> src/util/virutil.h | 4 ++
> 4 files changed, 102 insertions(+), 54 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9d5f74b..26c4553 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1931,6 +1931,7 @@ virEnumFromString;
> virEnumToString;
> virFindFCHostCapableVport;
> virFormatIntDecimal;
> +virGetConnectableIPAsString;
> virGetDeviceID;
> virGetDeviceUnprivSGIO;
> virGetFCHostNameByWWN;
> diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c
> index ad38ab2..f98fbce 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -1,7 +1,7 @@
> /*
> * storage_backend_iscsi.c: storage backend for iSCSI handling
> *
> - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc.
> + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc.
> * Copyright (C) 2007-2008 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -44,58 +44,16 @@
> #include "vircommand.h"
> #include "virrandom.h"
> #include "virstring.h"
> +#include "virutil.h"
>
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> -static int
> -virStorageBackendISCSITargetIP(const char *hostname,
> - char *ipaddr,
> - size_t ipaddrlen)
> -{
> - struct addrinfo hints;
> - struct addrinfo *result = NULL;
> - int ret;
> -
> - memset(&hints, 0, sizeof(hints));
> - hints.ai_flags = AI_ADDRCONFIG;
> - hints.ai_family = AF_INET;
> - hints.ai_socktype = SOCK_STREAM;
> - hints.ai_protocol = 0;
> -
> - ret = getaddrinfo(hostname, NULL, &hints, &result);
> - if (ret != 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("host lookup failed %s"),
> - gai_strerror(ret));
> - return -1;
> - }
> -
> - if (result == NULL) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("no IP address for target %s"),
> - hostname);
> - return -1;
> - }
> -
> - if (getnameinfo(result->ai_addr, result->ai_addrlen,
> - ipaddr, ipaddrlen, NULL, 0,
> - NI_NUMERICHOST) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("cannot format ip addr for %s"),
> - hostname);
> - freeaddrinfo(result);
> - return -1;
> - }
> -
> - freeaddrinfo(result);
> - return 0;
> -}
> -
> static char *
> virStorageBackendISCSIPortal(virStoragePoolSourcePtr source)
> {
> - char ipaddr[NI_MAXHOST];
> - char *portal;
> + char *ip;
> + char *portal = NULL;
> + int port;
>
> if (source->nhost != 1) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -103,17 +61,15 @@ virStorageBackendISCSIPortal(virStoragePoolSourcePtr source)
> return NULL;
> }
>
> - if (virStorageBackendISCSITargetIP(source->hosts[0].name,
> - ipaddr, sizeof(ipaddr)) < 0)
> + port = source->hosts[0].port ? source->hosts[0].port : 3260;
> +
> + if (!(ip = virGetConnectableIPAsString(source->hosts[0].name, port)))
> return NULL;
>
> - if (virAsprintf(&portal, "%s:%d,1", ipaddr,
> - source->hosts[0].port ?
> - source->hosts[0].port : 3260) < 0) {
> + if (virAsprintf(&portal, "%s,1", ip) < 0)
> virReportOOMError();
> - return NULL;
> - }
>
> + VIR_FREE(ip);
> return portal;
> }
>
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 028f1d1..d8400db 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -650,6 +650,93 @@ cleanup:
> return result;
> }
>
> +/*
> + * Resolve 'host' and return the first address we've connected to
> + * as a string:
> + * address:port
> + * for IPv4 addresses and
> + * [address]:port
> + * for IPv6 addresses.
> + */
> +char *
> +virGetConnectableIPAsString(const char *host,
> + int port)
> +{
> + struct addrinfo hints;
> + struct addrinfo *res, *rp;
> + int saved_errno = EINVAL;
> + char *ret = NULL;
> + char *buf = NULL;
> + char *portstr = NULL;
> + int rc;
> +
> + if (VIR_ALLOC_N(buf, NI_MAXHOST) < 0)
> + goto no_memory;
> +
> + if (virAsprintf(&portstr, "%d", port) < 0)
> + goto no_memory;
> +
> + memset(&hints, 0, sizeof(hints));
> + hints.ai_socktype = SOCK_STREAM;
> + hints.ai_flags = AI_ADDRCONFIG;
> +
> + if ((rc = getaddrinfo(host, portstr, &hints, &res))) {
> + virReportError(VIR_ERR_UNKNOWN_HOST,
> + _("unable to resolve '%s': %s"),
> + host, gai_strerror(rc));
> + goto cleanup;
> + }
> +
> + for (rp = res; rp; rp = rp->ai_next) {
> + int sockfd;
> +
> + sockfd = socket(rp->ai_family, rp->ai_socktype, rp->ai_protocol);
> + if (sockfd == -1) {
> + saved_errno = errno;
> + continue;
> + }
> + if (connect(sockfd, rp->ai_addr, rp->ai_addrlen) != -1) {
> + VIR_FORCE_CLOSE(sockfd);
> + break;
> + }
> + saved_errno = errno;
> + VIR_FORCE_CLOSE(sockfd);
> + }
> +
> + if (!rp) {
> + virReportSystemError(saved_errno,
> + _("unable to connect to '%s:%s'"),
> + host, portstr);
> + goto cleanup;
> + }
> +
> + if ((rc = getnameinfo(rp->ai_addr, rp->ai_addrlen, buf, NI_MAXHOST,
> + NULL, 0, NI_NUMERICHOST)) < 0) {
> + virReportError(VIR_ERR_SYSTEM_ERROR,
> + _("Cannot convert IP address to string: %s"),
> + gai_strerror(rc));
> + goto cleanup;
> + }
> +
> + if (rp->ai_family == AF_INET6) {
> + if (virAsprintf(&ret, "[%s]:%d", buf, port) < 0)
> + goto no_memory;
> + } else if (virAsprintf(&ret, "%s:%d", buf, port) < 0) {
> + goto no_memory;
> + }
It seems to me that this is just duplicating code that iscsiadm ought
to already be able todo itself, if we just passed the hostname straight
on down. IOW, we should just stop turning the hostname into an IP
address & pass to iscsiadm unchanged.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list