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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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 04/24/2015 04:25 AM, Peter Krempa wrote: 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, Oh yeah ... right, but they won't matter with the adjustment. John

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(a)redhat.com&gt; --- 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