[libvirt] [PATCH v2 0/9] Additional host name check for network storage pools

v1 here: http://www.redhat.com/archives/libvir-list/2015-April/msg00873.html Essentially a rewrite of v1 changing to use virsocketaddr.* rather than virutil.* Also making check for hostname duplication prior to startPool rather than during XML processing John Ferlan (9): socketaddr: Add isNumeric flag to virSocketAddrParseInternal socketaddr: Introduce virSocketAddrParseName iscsi: Check for validity of pool source hostname netfs: Check for validity of pool source hostname gluster: Check for validity of pool source hostname sheepdog: Check for validity of pool source hostname virsockaddr: Split up functionality of virSocketAddrFormatFull virsocketaddr: Introduce virSocketAddrIsSameTCPHost storage: Check for duplicate resolved host name at pool create src/conf/storage_conf.c | 80 ++++++++++++++- src/conf/storage_conf.h | 6 +- src/libvirt_private.syms | 3 + src/storage/storage_backend_fs.c | 5 +- src/storage/storage_backend_gluster.c | 6 +- src/storage/storage_backend_iscsi.c | 6 +- src/storage/storage_backend_sheepdog.c | 38 ++++--- src/storage/storage_driver.c | 9 +- src/util/virsocketaddr.c | 180 ++++++++++++++++++++++++++++----- src/util/virsocketaddr.h | 9 +- 10 files changed, 295 insertions(+), 47 deletions(-) -- 2.1.0

Adjust virSocketAddrParseInternal to take a boolean 'isNumeric' in order to determine whether to set "ai_flags = AI_NUMERICHOST;" - IOW - expect a numeric IP Address of sorts in the 'val' to be resolved. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virsocketaddr.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 67ed330..0e9a39c 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2014 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -80,6 +80,7 @@ static int virSocketAddrParseInternal(struct addrinfo **res, const char *val, int family, + bool isNumeric, bool reportError) { struct addrinfo hints; @@ -92,7 +93,8 @@ virSocketAddrParseInternal(struct addrinfo **res, memset(&hints, 0, sizeof(hints)); hints.ai_family = family; - hints.ai_flags = AI_NUMERICHOST; + if (isNumeric) + hints.ai_flags = AI_NUMERICHOST; if ((err = getaddrinfo(val, NULL, &hints, res)) != 0) { if (reportError) virReportError(VIR_ERR_SYSTEM_ERROR, @@ -121,7 +123,7 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) int len; struct addrinfo *res; - if (virSocketAddrParseInternal(&res, val, family, true) < 0) + if (virSocketAddrParseInternal(&res, val, family, true, true) < 0) return -1; if (res == NULL) { @@ -878,7 +880,7 @@ virSocketAddrNumericFamily(const char *address) struct addrinfo *res; unsigned short family; - if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, false) < 0) + if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, true, false) < 0) return -1; family = res->ai_addr->sa_family; -- 2.1.0

