Problem with VIR_FREE() is that we are not linking libvirt-utils.so to our mock libs therefore there will be an unresolved symbol. Fortunately, nsstest that eventually links with the nssmock links also with libvirt-utils.so and thus the symbol is resolved after all. However, if one wants to run the test binary under valgrind it is impossible to do so. Because of the unresolved symbol. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- cfg.mk | 2 +- tests/nssmock.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index a4305a8..69e3f3a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1159,7 +1159,7 @@ exclude_file_name_regexp--sc_prohibit_select = \ ^cfg\.mk$$ exclude_file_name_regexp--sc_prohibit_raw_allocation = \ - ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|vircgroupmock)\.c|tools/wireshark/src/packet-libvirt\.c)$$ + ^(docs/hacking\.html\.in|src/util/viralloc\.[ch]|examples/.*|tests/(securityselinuxhelper|(vircgroup|nss)mock)\.c|tools/wireshark/src/packet-libvirt\.c)$$ exclude_file_name_regexp--sc_prohibit_readlink = \ ^src/(util/virutil|lxc/lxc_container)\.c$$ diff --git a/tests/nssmock.c b/tests/nssmock.c index b0259a3..273af06 100644 --- a/tests/nssmock.c +++ b/tests/nssmock.c @@ -91,7 +91,7 @@ open(const char *path, int flags, ...) ret = real_open(newpath ? newpath : path, flags); } - VIR_FREE(newpath); + free(newpath); return ret; } @@ -109,7 +109,7 @@ opendir(const char *path) ret = real_opendir(newpath ? newpath : path); - VIR_FREE(newpath); + free(newpath); return ret; } #else -- 2.8.4

Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/libvirt_private.syms | 1 + src/util/virstring.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virstring.h | 3 +++ tests/virstringtest.c | 38 ++++++++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bd46e5f..b1f42f2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2465,6 +2465,7 @@ virStringListGetFirstWithPrefix; virStringListHasString; virStringListJoin; virStringListLength; +virStringListRemove; virStringReplace; virStringSearch; virStringSortCompare; diff --git a/src/util/virstring.c b/src/util/virstring.c index d2fb543..7d2c4c7 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -206,6 +206,56 @@ virStringListAdd(const char **strings, /** + * virStringListRemove: + * @strings: a NULL-terminated array of strings + * @newStrings: new NULL-terminated array of strings + * @item: string to remove + * + * Creates new strings list with all strings duplicated except + * for every occurrence of @item. Callers is responsible for + * freeing both @strings and returned list. + * + * Returns the number of items in the new list (excluding NULL + * anchor), -1 on error. + */ +int +virStringListRemove(const char **strings, + char ***newStrings, + const char *item) +{ + char **ret = NULL; + size_t i, j = 0; + + for (i = 0; strings && strings[i]; i++) { + if (STRNEQ(strings[i], item)) + j++; + } + + if (!j) { + *newStrings = NULL; + return 0; + } + + if (VIR_ALLOC_N(ret, j + 1) < 0) + goto error; + + for (i = 0, j = 0; strings[i]; i++) { + if (STREQ(strings[i], item)) + continue; + if (VIR_STRDUP(ret[j], strings[i]) < 0) + goto error; + j++; + } + + *newStrings = ret; + return j; + error: + virStringListFree(ret); + return -1; +} + + +/** * virStringListFree: * @str_array: a NULL-terminated array of strings to free * diff --git a/src/util/virstring.h b/src/util/virstring.h index da9d35c..88eacd4 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -43,6 +43,9 @@ char *virStringListJoin(const char **strings, char **virStringListAdd(const char **strings, const char *item); +int virStringListRemove(const char **strings, + char ***newStrings, + const char *item); void virStringListFree(char **strings); void virStringListFreeCount(char **strings, diff --git a/tests/virstringtest.c b/tests/virstringtest.c index 43657c8..a421f7b 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -162,6 +162,42 @@ static int testAdd(const void *args) } +static int testRemove(const void *args) +{ + const struct testSplitData *data = args; + char **list = NULL; + size_t ntokens; + size_t i; + int ret = -1; + + if (!(list = virStringSplitCount(data->string, data->delim, + data->max_tokens, &ntokens))) { + VIR_DEBUG("Got no tokens at all"); + return -1; + } + + for (i = 0; data->tokens[i]; i++) { + char **tmp; + if (virStringListRemove((const char **) list, + &tmp, data->tokens[i]) < 0) + goto cleanup; + virStringListFree(list); + list = tmp; + tmp = NULL; + } + + if (list && list[0]) { + virFilePrintf(stderr, "Not removed all tokens: %s", list[0]); + goto cleanup; + } + + ret = 0; + cleanup: + virStringListFree(list); + return ret; +} + + static bool fail; static const char * @@ -636,6 +672,8 @@ mymain(void) ret = -1; \ if (virTestRun("Add " #str, testAdd, &joinData) < 0) \ ret = -1; \ + if (virTestRun("Remove " #str, testRemove, &splitData) < 0) \ + ret = -1; \ } while (0) const char *tokens1[] = { NULL }; -- 2.8.4

On Wed, Nov 30, 2016 at 11:25:55AM +0100, Pino Toscano wrote: I'm not sure to what use would such an information serve. The way the patch suggest removes the necessity for yet another call just to determine the length of the new list. In python, sure, that would be IMHO the correct reasoning as the intended use of the objects and its methods but since the caller is indeed responsible for freeing both of the lists, returning the size of the new list, stripped down from the non-desired elements, is a valuable information for free. Additionally, I'd expect the caller to also know how many elements they're trying to remove. I think in this case you might want to look at this as a black-box, you know what the function should return and that should be enough, therefore returning 0 should be an indicator that there either is a some rubbish assigned by the function or your variable wasn't touched at all, but you cannot tell, the function does not state explicitly what it does with it, you only know that you have to call free() unless there was an error returned by the function. Agreed. Erik

On Wed, Nov 30, 2016 at 10:59:31AM +0100, Michal Privoznik wrote: In on of the reviews I received I was advised not to use this^^ with anything else than pointers, so I'd like to pass this advice further on. As Pino noted, this is a functional primitive other APIs will rely on, so I think we'd better be sure it always returns the correct size, at least I usually don't run valgring on our test suite. Otherwise looks perfectly fine, ACK. Erik

This module will be used to track: <domain, mac address list> pairs. It will be important to know these mappings without libvirt connection (that is from a JSON file), because NSS module will use those to provide better host name translation. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/libvirt_private.syms | 9 + src/util/virmacmap.c | 399 +++++++++++++++++++++++++++++++++++ src/util/virmacmap.h | 48 +++++ tests/Makefile.am | 14 ++ tests/virmacmapmock.c | 29 +++ tests/virmacmaptest.c | 232 ++++++++++++++++++++ tests/virmacmaptestdata/complex.json | 45 ++++ tests/virmacmaptestdata/empty.json | 3 + tests/virmacmaptestdata/simple.json | 8 + tests/virmacmaptestdata/simple2.json | 16 ++ 12 files changed, 805 insertions(+) create mode 100644 src/util/virmacmap.c create mode 100644 src/util/virmacmap.h create mode 100644 tests/virmacmapmock.c create mode 100644 tests/virmacmaptest.c create mode 100644 tests/virmacmaptestdata/complex.json create mode 100644 tests/virmacmaptestdata/empty.json create mode 100644 tests/virmacmaptestdata/simple.json create mode 100644 tests/virmacmaptestdata/simple2.json diff --git a/po/POTFILES.in b/po/POTFILES.in index bdff679..b0a1ed4 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -213,6 +213,7 @@ src/util/virkeyfile.c src/util/virlease.c src/util/virlockspace.c src/util/virlog.c +src/util/virmacmap.c src/util/virnetdev.c src/util/virnetdevbandwidth.c src/util/virnetdevbridge.c diff --git a/src/Makefile.am b/src/Makefile.am index d440548..a9106fa 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -137,6 +137,7 @@ UTIL_SOURCES = \ util/virlockspace.c util/virlockspace.h \ util/virlog.c util/virlog.h \ util/virmacaddr.h util/virmacaddr.c \ + util/virmacmap.h util/virmacmap.c \ util/virnetdev.h util/virnetdev.c \ util/virnetdevbandwidth.h util/virnetdevbandwidth.c \ util/virnetdevbridge.h util/virnetdevbridge.c \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0641030..963876b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1926,6 +1926,15 @@ virMacAddrSet; virMacAddrSetRaw; +# util/virmacmap.h +virMACMapMgrAdd; +virMACMapMgrFlush; +virMACMapMgrFlushStr; +virMACMapMgrLookup; +virMACMapMgrNew; +virMACMapMgrRemove; + + # util/virnetdev.h virNetDevAddMulti; virNetDevDelMulti; diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c new file mode 100644 index 0000000..38c2ffd --- /dev/null +++ b/src/util/virmacmap.c @@ -0,0 +1,399 @@ +/* + * virmacmap.c: MAC address <-> Domain name mapping + * + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Authors: + * Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virmacmap.h" +#include "virobject.h" +#include "virlog.h" +#include "virjson.h" +#include "virfile.h" +#include "virhash.h" +#include "virstring.h" +#include "viralloc.h" + +#define VIR_FROM_THIS VIR_FROM_NETWORK + +VIR_LOG_INIT("util.virmacmap"); + +/** + * VIR_MAC_MAP_FILE_SIZE_MAX: + * + * Macro providing the upper limit on the size of mac maps file + */ +#define VIR_MAC_MAP_FILE_SIZE_MAX (32 * 1024 * 1024) + +struct virMACMapMgr { + virObjectLockable parent; + + virHashTablePtr macs; +}; + + +static virClassPtr virMACMapMgrClass; + + +static void +virMACMapMgrDispose(void *obj) +{ + virMACMapMgrPtr mgr = obj; + virHashFree(mgr->macs); +} + + +static void +virMACMapMgrHashFree(void *payload, const void *name ATTRIBUTE_UNUSED) +{ + virStringListFree(payload); +} + + +static int virMACMapMgrOnceInit(void) +{ + if (!(virMACMapMgrClass = virClassNew(virClassForObjectLockable(), + "virMACMapMgrClass", + sizeof(virMACMapMgr), + virMACMapMgrDispose))) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virMACMapMgr); + + +static int +virMACMapMgrAddLocked(virMACMapMgrPtr mgr, + const char *domain, + const char *mac) +{ + int ret = -1; + const char **macsList = NULL; + char **newMacsList = NULL; + + if ((macsList = virHashLookup(mgr->macs, domain)) && + virStringListHasString(macsList, mac)) { + ret = 0; + goto cleanup; + } + + if (!(newMacsList = virStringListAdd(macsList, mac)) || + virHashUpdateEntry(mgr->macs, domain, newMacsList) < 0) + goto cleanup; + newMacsList = NULL; + + ret = 0; + cleanup: + virStringListFree(newMacsList); + return ret; +} + + +static int +virMACMapMgrRemoveLocked(virMACMapMgrPtr mgr, + const char *domain, + const char *mac) +{ + const char **macsList = NULL; + char **newMacsList = NULL; + int ret = -1; + int rv; + + if (!(macsList = virHashLookup(mgr->macs, domain))) + return 0; + + if (!virStringListHasString(macsList, mac)) { + ret = 0; + goto cleanup; + } + + rv = virStringListRemove(macsList, &newMacsList, mac); + if (rv < 0) { + goto cleanup; + } else if (rv == 0) { + virHashRemoveEntry(mgr->macs, domain); + } else { + if (virHashUpdateEntry(mgr->macs, domain, newMacsList) < 0) + goto cleanup; + } + newMacsList = NULL; + + ret = 0; + cleanup: + virStringListFree(newMacsList); + return ret; +} + + +static int +virMACMapMgrLoadFile(virMACMapMgrPtr mgr, + const char *file) +{ + char *map_str = NULL; + virJSONValuePtr map = NULL; + int map_str_len = 0; + size_t i; + int ret = -1; + + if (virFileExists(file) && + (map_str_len = virFileReadAll(file, + VIR_MAC_MAP_FILE_SIZE_MAX, + &map_str)) < 0) + goto cleanup; + + if (map_str_len == 0) { + ret = 0; + goto cleanup; + } + + if (!(map = virJSONValueFromString(map_str))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("invalid json in file: %s"), + file); + goto cleanup; + } + + if (!virJSONValueIsArray(map)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Malformed file structure: %s"), + file); + goto cleanup; + } + + for (i = 0; i < virJSONValueArraySize(map); i++) { + virJSONValuePtr tmp = virJSONValueArrayGet(map, i); + virJSONValuePtr macs; + const char *domain; + size_t j; + + if (!(domain = virJSONValueObjectGetString(tmp, "domain"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing domain")); + goto cleanup; + } + + if (!(macs = virJSONValueObjectGetArray(tmp, "macs"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing macs")); + goto cleanup; + } + + for (j = 0; j < virJSONValueArraySize(macs); j++) { + virJSONValuePtr macJSON = virJSONValueArrayGet(macs, j); + const char *mac = virJSONValueGetString(macJSON); + + if (virMACMapMgrAddLocked(mgr, domain, mac) < 0) + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(map_str); + virJSONValueFree(map); + return ret; +} + + +static int +virMACMapHashDumper(void *payload, + const void *name, + void *data) +{ + virJSONValuePtr obj = NULL; + virJSONValuePtr arr = NULL; + const char **macs = payload; + size_t i; + int ret = -1; + + if (!(obj = virJSONValueNewObject()) || + !(arr = virJSONValueNewArray())) + goto cleanup; + + for (i = 0; macs[i]; i++) { + virJSONValuePtr m = virJSONValueNewString(macs[i]); + + if (!m || + virJSONValueArrayAppend(arr, m) < 0) { + virJSONValueFree(m); + goto cleanup; + } + } + + if (virJSONValueObjectAppendString(obj, "domain", name) < 0 || + virJSONValueObjectAppend(obj, "macs", arr) < 0) + goto cleanup; + arr = NULL; + + if (virJSONValueArrayAppend(data, obj) < 0) + goto cleanup; + obj = NULL; + + ret = 0; + cleanup: + virJSONValueFree(obj); + virJSONValueFree(arr); + return ret; +} + + +static int +virMACMapMgrDumpStr(virMACMapMgrPtr mgr, + char **str) +{ + virJSONValuePtr arr; + int ret = -1; + + if (!(arr = virJSONValueNewArray())) + goto cleanup; + + if (virHashForEach(mgr->macs, virMACMapHashDumper, arr) < 0) + goto cleanup; + + if (!(*str = virJSONValueToString(arr, true))) + goto cleanup; + + ret = 0; + cleanup: + virJSONValueFree(arr); + return ret; +} + + +static int +virMACMapMgrWriteFile(virMACMapMgrPtr mgr, + const char *file) +{ + char *str; + int ret = -1; + + if (virMACMapMgrDumpStr(mgr, &str) < 0) + goto cleanup; + + if (virFileRewriteStr(file, 0644, str) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(str); + return ret; +} + + +#define VIR_MAC_HASH_TABLE_SIZE 10 + +virMACMapMgrPtr +virMACMapMgrNew(const char *file) +{ + virMACMapMgrPtr mgr; + + if (virMACMapMgrInitialize() < 0) + return NULL; + + if (!(mgr = virObjectLockableNew(virMACMapMgrClass))) + return NULL; + + virObjectLock(mgr); + if (!(mgr->macs = virHashCreate(VIR_MAC_HASH_TABLE_SIZE, + virMACMapMgrHashFree))) + goto error; + + if (file && + virMACMapMgrLoadFile(mgr, file) < 0) + goto error; + + virObjectUnlock(mgr); + return mgr; + + error: + virObjectUnlock(mgr); + virObjectUnref(mgr); + return NULL; +} + + +int +virMACMapMgrAdd(virMACMapMgrPtr mgr, + const char *domain, + const char *mac) +{ + int ret; + + virObjectLock(mgr); + ret = virMACMapMgrAddLocked(mgr, domain, mac); + virObjectUnlock(mgr); + return ret; +} + + +int +virMACMapMgrRemove(virMACMapMgrPtr mgr, + const char *domain, + const char *mac) +{ + int ret; + + virObjectLock(mgr); + ret = virMACMapMgrRemoveLocked(mgr, domain, mac); + virObjectUnlock(mgr); + return ret; +} + + +const char *const * +virMACMapMgrLookup(virMACMapMgrPtr mgr, + const char *domain) +{ + const char *const *ret; + + virObjectLock(mgr); + ret = virHashLookup(mgr->macs, domain); + virObjectUnlock(mgr); + return ret; +} + + +int +virMACMapMgrFlush(virMACMapMgrPtr mgr, + const char *filename) +{ + int ret; + + virObjectLock(mgr); + ret = virMACMapMgrWriteFile(mgr, filename); + virObjectUnlock(mgr); + return ret; +} + + +int +virMACMapMgrFlushStr(virMACMapMgrPtr mgr, + char **str) +{ + int ret; + + virObjectLock(mgr); + ret = virMACMapMgrDumpStr(mgr, str); + virObjectUnlock(mgr); + return ret; +} diff --git a/src/util/virmacmap.h b/src/util/virmacmap.h new file mode 100644 index 0000000..6ebba7c --- /dev/null +++ b/src/util/virmacmap.h @@ -0,0 +1,48 @@ +/* + * virmacmap.h: MAC address <-> Domain name mapping + * + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Authors: + * Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#ifndef __VIR_MACMAP_H__ +# define __VIR_MACMAP_H__ + +typedef struct virMACMapMgr virMACMapMgr; +typedef virMACMapMgr *virMACMapMgrPtr; + +virMACMapMgrPtr virMACMapMgrNew(const char *file); + +int virMACMapMgrAdd(virMACMapMgrPtr mgr, + const char *domain, + const char *mac); + +int virMACMapMgrRemove(virMACMapMgrPtr mgr, + const char *domain, + const char *mac); + +const char *const *virMACMapMgrLookup(virMACMapMgrPtr mgr, + const char *domain); + +int virMACMapMgrFlush(virMACMapMgrPtr mgr, + const char *filename); + +int virMACMapMgrFlushStr(virMACMapMgrPtr mgr, + char **str); +#endif /* __VIR_MACMAPPING_H__ */ diff --git a/tests/Makefile.am b/tests/Makefile.am index 6a24861..9d5583d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -144,6 +144,7 @@ EXTRA_DIST = \ vircgroupdata \ virconfdata \ virfiledata \ + virmacmaptestdata \ virmock.h \ virnetdaemondata \ virnetdevtestdata \ @@ -191,6 +192,7 @@ test_programs = virshtest sockettest \ domainconftest \ virhostdevtest \ vircaps2xmltest \ + virmacmaptest \ virnetdevtest \ virtypedparamtest \ $(NULL) @@ -406,6 +408,7 @@ test_libraries = libshunload.la \ virhostcpumock.la \ nssmock.la \ domaincapsmock.la \ + virmacmapmock.la \ $(NULL) if WITH_QEMU test_libraries += libqemumonitortestutils.la \ @@ -1137,6 +1140,17 @@ nsslinktest_CFLAGS = \ nsslinktest_LDADD = ../tools/nss/libnss_libvirt_impl.la nsslinktest_LDFLAGS = $(NULL) +virmacmapmock_la_SOURCES = \ + virmacmapmock.c +virmacmapmock_la_CFLAGS = $(AM_CFLAGS) +virmacmapmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virmacmapmock_la_LIBADD = $(MOCKLIBS_LIBS) + +virmacmaptest_SOURCES = \ + virmacmaptest.c testutils.h testutils.c +virmacmaptest_CLFAGS = $(AM_CFLAGS); +virmacmaptest_LDADD = $(LDADDS) + virnetdevtest_SOURCES = \ virnetdevtest.c testutils.h testutils.c virnetdevtest_CFLAGS = $(AM_CFLAGS) $(LIBNL_CFLAGS) diff --git a/tests/virmacmapmock.c b/tests/virmacmapmock.c new file mode 100644 index 0000000..d01f1c9 --- /dev/null +++ b/tests/virmacmapmock.c @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "virrandom.h" + +uint64_t virRandomBits(int nbits ATTRIBUTE_UNUSED) +{ + return 4; /* chosen by fair dice roll. + guaranteed to be random. */ +} diff --git a/tests/virmacmaptest.c b/tests/virmacmaptest.c new file mode 100644 index 0000000..a983b54 --- /dev/null +++ b/tests/virmacmaptest.c @@ -0,0 +1,232 @@ +/* + * Copyright (C) 2016 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>;. + * + * Author: Michal Privoznik <mprivozn(a)redhat.com&gt; + */ + +#include <config.h> + +#include "testutils.h" +#include "virmacmap.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +struct testData { + const char *file; + const char *domain; + const char * const * macs; + virMACMapMgrPtr mgr; +}; + + +static int +testMACLookup(const void *opaque) +{ + const struct testData *data = opaque; + virMACMapMgrPtr mgr = NULL; + const char * const * macs; + size_t i, j; + char *file = NULL; + int ret = -1; + + if (virAsprintf(&file, "%s/virmacmaptestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virMACMapMgrNew(file))) + goto cleanup; + + macs = virMACMapMgrLookup(mgr, data->domain); + + for (i = 0; macs && macs[i]; i++) { + for (j = 0; data->macs && data->macs[j]; j++) { + if (STREQ(macs[i], data->macs[j])) + break; + } + + if (!data->macs || !data->macs[j]) { + fprintf(stderr, + "Unexpected %s in the returned list of MACs\n", macs[i]); + goto cleanup; + } + } + + for (i = 0; data->macs && data->macs[i]; i++) { + for (j = 0; macs && macs[j]; j++) { + if (STREQ(data->macs[i], macs[j])) + break; + } + + if (!macs || !macs[j]) { + fprintf(stderr, + "Expected %s in the returned list of MACs\n", data->macs[i]); + goto cleanup; + } + } + + ret = 0; + cleanup: + VIR_FREE(file); + virObjectUnref(mgr); + return ret; +} + + +static int +testMACRemove(const void *opaque) +{ + const struct testData *data = opaque; + virMACMapMgrPtr mgr = NULL; + const char * const * macs; + size_t i; + char *file = NULL; + int ret = -1; + + if (virAsprintf(&file, "%s/virmacmaptestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (!(mgr = virMACMapMgrNew(file))) + goto cleanup; + + for (i = 0; data->macs && data->macs[i]; i++) { + if (virMACMapMgrRemove(mgr, data->domain, data->macs[i]) < 0) { + fprintf(stderr, + "Error when removing %s from the list of MACs\n", data->macs[i]); + goto cleanup; + } + } + + if ((macs = virMACMapMgrLookup(mgr, data->domain))) { + fprintf(stderr, + "Not removed all MACs for domain %s: %s\n", data->domain, macs[0]); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(file); + virObjectUnref(mgr); + return ret; +} + + +static int +testMACFlush(const void *opaque) +{ + const struct testData *data = opaque; + char *file = NULL; + char *str = NULL; + int ret = -1; + + if (virAsprintf(&file, "%s/virmacmaptestdata/%s.json", + abs_srcdir, data->file) < 0) + goto cleanup; + + if (virMACMapMgrFlushStr(data->mgr, &str) < 0) + goto cleanup; + + if (virTestCompareToFile(str, file) < 0) + goto cleanup; + + ret = 0; + cleanup: + VIR_FREE(file); + VIR_FREE(str); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + virMACMapMgrPtr mgr = NULL; + +#define DO_TEST_BASIC(f, d, ...) \ + do { \ + const char * const m[] = {__VA_ARGS__, NULL }; \ + struct testData data = {.file = f, .domain = d, .macs = m}; \ + if (virTestRun("Lookup " #d " in " #f, \ + testMACLookup, &data) < 0) \ + ret = -1; \ + if (virTestRun("Remove " #d " in " #f, \ + testMACRemove, &data) < 0) \ + ret = -1; \ + } while (0) + +#define DO_TEST_FLUSH_PROLOGUE \ + if (!(mgr = virMACMapMgrNew(NULL))) \ + ret = -1; + +#define DO_TEST_FLUSH(d, ...) \ + do { \ + const char * const m[] = {__VA_ARGS__, NULL }; \ + size_t i; \ + for (i = 0; m[i]; i++) { \ + if (virMACMapMgrAdd(mgr, d, m[i]) < 0) { \ + virObjectUnref(mgr); \ + mgr = NULL; \ + ret = -1; \ + } \ + } \ + } while (0) + + +#define DO_TEST_FLUSH_EPILOGUE(f) \ + do { \ + struct testData data = {.file = f, .mgr = mgr}; \ + if (virTestRun("Flush " #f, testMACFlush, &data) < 0) \ + ret = -1; \ + virObjectUnref(mgr); \ + mgr = NULL; \ + } while (0) + + DO_TEST_BASIC("empty", "none", NULL); + DO_TEST_BASIC("simple", "f24", "aa:bb:cc:dd:ee:ff"); + DO_TEST_BASIC("simple2", "f24", "aa:bb:cc:dd:ee:ff", "a1:b2:c3:d4:e5:f6"); + DO_TEST_BASIC("simple2", "f25", "00:11:22:33:44:55", "aa:bb:cc:00:11:22"); + + DO_TEST_FLUSH_PROLOGUE; + DO_TEST_FLUSH_EPILOGUE("empty"); + + DO_TEST_FLUSH_PROLOGUE; + DO_TEST_FLUSH("f24", "aa:bb:cc:dd:ee:ff"); + DO_TEST_FLUSH_EPILOGUE("simple"); + + DO_TEST_FLUSH_PROLOGUE; + DO_TEST_FLUSH("f24", "aa:bb:cc:dd:ee:ff", "a1:b2:c3:d4:e5:f6"); + DO_TEST_FLUSH("f25", "00:11:22:33:44:55", "aa:bb:cc:00:11:22"); + DO_TEST_FLUSH_EPILOGUE("simple2"); + + DO_TEST_FLUSH_PROLOGUE; + DO_TEST_FLUSH("dom0", "e1:81:5d:f3:41:57", "76:0a:2a:a0:51:86", "01:c7:fc:01:c7:fc"); + DO_TEST_FLUSH("dom0", "8e:82:53:60:32:4a", "14:7a:25:dc:7d:a0", "f8:d7:75:f8:d7:75"); + DO_TEST_FLUSH("dom0", "73:d2:50:fb:0f:b1", "82:ee:a7:9b:e3:69", "a8:b4:cb:a8:b4:cb"); + DO_TEST_FLUSH("dom0", "7e:81:86:0f:0b:fb", "94:e2:00:d9:4c:70", "dc:7b:83:dc:7b:83"); + DO_TEST_FLUSH("dom0", "d1:19:a5:a1:52:a8", "22:03:a0:bf:cb:4a", "e3:c7:f8:e3:c7:f8"); + DO_TEST_FLUSH("dom0", "aa:bf:3f:4f:21:8d", "28:67:45:72:8f:47", "eb:08:cd:eb:08:cd"); + DO_TEST_FLUSH("dom0", "bd:f8:a7:e5:e2:bd", "c7:80:e3:b9:18:4d", "ce:da:c0:ce:da:c0"); + DO_TEST_FLUSH("dom1", "8b:51:1d:9f:2f:29", "7c:ae:4c:3e:e1:11", "c6:68:4e:98:ff:6a"); + DO_TEST_FLUSH("dom1", "43:0e:33:a1:3f:0f", "7a:3e:ed:bb:15:27", "b1:17:fd:95:d2:1b"); + DO_TEST_FLUSH("dom1", "9e:89:49:99:51:0e", "89:b4:3f:08:88:2c", "54:0b:4c:e2:0a:39"); + DO_TEST_FLUSH("dom1", "bb:88:07:19:51:9d", "b7:f1:1a:40:a2:95", "88:94:39:a3:90:b4"); + DO_TEST_FLUSH_EPILOGUE("complex"); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virmacmapmock.so") diff --git a/tests/virmacmaptestdata/complex.json b/tests/virmacmaptestdata/complex.json new file mode 100644 index 0000000..192347c --- /dev/null +++ b/tests/virmacmaptestdata/complex.json @@ -0,0 +1,45 @@ +[ + { + "domain": "dom0", + "macs": [ + "e1:81:5d:f3:41:57", + "76:0a:2a:a0:51:86", + "01:c7:fc:01:c7:fc", + "8e:82:53:60:32:4a", + "14:7a:25:dc:7d:a0", + "f8:d7:75:f8:d7:75", + "73:d2:50:fb:0f:b1", + "82:ee:a7:9b:e3:69", + "a8:b4:cb:a8:b4:cb", + "7e:81:86:0f:0b:fb", + "94:e2:00:d9:4c:70", + "dc:7b:83:dc:7b:83", + "d1:19:a5:a1:52:a8", + "22:03:a0:bf:cb:4a", + "e3:c7:f8:e3:c7:f8", + "aa:bf:3f:4f:21:8d", + "28:67:45:72:8f:47", + "eb:08:cd:eb:08:cd", + "bd:f8:a7:e5:e2:bd", + "c7:80:e3:b9:18:4d", + "ce:da:c0:ce:da:c0" + ] + }, + { + "domain": "dom1", + "macs": [ + "8b:51:1d:9f:2f:29", + "7c:ae:4c:3e:e1:11", + "c6:68:4e:98:ff:6a", + "43:0e:33:a1:3f:0f", + "7a:3e:ed:bb:15:27", + "b1:17:fd:95:d2:1b", + "9e:89:49:99:51:0e", + "89:b4:3f:08:88:2c", + "54:0b:4c:e2:0a:39", + "bb:88:07:19:51:9d", + "b7:f1:1a:40:a2:95", + "88:94:39:a3:90:b4" + ] + } +] diff --git a/tests/virmacmaptestdata/empty.json b/tests/virmacmaptestdata/empty.json new file mode 100644 index 0000000..41b42e6 --- /dev/null +++ b/tests/virmacmaptestdata/empty.json @@ -0,0 +1,3 @@ +[ + +] diff --git a/tests/virmacmaptestdata/simple.json b/tests/virmacmaptestdata/simple.json new file mode 100644 index 0000000..ea0fb08 --- /dev/null +++ b/tests/virmacmaptestdata/simple.json @@ -0,0 +1,8 @@ +[ + { + "domain": "f24", + "macs": [ + "aa:bb:cc:dd:ee:ff" + ] + } +] diff --git a/tests/virmacmaptestdata/simple2.json b/tests/virmacmaptestdata/simple2.json new file mode 100644 index 0000000..91b2cde --- /dev/null +++ b/tests/virmacmaptestdata/simple2.json @@ -0,0 +1,16 @@ +[ + { + "domain": "f25", + "macs": [ + "00:11:22:33:44:55", + "aa:bb:cc:00:11:22" + ] + }, + { + "domain": "f24", + "macs": [ + "aa:bb:cc:dd:ee:ff", + "a1:b2:c3:d4:e5:f6" + ] + } +] -- 2.8.4

So far the NSS module looks up only hostnames as provided by guests themselves. However, there are some cases where this is not enough: e.g. when there's a fresh new guest being installed (with some generic hostname) say from a live ISO image; or some (older) systems don't advertise their hostname in DHCP transactions at all. In cases like that it would be helpful if we translate domain name as seen by libvirt too so that users can: # virsh start $dom && ssh $dom Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- docs/nss.html.in | 13 ++-- src/Makefile.am | 8 ++ tests/nssdata/virbr0.macs | 23 ++++++ tests/nssdata/virbr0.status | 5 ++ tests/nssdata/virbr1.macs | 21 ++++++ tests/nssdata/virbr1.status | 5 ++ tests/nssmock.c | 21 ++++++ tests/nsstest.c | 2 + tools/nss/libvirt_nss.c | 173 ++++++++++++++++++++++++++++++++++---------- 9 files changed, 226 insertions(+), 45 deletions(-) create mode 100644 tests/nssdata/virbr0.macs create mode 100644 tests/nssdata/virbr1.macs diff --git a/docs/nss.html.in b/docs/nss.html.in index 84ea8df..dc0ebfa 100644 --- a/docs/nss.html.in +++ b/docs/nss.html.in @@ -100,9 +100,10 @@ hosts: files libvirt dns <h2><a name="Limitations">Limitations</a></h2> <ol> - <li>The libvirt NSS module matches only hostnames provided by guest. If - the libvirt name and one advertised by guest differs, the latter is - matched.</li> + <li><del>The libvirt NSS module matches only hostnames provided by guest. + If the libvirt name and one advertised by guest differs, the latter is + matched.</del> As of <code>v2.6.0</code> the libvirt nss module + translates both hostnames provided by guest and libvirt domain names.</li> <li>The module works only in that cases where IP addresses are assigned by dnsmasq spawned by libvirt. Libvirt NATed networks are typical example.</li> @@ -134,8 +135,10 @@ virsh domifaddr --source lease $domain </pre> <p> - If there's no record for either of the aforementioned commands, it's very - likely that NSS module won't find anything and vice versa. + <del>If there's no record for either of the aforementioned commands, it's + very likely that NSS module won't find anything and vice versa.</del> + As of <code>v2.6.0</code> the NSS module should provide hostname + translation even if no output had been produced by aforementioned command. </p> </body> </html> diff --git a/src/Makefile.am b/src/Makefile.am index a9106fa..8c620d5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -3033,6 +3033,10 @@ libvirt_nss_la_SOURCES = \ util/virerror.h \ util/virfile.c \ util/virfile.h \ + util/virhash.c \ + util/virhash.h \ + util/virhashcode.c \ + util/virhashcode.h \ util/virjson.c \ util/virjson.h \ util/virkmod.c \ @@ -3041,12 +3045,16 @@ libvirt_nss_la_SOURCES = \ util/virlease.h \ util/virlog.c \ util/virlog.h \ + util/virmacmap.c \ + util/virmacmap.h \ util/virobject.c \ util/virobject.h \ util/virpidfile.c \ util/virpidfile.h \ util/virprocess.c \ util/virprocess.h \ + util/virrandom.c \ + util/virrandom.h \ util/virsocketaddr.c \ util/virsocketaddr.h \ util/virstring.c \ diff --git a/tests/nssdata/virbr0.macs b/tests/nssdata/virbr0.macs new file mode 100644 index 0000000..45f9e2b --- /dev/null +++ b/tests/nssdata/virbr0.macs @@ -0,0 +1,23 @@ +[ + { + "domain": "fedora", + "macs": [ + "52:54:00:a4:6f:91", + "52:54:00:a4:6f:92" + ] + }, + { + "domain": "gentoo", + "macs": [ + "52:54:00:3a:b5:0c" + ] + }, + { + "domain": "debian", + "macs": [ + "52:54:00:11:22:33", + "52:54:00:a2:37:4c", + "52:54:00:f8:5b:1d" + ] + } +] diff --git a/tests/nssdata/virbr0.status b/tests/nssdata/virbr0.status index 6ebe954..d040774 100644 --- a/tests/nssdata/virbr0.status +++ b/tests/nssdata/virbr0.status @@ -16,5 +16,10 @@ "mac-address": "52:54:00:3a:b5:0c", "hostname": "gentoo", "expiry-time": 2000000000 + }, + { + "ip-address": "192.168.122.2", + "mac-address": "52:54:00:11:22:33", + "expiry-time": 2000000000 } ] diff --git a/tests/nssdata/virbr1.macs b/tests/nssdata/virbr1.macs new file mode 100644 index 0000000..2140737 --- /dev/null +++ b/tests/nssdata/virbr1.macs @@ -0,0 +1,21 @@ +[ + { + "domain": "fedora", + "macs": [ + "52:54:00:a4:6f:93", + "52:54:00:a4:6f:94" + ] + }, + { + "domain": "gentoo", + "macs": [ + "52:54:00:04:be:64" + ] + }, + { + "domain": "suse", + "macs": [ + "52:54:00:aa:bb:cc" + ] + } +] diff --git a/tests/nssdata/virbr1.status b/tests/nssdata/virbr1.status index f8a9157..4951d45 100644 --- a/tests/nssdata/virbr1.status +++ b/tests/nssdata/virbr1.status @@ -16,5 +16,10 @@ "mac-address": "52:54:00:a4:6f:94", "hostname": "fedora", "expiry-time": 1 + }, + { + "ip-address": "192.168.122.3", + "mac-address": "52:54:00:aa:bb:cc", + "expiry-time": 2000000000 } ] diff --git a/tests/nssmock.c b/tests/nssmock.c index 273af06..7929c30 100644 --- a/tests/nssmock.c +++ b/tests/nssmock.c @@ -26,6 +26,7 @@ # include <dirent.h> # include <sys/stat.h> # include <fcntl.h> +# include <unistd.h> # include "configmake.h" # include "virstring.h" @@ -33,6 +34,7 @@ static int (*real_open)(const char *path, int flags, ...); static DIR * (*real_opendir)(const char *name); +static int (*real_access)(const char *path, int mode); # define LEASEDIR LOCALSTATEDIR "/lib/libvirt/dnsmasq/" @@ -47,6 +49,7 @@ init_syms(void) VIR_MOCK_REAL_INIT(open); VIR_MOCK_REAL_INIT(opendir); + VIR_MOCK_REAL_INIT(access); } static int @@ -112,6 +115,24 @@ opendir(const char *path) free(newpath); return ret; } + +int +access(const char *path, int mode) +{ + int ret; + char *newpath = NULL; + + init_syms(); + + if (STRPREFIX(path, LEASEDIR) && + getrealpath(&newpath, path) < 0) + return -1; + + ret = real_access(newpath ? newpath : path, mode); + + free(newpath); + return ret; +} #else /* Nothing to override if NSS plugin is not enabled */ #endif diff --git a/tests/nsstest.c b/tests/nsstest.c index 8648c4a..3cf2795 100644 --- a/tests/nsstest.c +++ b/tests/nsstest.c @@ -189,6 +189,8 @@ mymain(void) DO_TEST("gentoo", AF_INET6, "2001:1234:dead:beef::2"); DO_TEST("gentoo", AF_UNSPEC, "192.168.122.254"); DO_TEST("non-existent", AF_UNSPEC, NULL); + DO_TEST("debian", AF_INET, "192.168.122.2"); + DO_TEST("suse", AF_INET, "192.168.122.3"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 0d59825..c7802fc 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -47,6 +47,8 @@ #include "virstring.h" #include "virsocketaddr.h" #include "configmake.h" +#include "virmacmap.h" +#include "virobject.h" #if 0 # define ERROR(...) \ @@ -79,6 +81,68 @@ typedef struct { int af; } leaseAddress; + +static int +appendAddr(leaseAddress **tmpAddress, + size_t *ntmpAddress, + virJSONValuePtr lease, + int af) +{ + int ret = -1; + const char *ipAddr; + virSocketAddr sa; + int family; + size_t i; + + if (!(ipAddr = virJSONValueObjectGetString(lease, "ip-address"))) { + ERROR("ip-address field missing for %s", name); + goto cleanup; + } + + DEBUG("IP address: %s", ipAddr); + + if (virSocketAddrParse(&sa, ipAddr, AF_UNSPEC) < 0) { + ERROR("Unable to parse %s", ipAddr); + goto cleanup; + } + + family = VIR_SOCKET_ADDR_FAMILY(&sa); + if (af != AF_UNSPEC && af != family) { + DEBUG("Skipping address which family is %d, %d requested", family, af); + ret = 0; + goto cleanup; + } + + for (i = 0; i < *ntmpAddress; i++) { + if (memcmp((*tmpAddress)[i].addr, + (family == AF_INET ? + (void *) &sa.data.inet4.sin_addr.s_addr : + (void *) &sa.data.inet6.sin6_addr.s6_addr), + FAMILY_ADDRESS_SIZE(family)) == 0) { + DEBUG("IP address already in the list"); + ret = 0; + goto cleanup; + } + } + + if (VIR_REALLOC_N_QUIET(*tmpAddress, *ntmpAddress + 1) < 0) { + ERROR("Out of memory"); + goto cleanup; + } + + (*tmpAddress)[*ntmpAddress].af = family; + memcpy((*tmpAddress)[*ntmpAddress].addr, + (family == AF_INET ? + (void *) &sa.data.inet4.sin_addr.s_addr : + (void *) &sa.data.inet6.sin6_addr.s6_addr), + FAMILY_ADDRESS_SIZE(family)); + (*ntmpAddress)++; + ret = 0; + cleanup: + return ret; +} + + /** * findLease: * @name: domain name to lookup @@ -112,11 +176,13 @@ findLease(const char *name, const char *leaseDir = LEASEDIR; struct dirent *entry; virJSONValuePtr leases_array = NULL; - ssize_t i, nleases; + ssize_t i, j, nleases; leaseAddress *tmpAddress = NULL; size_t ntmpAddress = 0; time_t currtime; long long expirytime; + virMACMapMgrPtr *macmaps = NULL; + size_t nMacmaps = 0; *address = NULL; *naddress = 0; @@ -127,7 +193,6 @@ findLease(const char *name, goto cleanup; } - if (virDirOpenQuiet(&dir, leaseDir) < 0) { ERROR("Failed to open dir '%s'", leaseDir); goto cleanup; @@ -142,23 +207,36 @@ findLease(const char *name, while ((ret = virDirRead(dir, &entry, leaseDir)) > 0) { char *path; - if (!virFileHasSuffix(entry->d_name, ".status")) - continue; + if (virFileHasSuffix(entry->d_name, ".status")) { + if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) + goto cleanup; - if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) - goto cleanup; + DEBUG("Processing %s", path); + if (virLeaseReadCustomLeaseFile(leases_array, path, NULL, NULL) < 0) { + ERROR("Unable to parse %s", path); + VIR_FREE(path); + goto cleanup; + } + VIR_FREE(path); + } else if (virFileHasSuffix(entry->d_name, ".macs")) { + if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) + goto cleanup; - DEBUG("Processing %s", path); + if (VIR_REALLOC_N_QUIET(macmaps, nMacmaps + 1) < 0) { + VIR_FREE(path); + goto cleanup; + } - if (virLeaseReadCustomLeaseFile(leases_array, path, NULL, NULL) < 0) { - ERROR("Unable to parse %s", path); + DEBUG("Processing %s", path); + if (!(macmaps[nMacmaps] = virMACMapMgrNew(path))) { + ERROR("Unable to parse %s", path); + VIR_FREE(path); + goto cleanup; + } + nMacmaps++; VIR_FREE(path); - goto cleanup; } - - VIR_FREE(path); } - VIR_DIR_CLOSE(dir); nleases = virJSONValueArraySize(leases_array); @@ -172,9 +250,6 @@ findLease(const char *name, for (i = 0; i < nleases; i++) { virJSONValuePtr lease; const char *lease_name; - virSocketAddr sa; - const char *ipAddr; - int family; lease = virJSONValueArrayGet(leases_array, i); @@ -204,36 +279,51 @@ findLease(const char *name, DEBUG("Found record for %s", lease_name); *found = true; - if (!(ipAddr = virJSONValueObjectGetString(lease, "ip-address"))) { - ERROR("ip-address field missing for %s", name); + if (appendAddr(&tmpAddress, &ntmpAddress, lease, af) < 0) goto cleanup; - } + } - DEBUG("IP address: %s", ipAddr); + for (i = 0; i < nMacmaps; i++) { + const char **macs = (const char **) virMACMapMgrLookup(macmaps[i], name); - if (virSocketAddrParse(&sa, ipAddr, AF_UNSPEC) < 0) { - ERROR("Unable to parse %s", ipAddr); - goto cleanup; - } - - family = VIR_SOCKET_ADDR_FAMILY(&sa); - if (af != AF_UNSPEC && af != family) { - DEBUG("Skipping address which family is %d, %d requested", family, af); + if (!macs) continue; - } - if (VIR_REALLOC_N_QUIET(tmpAddress, ntmpAddress + 1) < 0) { - ERROR("Out of memory"); - goto cleanup; - } + for (j = 0; j < nleases; j++) { + virJSONValuePtr lease = virJSONValueArrayGet(leases_array, j); + const char *macAddr; + + if (!lease) { + /* This should never happen (TM) */ + ERROR("Unable to get element %zd of %zd", j, nleases); + goto cleanup; + } + + macAddr = virJSONValueObjectGetString(lease, "mac-address"); + if (!macAddr) + continue; + + if (!virStringListHasString(macs, macAddr)) + continue; - tmpAddress[ntmpAddress].af = family; - memcpy(tmpAddress[ntmpAddress].addr, - (family == AF_INET ? - (void *) &sa.data.inet4.sin_addr.s_addr : - (void *) &sa.data.inet6.sin6_addr.s6_addr), - FAMILY_ADDRESS_SIZE(family)); - ntmpAddress++; + if (virJSONValueObjectGetNumberLong(lease, "expiry-time", &expirytime) < 0) { + /* A lease cannot be present without expiry-time */ + ERROR("expiry-time field missing for %s", name); + goto cleanup; + } + + /* Do not report expired lease */ + if (expirytime < (long long) currtime) { + DEBUG("Skipping expired lease for %s", name); + continue; + } + + DEBUG("Found record for %s", name); + *found = true; + + if (appendAddr(&tmpAddress, &ntmpAddress, lease, af) < 0) + goto cleanup; + } } *address = tmpAddress; @@ -248,6 +338,9 @@ findLease(const char *name, VIR_FREE(tmpAddress); virJSONValueFree(leases_array); VIR_DIR_CLOSE(dir); + while (nMacmaps) + virObjectUnref(macmaps[--nMacmaps]); + VIR_FREE(macmaps); return ret; } -- 2.8.4