On 05/15/2018 01:43 PM, Daniel P. Berrangé wrote:
> Introduce a new struct to act as the manager of a collection of
> virNWFilterBindingObjPtr objects.
> ---
> src/conf/Makefile.inc.am | 2 +
> src/conf/virnwfilterbindingobjlist.c | 475 +++++++++++++++++++++++++++
> src/conf/virnwfilterbindingobjlist.h | 66 ++++
> src/libvirt_private.syms | 11 +
> 4 files changed, 554 insertions(+)
> create mode 100644 src/conf/virnwfilterbindingobjlist.c
> create mode 100644 src/conf/virnwfilterbindingobjlist.h
>
[...]
> diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c
> new file mode 100644
> index 0000000000..bf0869a216
> --- /dev/null
> +++ b/src/conf/virnwfilterbindingobjlist.c
> @@ -0,0 +1,475 @@
> +/*
> + * virnwfilterbindingobjlist.c: binding objects list utilities
> + *
> + * Copyright (C) 2006-2018 Red Hat, Inc.
> + * Copyright (C) 2006-2008 Daniel P. Berrange
> + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
All of them? Why not more like the *bindingobj.c...
> + *
> + * 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.
> + *
[...]
> +
> +virNWFilterBindingObjListPtr virNWFilterBindingObjListNew(void)
virNWFilterBindingObjListPtr
virNWFilterBindingObjListNew(void)
> +{
> + virNWFilterBindingObjListPtr bindings;
> +
> + if (virNWFilterBindingObjListInitialize() < 0)
> + return NULL;
> +
> + if (!(bindings = virObjectRWLockableNew(virNWFilterBindingObjListClass)))
> + return NULL;
> +
> + if (!(bindings->objs = virHashCreate(50, virObjectFreeHashData))) {
> + virObjectUnref(bindings);
> + return NULL;
> + }
> +
> + return bindings;
> +}
> +
> +static void virNWFilterBindingObjListDispose(void *obj)
static void
virNWFilterBindingObjListDispose(void *obj)
> +{
> + virNWFilterBindingObjListPtr bindings = obj;
> +
> + virHashFree(bindings->objs);
> +}
[...]
> +/**
> + * @bindings: NWFilterBinding object list pointer
> + * @binding: NWFilterBinding object to be added
> + *
> + * Upon entry @binding should have at least 1 ref and be locked.
> + *
> + * Add the @binding into the @bindings->objs hash
> + * tables. Once successfully added into a table, increase the
> + * reference count since upon removal in virHashRemoveEntry
> + * the virObjectUnref will be called since the hash tables were
> + * configured to call virObjectFreeHashData when the object is
> + * removed from the hash table.
> + *
> + * Returns 0 on success with 3 references and locked
2 refs - 1 from ObjNew and 1 from ObjRef
> + * -1 on failure with 1 reference and locked
> + */
> +static int
> +virNWFilterBindingObjListAddObjLocked(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjPtr binding)
> +{
> + if (virHashAddEntry(bindings->objs, binding->def->portdevname, binding) < 0)
> + return -1;
> + virObjectRef(binding);
> +
> + return 0;
> +}
> +
> +
> +/*
> + * virNWFilterBindingObjListAddLocked:
> + *
> + * The returned @binding from this function will be locked and ref
> + * counted. The caller is expected to use virNWFilterBindingObjEndAPI
> + * when it completes usage.
> + */
> +static virNWFilterBindingObjPtr
> +virNWFilterBindingObjListAddLocked(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingDefPtr def)
> +{
> + virNWFilterBindingObjPtr binding;
> +
> + /* See if a BINDING with matching portdev already exists */
> + if ((binding = virNWFilterBindingObjListFindByPortDevLocked(bindings, def->portdevname))) {
That's a long line
> + virReportError(VIR_ERR_OPERATION_FAILED,
> + _("binding '%s' already exists"),
> + def->portdevname);
> + return NULL;
@binding is returned reffed and locked, so we need to EndAPI on it - so
goto error here.
> + }
> +
> + if (!(binding = virNWFilterBindingObjNew()))
> + goto cleanup;
> + binding->def = def;
> +
> + if (virNWFilterBindingObjListAddObjLocked(bindings, binding) < 0)
> + goto error;
> +
> + cleanup:
> + return binding;
> +
> + error:
> + virNWFilterBindingObjEndAPI(&binding);
> + return NULL;
> +}
> +
> +
> +virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingDefPtr def)
virNWFilterBindingObjPtr
virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings,
virNWFilterBindingDefPtr def)
> +{
> + virNWFilterBindingObjPtr ret;
> +
> + virObjectRWLockWrite(bindings);
> + ret = virNWFilterBindingObjListAddLocked(bindings, def);
> + virObjectRWUnlock(bindings);
> + return ret;
> +}
> +
> +
> +/* The caller must hold lock on 'bindings' in addition to 'virNWFilterBindingObjListRemove'
> + * requirements
> + *
> + * Can be used to remove current element while iterating with
> + * virNWFilterBindingObjListForEach
> + */
> +void
static void
> +virNWFilterBindingObjListRemoveLocked(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjPtr binding)
> +{
> + virHashRemoveEntry(bindings->objs, binding->def->portdevname);
If _virNWFilterBindingObj becomes private then need accessor for @def...
Something not generally necessary in other code since Obj and ObjList
"live" in the same module.
> +}
> +
> +
> +/**
> + * @bindings: Pointer to the binding object list
> + * @binding: NWFilterBinding pointer from either after Add or FindBy* API where the
> + * @binding was successfully added to both the bindings->objs
added to the bindings->objs
> + * hash tables that now would need to be removed.
hash table
> + *
> + * The caller must hold a lock on the driver owning 'bindings',
^^ is this part necessarily true any more since we have self locking
lists? Although for nwfilter - it's one of those that hasn't yet been
converted to the hash table model, so I guess depends on the consumer.
> + * and must also have locked and ref counted 'binding', to ensure
> + * no one else is either waiting for 'binding' or still using it.
> + *
> + * When this function returns, @binding will be removed from the hash
> + * tables and returned with lock and refcnt that was present upon entry.
> + */
> +void
> +virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjPtr binding)
> +{
> + binding->removing = true;
> + virObjectRef(binding);
> + virObjectUnlock(binding);
> + virObjectRWLockWrite(bindings);
> + virObjectLock(binding);
> + virNWFilterBindingObjListRemoveLocked(bindings, binding);
> + virObjectUnref(binding);
> + virObjectRWUnlock(bindings);
> +}
> +
> +
> +static virNWFilterBindingObjPtr
> +virNWFilterBindingObjListLoadStatus(virNWFilterBindingObjListPtr bindings,
> + const char *statusDir,
> + const char *name)
> +{
> + char *statusFile = NULL;
> + virNWFilterBindingObjPtr obj = NULL;
> +
> + if ((statusFile = virNWFilterBindingObjConfigFile(statusDir, name)) == NULL)
> + goto error;
> +
> + if (!(obj = virNWFilterBindingObjParseFile(statusFile)))
> + goto error;
> +
> + if (virHashLookup(bindings->objs, obj->def->portdevname) != NULL) {
If _virNWFilterBindingObj becomes private, then this would require an
accessor for obj->def...
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected binding %s already exists"),
> + obj->def->portdevname);
> + goto error;
> + }
> +
> + if (virNWFilterBindingObjListAddObjLocked(bindings, obj) < 0)
> + goto error;
> +
> + VIR_FREE(statusFile);
> + return obj;
> +
> + error:
> + virNWFilterBindingObjEndAPI(&obj);
> + VIR_FREE(statusFile);
> + return NULL;
> +}
> +
> +
> +int
> +virNWFilterBindingObjListLoadAllConfigs(virNWFilterBindingObjListPtr bindings,
> + const char *configDir)
> +{
> + DIR *dir;
> + struct dirent *entry;
> + int ret = -1;
> + int rc;
> +
> + VIR_INFO("Scanning for configs in %s", configDir);
> +
> + if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0)
> + return rc;
> +
> + virObjectRWLockWrite(bindings);
> +
> + while ((ret = virDirRead(dir, &entry, configDir)) > 0) {
> + virNWFilterBindingObjPtr binding;
> +
> + if (!virFileStripSuffix(entry->d_name, ".xml"))
> + continue;
> +
> + /* NB: ignoring errors, so one malformed config doesn't
> + kill the whole process */
> + VIR_INFO("Loading config file '%s.xml'", entry->d_name);
> + binding = virNWFilterBindingObjListLoadStatus(bindings,
> + configDir,
> + entry->d_name);
> + if (binding) {
> + virNWFilterBindingObjEndAPI(&binding);
> + } else {
> + VIR_ERROR(_("Failed to load config for binding '%s'"), entry->d_name);
> + }
The { } { } aren't necessary, but syntax-check doesn't complain either
> + }
> +
> + VIR_DIR_CLOSE(dir);
> + virObjectRWUnlock(bindings);
> + return ret;
> +}
> +
> +
[...]
> +static void
> +virNWFilterBindingObjListFilter(virNWFilterBindingObjPtr **list,
> + size_t *nbindings,
> + virConnectPtr conn,
> + virNWFilterBindingObjListACLFilter filter)
> +{
> + size_t i = 0;
> +
> + while (i < *nbindings) {
> + virNWFilterBindingObjPtr binding = (*list)[i];
> +
> + virObjectLock(binding);
> +
> + /* do not list the object if:
> + * 1) it's being removed.
> + * 2) connection does not have ACL to see it
> + * 3) it doesn't match the filter
> + */
> + if (binding->removing ||
> + (filter && !filter(conn, binding->def))) {
If @binding is private, then use accessor.
> + virObjectUnlock(binding);
> + virObjectUnref(binding);
> + VIR_DELETE_ELEMENT(*list, i, *nbindings);
> + continue;
> + }
> +
> + virObjectUnlock(binding);
> + i++;
> + }
> +}
> +
> +
> +static int
> +virNWFilterBindingObjListCollect(virNWFilterBindingObjListPtr domlist,
> + virConnectPtr conn,
> + virNWFilterBindingObjPtr **bindings,
> + size_t *nbindings,
> + virNWFilterBindingObjListACLFilter filter)
> +{
> + struct virNWFilterBindingListData data = { NULL, 0 };
> +
> + virObjectRWLockRead(domlist);
> + sa_assert(domlist->objs);
> + if (VIR_ALLOC_N(data.bindings, virHashSize(domlist->objs)) < 0) {
> + virObjectRWUnlock(domlist);
> + return -1;
> + }
> +
> + virHashForEach(domlist->objs, virNWFilterBindingObjListCollectIterator, &data);
> + virObjectRWUnlock(domlist);
> +
> + virNWFilterBindingObjListFilter(&data.bindings, &data.nbindings, conn, filter);
> +
> + *nbindings = data.nbindings;
> + *bindings = data.bindings;
> +
> + return 0;
> +}
> +
> +
> +int
> +virNWFilterBindingObjListExport(virNWFilterBindingObjListPtr bindings,
> + virConnectPtr conn,
> + virNWFilterBindingPtr **bindinglist,
> + virNWFilterBindingObjListACLFilter filter)
> +{
> + virNWFilterBindingObjPtr *bindingobjs = NULL;
> + size_t nbindings = 0;
> + size_t i;
> + int ret = -1;
> +
> + if (virNWFilterBindingObjListCollect(bindings, conn, &bindingobjs, &nbindings, filter) < 0)
long line
> + return -1;
> +
> + if (bindinglist) {
> + if (VIR_ALLOC_N(*bindinglist, nbindings + 1) < 0)
> + goto cleanup;
> +
> + for (i = 0; i < nbindings; i++) {
> + virNWFilterBindingObjPtr binding = bindingobjs[i];
> +
> + virObjectLock(binding);
> + (*bindinglist)[i] = virGetNWFilterBinding(conn, binding->def->portdevname, binding->def->filter);
Long line... also another place to have an accessor binding->def
> + virObjectUnlock(binding);
> +
> + if (!(*bindinglist)[i])
> + goto cleanup;
> + }
> + }
> +
> + ret = nbindings;
> +
> + cleanup:
> + virObjectListFreeCount(bindingobjs, nbindings);
> + if (ret < 0) {
> + virObjectListFreeCount(*bindinglist, nbindings);
> + *bindinglist = NULL;
> + }
> + return ret;
> +}
> diff --git a/src/conf/virnwfilterbindingobjlist.h b/src/conf/virnwfilterbindingobjlist.h
> new file mode 100644
> index 0000000000..252da90baa
> --- /dev/null
> +++ b/src/conf/virnwfilterbindingobjlist.h
> @@ -0,0 +1,66 @@
> +/*
> + * virnwfilterbindingobjlist.h: domain objects list utilities
well I know where you copied this from ;-)
> + *
> + * Copyright (C) 2006-2018 Red Hat, Inc.
> + * Copyright (C) 2006-2008 Daniel P. Berrange
> + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
Similar to above.
> + *
> + * 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: Daniel P. Berrange <berrange@xxxxxxxxxx>
> + */
> +
> +#ifndef __VIRNWFILTERBINDINGOBJ_H__
> +# define __VIRNWFILTERBINDINGOBJT_H__
VIR_NWFILTER_BINDING_OBJ_LIST_H
(3 occurrences)
> +
> +# include "virnwfilterbindingobj.h"
> +
> +typedef struct _virNWFilterBindingObjList virNWFilterBindingObjList;
> +typedef virNWFilterBindingObjList *virNWFilterBindingObjListPtr;
> +
These are some long lines - just format like the .c file..
> +virNWFilterBindingObjListPtr virNWFilterBindingObjListNew(void);
> +
> +virNWFilterBindingObjPtr virNWFilterBindingObjListFindByPortDev(virNWFilterBindingObjListPtr bindings,
> + const char *name);
> +
> +virNWFilterBindingObjPtr virNWFilterBindingObjListAdd(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingDefPtr def);
> +
> +void virNWFilterBindingObjListRemove(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjPtr binding);
> +void virNWFilterBindingObjListRemoveLocked(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjPtr binding);
> +
> +int virNWFilterBindingObjListLoadAllConfigs(virNWFilterBindingObjListPtr bindings,
> + const char *configDir);
> +
> +
> +typedef int (*virNWFilterBindingObjListIterator)(virNWFilterBindingObjPtr binding,
> + void *opaque);
> +
> +int virNWFilterBindingObjListForEach(virNWFilterBindingObjListPtr bindings,
> + virNWFilterBindingObjListIterator callback,
> + void *opaque);
> +
> +typedef bool (*virNWFilterBindingObjListACLFilter)(virConnectPtr conn,
> + virNWFilterBindingDefPtr def);
> +
> +int virNWFilterBindingObjListExport(virNWFilterBindingObjListPtr bindings,
> + virConnectPtr conn,
> + virNWFilterBindingPtr **bindinglist,
> + virNWFilterBindingObjListACLFilter filter);
> +
> +
> +#endif /* __VIRNWFILTERBINDINGOBJLIST_H__ */
John
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