Add a new function virSocketAddrParseName which unlike virSocketAddrParse will be capable of processing a network address that needs to be looked up and resolved. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++-- src/util/virsocketaddr.h | 7 ++++++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..848b44a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2065,6 +2065,7 @@ virSocketAddrNumericFamily; virSocketAddrParse; virSocketAddrParseIPv4; virSocketAddrParseIPv6; +virSocketAddrParseName; virSocketAddrPrefixToNetmask; virSocketAddrSetIPv4Addr; virSocketAddrSetPort; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 0e9a39c..83518a0 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -80,6 +80,7 @@ static int virSocketAddrParseInternal(struct addrinfo **res, const char *val, int family, + int protocol, bool isNumeric, bool reportError) { @@ -93,6 +94,7 @@ virSocketAddrParseInternal(struct addrinfo **res, memset(&hints, 0, sizeof(hints)); hints.ai_family = family; + hints.ai_protocol = protocol; if (isNumeric) hints.ai_flags = AI_NUMERICHOST; if ((err = getaddrinfo(val, NULL, &hints, res)) != 0) { @@ -123,7 +125,7 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) int len; struct addrinfo *res; - if (virSocketAddrParseInternal(&res, val, family, true, true) < 0) + if (virSocketAddrParseInternal(&res, val, family, 0, true, true) < 0) return -1; if (res == NULL) { @@ -143,6 +145,51 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family) return len; } +/** + * virSocketAddrParseName: + * @addr: where to store the return value, optional. + * @val: either a numeric network address IPv4 or IPv6 or a network + * hostname to be looked up and resolved. + * @family: address family to pass down to getaddrinfo + * @protocol: specific protocol to use + * + * Mostly a wrapper for getaddrinfo() extracting the address storage + * from the numeric string like 1.2.3.4 or 2001:db8:85a3:0:0:8a2e:370:7334 + * or a resolvable network hostname such as "redhat.com" + * + * Returns the length of the network address or -1 in case of error. + */ +int +virSocketAddrParseName(virSocketAddrPtr addr, + const char *val, + int family, + int protocol) +{ + int len; + struct addrinfo *res; + + if (virSocketAddrParseInternal(&res, val, family, + protocol, false, true) < 0) + return -1; + + if (res == NULL) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("No addresses found for '%s' using family='%d' " + "and protocol='%d'"), + val, family, protocol); + return -1; + } + + len = res->ai_addrlen; + if (addr != NULL) { + memcpy(&addr->data.stor, res->ai_addr, len); + addr->len = res->ai_addrlen; + } + + freeaddrinfo(res); + return len; +} + /* * virSocketAddrParseIPv4: * @val: an IPv4 numeric address @@ -880,7 +927,8 @@ virSocketAddrNumericFamily(const char *address) struct addrinfo *res; unsigned short family; - if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, true, false) < 0) + if (virSocketAddrParseInternal(&res, address, AF_UNSPEC, + 0, true, false) < 0) return -1; family = res->ai_addr->sa_family; diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 99ab46f..3bbf119 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2009-2013 Red Hat, Inc. + * Copyright (C) 2009-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -78,6 +78,11 @@ int virSocketAddrParse(virSocketAddrPtr addr, const char *val, int family); +int virSocketAddrParseName(virSocketAddrPtr addr, + const char *val, + int family, + int protocol); + int virSocketAddrParseIPv4(virSocketAddrPtr addr, const char *val); -- 2.1.0

Ensure that the pool that's being started has a source pool hostname that can be resolved before trying to start an iSCSI session. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_iscsi.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 197d333..0482dfb 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-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -385,6 +385,10 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, return -1; } + if (virSocketAddrParseName(NULL, pool->def->source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) < 0) + return -1; + if (pool->def->source.ndevice != 1 || pool->def->source.devices[0].path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.1.0

Before attempting to mount the netfs pool, ensure the source pool hostname can be resolved to something this host knows. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_fs.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 521dc70..0263913 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -1,7 +1,7 @@ /* * storage_backend_fs.c: storage backend for FS and directory handling * - * Copyright (C) 2007-2014 Red Hat, Inc. + * Copyright (C) 2007-2015 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -408,6 +408,9 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) "%s", _("missing source path")); return -1; } + if (virSocketAddrParseName(NULL, pool->def->source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) < 0) + return -1; } else { if (pool->def->source.ndevice != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, -- 2.1.0

Prior to attempting to open the gluster connection, let's make sure we can resolve the source pool hostname. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_gluster.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index d2e79bc..a10f784 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -1,7 +1,7 @@ /* * storage_backend_gluster.c: storage backend for Gluster handling * - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -96,6 +96,10 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) trailing_slash = false; } + if (virSocketAddrParseName(NULL, pool->def->source.hosts[0].name, + AF_UNSPEC, IPPROTO_TCP) < 0) + return NULL; + if (VIR_ALLOC(ret) < 0) return NULL; -- 2.1.0

If there is a pool source hostname provided, let's make sure the name can be resolved before trying to connect to it via a virCommand sequence. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_sheepdog.c | 38 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index af15c3b..b92c7a3 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -1,7 +1,7 @@ /* * storage_backend_sheepdog.c: storage backend for Sheepdog handling * - * Copyright (C) 2013-2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * Copyright (C) 2012 Wido den Hollander * Copyright (C) 2012 Frank Spijkerman * Copyright (C) 2012 Sebastian Wiedenroth @@ -40,9 +40,6 @@ static int virStorageBackendSheepdogRefreshVol(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol); -void virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, - virStoragePoolObjPtr pool); - int virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool, char *output) @@ -90,7 +87,7 @@ virStorageBackendSheepdogParseNodeInfo(virStoragePoolDefPtr pool, return -1; } -void +static int virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virStoragePoolObjPtr pool) { @@ -99,6 +96,8 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, if (pool->def->source.nhost > 0) { if (pool->def->source.hosts[0].name != NULL) address = pool->def->source.hosts[0].name; + if (virSocketAddrParseName(NULL, address, AF_UNSPEC, IPPROTO_TCP) < 0) + return -1; if (pool->def->source.hosts[0].port) port = pool->def->source.hosts[0].port; } @@ -106,6 +105,7 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, virCommandAddArgFormat(cmd, "%s", address); virCommandAddArg(cmd, "-p"); virCommandAddArgFormat(cmd, "%d", port); + return 0; } static int @@ -151,7 +151,8 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED, size_t i; virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", "-r", NULL); - virStorageBackendSheepdogAddHostArg(cmd, pool); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -196,7 +197,8 @@ virStorageBackendSheepdogRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, virCommandPtr cmd; cmd = virCommandNewArgList(COLLIE, "node", "info", "-r", NULL); - virStorageBackendSheepdogAddHostArg(cmd, pool); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -218,13 +220,16 @@ virStorageBackendSheepdogDeleteVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { + int ret = -1; virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "delete", vol->name, NULL); - virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; + ret = virCommandRun(cmd, NULL); + cleanup: virCommandFree(cmd); return ret; } @@ -275,7 +280,8 @@ virStorageBackendSheepdogBuildVol(virConnectPtr conn, cmd = virCommandNewArgList(COLLIE, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->target.capacity); - virStorageBackendSheepdogAddHostArg(cmd, pool); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -355,11 +361,12 @@ virStorageBackendSheepdogRefreshVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int ret; + int ret = -1; char *output = NULL; virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "list", vol->name, "-r", NULL); - virStorageBackendSheepdogAddHostArg(cmd, pool); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); @@ -391,14 +398,17 @@ virStorageBackendSheepdogResizeVol(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long long capacity, unsigned int flags) { + int ret = -1; virCheckFlags(0, -1); virCommandPtr cmd = virCommandNewArgList(COLLIE, "vdi", "resize", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", capacity); - virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); + if (virStorageBackendSheepdogAddHostArg(cmd, pool) < 0) + goto cleanup; + ret = virCommandRun(cmd, NULL); + cleanup: virCommandFree(cmd); return ret; -- 2.1.0

Create a local API virSocketAddrGetNumericHost which can be used by a future patch in order to obtain the numeric host nameinfo data Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virsocketaddr.c | 60 ++++++++++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 83518a0..993d460 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -333,6 +333,42 @@ virSocketAddrFormat(const virSocketAddr *addr) } +static char * +virSocketAddrGetNumericHost(const struct sockaddr *sa, + socklen_t salen, + bool withService, + const char *separator) +{ + char host[NI_MAXHOST], port[NI_MAXSERV]; + char *addrstr; + int err; + int flags = NI_NUMERICHOST; + + if (withService) + flags |= NI_NUMERICSERV; + + if ((err = getnameinfo(sa, salen, + host, sizeof(host), + port, sizeof(port), + flags)) != 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Cannot convert socket address to string: %s"), + gai_strerror(err)); + return NULL; + } + + if (withService) { + if (virAsprintf(&addrstr, "%s%s%s", host, separator, port) == -1) + return NULL; + } else { + if (VIR_STRDUP(addrstr, host) < 0) + return NULL; + } + + return addrstr; +} + + /* * virSocketAddrFormatFull: * @addr: an initialized virSocketAddrPtr @@ -348,9 +384,7 @@ virSocketAddrFormatFull(const virSocketAddr *addr, bool withService, const char *separator) { - char host[NI_MAXHOST], port[NI_MAXSERV]; char *addrstr; - int err; if (addr == NULL) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Missing address")); @@ -371,26 +405,8 @@ virSocketAddrFormatFull(const virSocketAddr *addr, return addrstr; } - if ((err = getnameinfo(&addr->data.sa, - addr->len, - host, sizeof(host), - port, sizeof(port), - NI_NUMERICHOST | NI_NUMERICSERV)) != 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Cannot convert socket address to string: %s"), - gai_strerror(err)); - return NULL; - } - - if (withService) { - if (virAsprintf(&addrstr, "%s%s%s", host, separator, port) == -1) - goto error; - } else { - if (VIR_STRDUP(addrstr, host) < 0) - goto error; - } - - return addrstr; + return virSocketAddrGetNumericHost(&addr->data.sa, addr->len, + withService, separator); error: return NULL; -- 2.1.0

Add new API to be able to compare two TCP host name strings or numeric IP Addresses in order to determine if the resolved and translated names match. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ 3 files changed, 65 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 848b44a..34cb871 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2058,6 +2058,7 @@ virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumericLocalhost; virSocketAddrIsPrivate; +virSocketAddrIsSameTCPHost; virSocketAddrIsWildcard; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 993d460..c602604 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -23,6 +23,7 @@ #include <config.h> +#include "viralloc.h" #include "virsocketaddr.h" #include "virerror.h" #include "virstring.h" @@ -980,3 +981,64 @@ virSocketAddrIsNumericLocalhost(const char *addr) return false; } + + +/** + * virSocketAddrIsSameTCPHost: + * @host1: either a numeric network address IPv4 or IPv6 or a network hostname + * @host2: either a numeric network address IPv4 or IPv6 or a network hostname + * + * For each hostname, get the array of resolved addresses and in a loop make + * compare the translated host IP Address looking for duplicates. + * + * Returns 1 if the hosts match, 0 if there is no match, and -1 on error + */ +int +virSocketAddrIsSameTCPHost(const char *host1, + const char *host2) +{ + int ret = -1; + struct addrinfo *reshost1 = NULL, *reshost2 = NULL; + struct addrinfo *curhost1, *curhost2; + char *host1rslv = NULL, *host2rslv = NULL; + + if (virSocketAddrParseInternal(&reshost1, host1, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + if (virSocketAddrParseInternal(&reshost2, host2, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + for (curhost1 = reshost1; curhost1; curhost1 = curhost1->ai_next) { + if (!(host1rslv = virSocketAddrGetNumericHost(curhost1->ai_addr, + curhost1->ai_addrlen, + false, NULL))) + goto cleanup; + for (curhost2 = reshost2; curhost2; curhost2 = curhost2->ai_next) { + if (curhost1->ai_family != curhost2->ai_family) + continue; + + if (!(host2rslv = virSocketAddrGetNumericHost(curhost2->ai_addr, + curhost2->ai_addrlen, + false, NULL))) + goto cleanup; + + if (STREQ(host1rslv, host2rslv)) { + ret = 1; + goto cleanup; + } + } + } + + ret = 0; + + cleanup: + if (reshost1) + freeaddrinfo(reshost1); + if (reshost2) + freeaddrinfo(reshost2); + VIR_FREE(host1rslv); + VIR_FREE(host2rslv); + return ret; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 3bbf119..2f9f358 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -137,4 +137,6 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr); int virSocketAddrNumericFamily(const char *address); bool virSocketAddrIsNumericLocalhost(const char *addr); + +int virSocketAddrIsSameTCPHost(const char *host1, const char *host2); #endif /* __VIR_SOCKETADDR_H__ */ -- 2.1.0

On Thu, Apr 23, 2015 at 16:12:16 -0400, John Ferlan wrote:
Add new API to be able to compare two TCP host name strings or numeric IP Addresses in order to determine if the resolved and translated names match.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ 3 files changed, 65 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 848b44a..34cb871 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2058,6 +2058,7 @@ virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumericLocalhost; virSocketAddrIsPrivate; +virSocketAddrIsSameTCPHost; virSocketAddrIsWildcard; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 993d460..c602604 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -23,6 +23,7 @@
#include <config.h>
+#include "viralloc.h" #include "virsocketaddr.h" #include "virerror.h" #include "virstring.h" @@ -980,3 +981,64 @@ virSocketAddrIsNumericLocalhost(const char *addr)
return false; } + + +/** + * virSocketAddrIsSameTCPHost: + * @host1: either a numeric network address IPv4 or IPv6 or a network hostname + * @host2: either a numeric network address IPv4 or IPv6 or a network hostname + * + * For each hostname, get the array of resolved addresses and in a loop make + * compare the translated host IP Address looking for duplicates. + * + * Returns 1 if the hosts match, 0 if there is no match, and -1 on error + */ +int +virSocketAddrIsSameTCPHost(const char *host1, + const char *host2) +{ + int ret = -1; + struct addrinfo *reshost1 = NULL, *reshost2 = NULL; + struct addrinfo *curhost1, *curhost2; + char *host1rslv = NULL, *host2rslv = NULL; + + if (virSocketAddrParseInternal(&reshost1, host1, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + if (virSocketAddrParseInternal(&reshost2, host2, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + for (curhost1 = reshost1; curhost1; curhost1 = curhost1->ai_next) { + if (!(host1rslv = virSocketAddrGetNumericHost(curhost1->ai_addr, + curhost1->ai_addrlen, + false, NULL))) + goto cleanup; + for (curhost2 = reshost2; curhost2; curhost2 = curhost2->ai_next) { + if (curhost1->ai_family != curhost2->ai_family) + continue; + + if (!(host2rslv = virSocketAddrGetNumericHost(curhost2->ai_addr, + curhost2->ai_addrlen, + false, NULL))) + goto cleanup; + + if (STREQ(host1rslv, host2rslv)) {
I'm still failing to see why the host address check portion of virSocketAddrEqual() isn't good for this job and you have to format the address as string.
+ ret = 1; + goto cleanup; + } + }
Next iteration leaks host2rslv.
+ }
Next iteration leaks host1rslv.
+ + ret = 0; + + cleanup: + if (reshost1) + freeaddrinfo(reshost1); + if (reshost2) + freeaddrinfo(reshost2); + VIR_FREE(host1rslv); + VIR_FREE(host2rslv); + return ret; +}
Peter

On 04/24/2015 04:25 AM, Peter Krempa wrote:
On Thu, Apr 23, 2015 at 16:12:16 -0400, John Ferlan wrote:
Add new API to be able to compare two TCP host name strings or numeric IP Addresses in order to determine if the resolved and translated names match.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ 3 files changed, 65 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 848b44a..34cb871 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2058,6 +2058,7 @@ virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumericLocalhost; virSocketAddrIsPrivate; +virSocketAddrIsSameTCPHost; virSocketAddrIsWildcard; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 993d460..c602604 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -23,6 +23,7 @@
#include <config.h>
+#include "viralloc.h" #include "virsocketaddr.h" #include "virerror.h" #include "virstring.h" @@ -980,3 +981,64 @@ virSocketAddrIsNumericLocalhost(const char *addr)
return false; } + + +/** + * virSocketAddrIsSameTCPHost: + * @host1: either a numeric network address IPv4 or IPv6 or a network hostname + * @host2: either a numeric network address IPv4 or IPv6 or a network hostname + * + * For each hostname, get the array of resolved addresses and in a loop make + * compare the translated host IP Address looking for duplicates. + * + * Returns 1 if the hosts match, 0 if there is no match, and -1 on error + */ +int +virSocketAddrIsSameTCPHost(const char *host1, + const char *host2) +{ + int ret = -1; + struct addrinfo *reshost1 = NULL, *reshost2 = NULL; + struct addrinfo *curhost1, *curhost2; + char *host1rslv = NULL, *host2rslv = NULL; + + if (virSocketAddrParseInternal(&reshost1, host1, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + if (virSocketAddrParseInternal(&reshost2, host2, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + for (curhost1 = reshost1; curhost1; curhost1 = curhost1->ai_next) { + if (!(host1rslv = virSocketAddrGetNumericHost(curhost1->ai_addr, + curhost1->ai_addrlen, + false, NULL))) + goto cleanup; + for (curhost2 = reshost2; curhost2; curhost2 = curhost2->ai_next) { + if (curhost1->ai_family != curhost2->ai_family) + continue; + + if (!(host2rslv = virSocketAddrGetNumericHost(curhost2->ai_addr, + curhost2->ai_addrlen, + false, NULL))) + goto cleanup; + + if (STREQ(host1rslv, host2rslv)) {
I'm still failing to see why the host address check portion of virSocketAddrEqual() isn't good for this job and you have to format the address as string.
Because I was more focused on the implementation that I had in my mind that did the getaddrinfo/getnameinfo loops. Many times the mind is a very difficult thing to convince to do something a different way - about to send an adjustment,
+ ret = 1; + goto cleanup; + } + }
Next iteration leaks host2rslv.
+ }
Next iteration leaks host1rslv.
Oh yeah ... right, but they won't matter with the adjustment. John
+ + ret = 0; + + cleanup: + if (reshost1) + freeaddrinfo(reshost1); + if (reshost2) + freeaddrinfo(reshost2); + VIR_FREE(host1rslv); + VIR_FREE(host2rslv); + return ret; +}
Peter

Add new API to be able to compare two TCP host name strings or numeric IP Addresses in order to determine if the resolved and translated names match. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virsocketaddr.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virsocketaddr.h | 2 ++ 3 files changed, 55 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 848b44a..34cb871 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2058,6 +2058,7 @@ virSocketAddrGetRange; virSocketAddrIsNetmask; virSocketAddrIsNumericLocalhost; virSocketAddrIsPrivate; +virSocketAddrIsSameTCPHost; virSocketAddrIsWildcard; virSocketAddrMask; virSocketAddrMaskByPrefix; diff --git a/src/util/virsocketaddr.c b/src/util/virsocketaddr.c index 993d460..5e1e392 100644 --- a/src/util/virsocketaddr.c +++ b/src/util/virsocketaddr.c @@ -980,3 +980,55 @@ virSocketAddrIsNumericLocalhost(const char *addr) return false; } + + +/** + * virSocketAddrIsSameTCPHost: + * @host1: either a numeric network address IPv4 or IPv6 or a network hostname + * @host2: either a numeric network address IPv4 or IPv6 or a network hostname + * + * For each hostname, get the array of resolved addresses and in a loop make + * compare the translated host IP Address looking for duplicates. + * + * Returns 1 if the hosts match, 0 if there is no match, and -1 on error + */ +int +virSocketAddrIsSameTCPHost(const char *host1, + const char *host2) +{ + int ret = -1; + struct addrinfo *reshost1 = NULL, *reshost2 = NULL; + struct addrinfo *curhost1, *curhost2; + virSocketAddr host1Addr, host2Addr; + + if (virSocketAddrParseInternal(&reshost1, host1, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + if (virSocketAddrParseInternal(&reshost2, host2, AF_UNSPEC, IPPROTO_TCP, + false, true) < 0) + goto cleanup; + + for (curhost1 = reshost1; curhost1; curhost1 = curhost1->ai_next) { + host1Addr.len = curhost1->ai_addrlen; + memcpy(&host1Addr.data.stor, curhost1->ai_addr, host1Addr.len); + for (curhost2 = reshost2; curhost2; curhost2 = curhost2->ai_next) { + host2Addr.len = curhost2->ai_addrlen; + memcpy(&host2Addr.data.stor, curhost2->ai_addr, host2Addr.len); + + if (virSocketAddrEqual(&host1Addr, &host2Addr)) { + ret = 1; + goto cleanup; + } + } + } + + ret = 0; + + cleanup: + if (reshost1) + freeaddrinfo(reshost1); + if (reshost2) + freeaddrinfo(reshost2); + return ret; +} diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h index 3bbf119..2f9f358 100644 --- a/src/util/virsocketaddr.h +++ b/src/util/virsocketaddr.h @@ -137,4 +137,6 @@ bool virSocketAddrIsWildcard(const virSocketAddr *addr); int virSocketAddrNumericFamily(const char *address); bool virSocketAddrIsNumericLocalhost(const char *addr); + +int virSocketAddrIsSameTCPHost(const char *host1, const char *host2); #endif /* __VIR_SOCKETADDR_H__ */ -- 2.1.0

https://bugzilla.redhat.com/show_bug.cgi?id=1171984 During storage pool Create or CreateXML add a call prior to the backend startPool that will compare the resolved source host name of the network storage pool to be started against the active, network storage pools resolved host name to ensure the to be started definition won't duplicate an existing, active pool for the same pool source path's. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 80 +++++++++++++++++++++++++++++++++++++++++++- src/conf/storage_conf.h | 6 +++- src/libvirt_private.syms | 1 + src/storage/storage_driver.c | 9 ++++- 4 files changed, 93 insertions(+), 3 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4852dfb..4793817 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1,7 +1,7 @@ /* * storage_conf.c: config handling for storage driver * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -43,6 +43,7 @@ #include "virbuffer.h" #include "viralloc.h" #include "virfile.h" +#include "virsocketaddr.h" #include "virstring.h" #include "virlog.h" @@ -2571,6 +2572,83 @@ virStoragePoolSourceFindDuplicate(virConnectPtr conn, return ret; } +/* + * Prior to starting a storage pool, this API will check to see whether + * any currently active pool is already using the same source dir/device + * and same resolved host name as the proposed definition. To do this, + * query the host in order to compare the resolved names for the pool + * and the definition. + * + * Returns 0 if the definition doesn't match any host, returns -1 on error + */ +int +virStoragePoolSourceHostCompare(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) +{ + int ret = -1; + size_t i; + + if (def->type != VIR_STORAGE_POOL_NETFS && + def->type != VIR_STORAGE_POOL_GLUSTER && + def->type != VIR_STORAGE_POOL_ISCSI && + def->type != VIR_STORAGE_POOL_SHEEPDOG) + return 0; + + for (i = 0; i < pools->count; i++) { + virStoragePoolObjPtr pool = pools->objs[i]; + virStoragePoolSourcePtr poolsrc, defsrc; + + virStoragePoolObjLock(pool); + + poolsrc = &pool->def->source; + defsrc = &def->source; + if (pool->active && pool->def->type == def->type && + poolsrc->nhost == 1 && defsrc->nhost == 1) { + + bool matchsrc = false; + const char *poolhost = poolsrc->hosts[0].name; + const char *defhost = defsrc->hosts[0].name; + + /* Only check the port if the to be started pool supplied one */ + if (defsrc->hosts[0].port != 0) { + if (poolsrc->hosts[0].port != defsrc->hosts[0].port) { + virStoragePoolObjUnlock(pool); + continue; + } + } + + if (pool->def->type == VIR_STORAGE_POOL_NETFS || + pool->def->type == VIR_STORAGE_POOL_GLUSTER) { + matchsrc = STREQ(poolsrc->dir, defsrc->dir); + } else if (pool->def->type == VIR_STORAGE_POOL_ISCSI) { + if (virStoragePoolSourceFindDuplicateDevices(pool, def)) + matchsrc = true; + } else if (pool->def->type == VIR_STORAGE_POOL_SHEEPDOG) { + matchsrc = true; + } + + if (matchsrc) { + ret = virSocketAddrIsSameTCPHost(poolhost, defhost); + virStoragePoolObjUnlock(pool); + if (ret == 0) + continue; + if (ret == 1) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("Storage source conflict with pool: '%s'"), + pool->def->name); + ret = -1; + } + goto cleanup; + } + } + virStoragePoolObjUnlock(pool); + } + ret = 0; + + cleanup: + return ret; +} + void virStoragePoolObjLock(virStoragePoolObjPtr obj) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 7471006..0d46376 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -1,7 +1,7 @@ /* * storage_conf.h: config handling for storage driver * - * Copyright (C) 2006-2008, 2010-2014 Red Hat, Inc. + * Copyright (C) 2006-2008, 2010-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -417,6 +417,10 @@ int virStoragePoolSourceFindDuplicate(virConnectPtr conn, virStoragePoolObjListPtr pools, virStoragePoolDefPtr def); +int virStoragePoolSourceHostCompare(virStoragePoolObjListPtr pools, + virStoragePoolDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void virStoragePoolObjLock(virStoragePoolObjPtr obj); void virStoragePoolObjUnlock(virStoragePoolObjPtr obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 34cb871..a489c83 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -836,6 +836,7 @@ virStoragePoolSourceDeviceClear; virStoragePoolSourceFindDuplicate; virStoragePoolSourceFindDuplicateDevices; virStoragePoolSourceFree; +virStoragePoolSourceHostCompare; virStoragePoolSourceListFormat; virStoragePoolSourceListNewSource; virStoragePoolTypeFromString; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 306d98e..922863a 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1,7 +1,7 @@ /* * storage_driver.c: core driver for storage APIs * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -674,8 +674,12 @@ storagePoolCreateXML(virConnectPtr conn, if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; + if (virStoragePoolSourceHostCompare(&driver->pools, def) < 0) + goto cleanup; + if (!(pool = virStoragePoolObjAssignDef(&driver->pools, def))) goto cleanup; + def = NULL; if (backend->startPool && @@ -846,6 +850,9 @@ storagePoolCreate(virStoragePoolPtr obj, goto cleanup; } + if (virStoragePoolSourceHostCompare(&driver->pools, pool->def) < 0) + goto cleanup; + VIR_INFO("Starting up storage pool '%s'", pool->def->name); if (backend->startPool && backend->startPool(obj->conn, pool) < 0) -- 2.1.0
participants (2)
-
John Ferlan
-
Peter Krempa