On 01/31/2018 10:10 PM, Stefan Berger wrote:
> On 12/08/2017 09:01 AM, John Ferlan wrote:
>> Implement the self locking object list for nwfilter object lists
>> that uses two hash tables to store the nwfilter object by UUID or
>> by Name.
>>
>> As part of this alter the uuid argument to virNWFilterObjLookupByUUID
>> to expect an already formatted uuidstr.
>>
>> Alter the existing list traversal code to implement the hash table
>> find/lookup/search functionality.
>>
>> Signed-off-by: John Ferlan <jfer...@redhat.com>
>> ---
>>   src/conf/virnwfilterobj.c      | 402
>> ++++++++++++++++++++++++++++-------------
>>   src/conf/virnwfilterobj.h      |   2 +-
>>   src/nwfilter/nwfilter_driver.c |   5 +-
>>   3 files changed, 282 insertions(+), 127 deletions(-)
>>
>> diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c
>> index 6b4758656..a4e6a03d2 100644
>> --- a/src/conf/virnwfilterobj.c
>> +++ b/src/conf/virnwfilterobj.c
>> @@ -43,12 +43,21 @@ struct _virNWFilterObj {
>>   };
>>
>>   struct _virNWFilterObjList {
>> -    size_t count;
>> -    virNWFilterObjPtr *objs;
>> +    virObjectRWLockable parent;
>> +
>> +    /* uuid string -> virNWFilterObj  mapping
>> +     * for O(1), lockless lookup-by-uuid */
>> +    virHashTable *objs;
>> +
>> +    /* name -> virNWFilterObj mapping for O(1),
>> +     * lockless lookup-by-name */
>> +    virHashTable *objsName;
>>   };
>>
>>   static virClassPtr virNWFilterObjClass;
>> +static virClassPtr virNWFilterObjListClass;
>>   static void virNWFilterObjDispose(void *opaque);
>> +static void virNWFilterObjListDispose(void *opaque);
>>
>>
>>   static int
>> @@ -60,6 +69,12 @@ virNWFilterObjOnceInit(void)
>>                                               virNWFilterObjDispose)))
>>           return -1;
>>
>> +    if (!(virNWFilterObjListClass =
>> virClassNew(virClassForObjectRWLockable(),
>> +                                                "virNWFilterObjList",
>> +                                               
>> sizeof(virNWFilterObjList),
>> +                                               
>> virNWFilterObjListDispose)))
>> +        return -1;
>> +
>>       return 0;
>>   }
>>
>> @@ -144,14 +159,20 @@ virNWFilterObjDispose(void *opaque)
>>   }
>>
>>
>> +static void
>> +virNWFilterObjListDispose(void *opaque)
>> +{
>> +    virNWFilterObjListPtr nwfilters = opaque;
>> +
>> +    virHashFree(nwfilters->objs);
>> +    virHashFree(nwfilters->objsName);
>> +}
>> +
>> +
>>   void
>>   virNWFilterObjListFree(virNWFilterObjListPtr nwfilters)
>>   {
>> -    size_t i;
>> -    for (i = 0; i < nwfilters->count; i++)
>> -        virObjectUnref(nwfilters->objs[i]);
>> -    VIR_FREE(nwfilters->objs);
>> -    VIR_FREE(nwfilters);
>> +    virObjectUnref(nwfilters);
>>   }
>>
>>
>> @@ -160,8 +181,23 @@ virNWFilterObjListNew(void)
>>   {
>>       virNWFilterObjListPtr nwfilters;
>>
>> -    if (VIR_ALLOC(nwfilters) < 0)
>> +    if (virNWFilterObjInitialize() < 0)
>> +        return NULL;
>> +
>> +    if (!(nwfilters = virObjectRWLockableNew(virNWFilterObjListClass)))
>> +        return NULL;
>> +
>> +    if (!(nwfilters->objs = virHashCreate(10, virObjectFreeHashData))) {

[1] As part of the magic of hash tables, when an element is removed from
the hash table the virObjectFreeHashData is called which does a
virObjectUnref...

>> +        virObjectUnref(nwfilters);
>> +        return NULL;
>> +    }
>> +
>> +    if (!(nwfilters->objsName = virHashCreate(10,
>> virObjectFreeHashData))) {
>> +        virHashFree(nwfilters->objs);
>> +        virObjectUnref(nwfilters);
>>           return NULL;
>> +    }
>> +
>>       return nwfilters;
>>   }
>>
>> @@ -170,83 +206,105 @@ void
>>   virNWFilterObjListRemove(virNWFilterObjListPtr nwfilters,
>>                            virNWFilterObjPtr obj)
>>   {
>> -    size_t i;
>> -
>> -    virObjectRWUnlock(obj);
>> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
>> +    virNWFilterDefPtr def;
>>
>> -    for (i = 0; i < nwfilters->count; i++) {
>> -        virObjectRWLockWrite(nwfilters->objs[i]);
>> -        if (nwfilters->objs[i] == obj) {
>> -            virObjectRWUnlock(nwfilters->objs[i]);
>> -            virObjectUnref(nwfilters->objs[i]);
>> +    if (!obj)
>> +        return;
>> +    def = obj->def;
>>
>> -            VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count);
>> -            break;
>> -        }
>> -        virObjectRWUnlock(nwfilters->objs[i]);
>> -    }
>> +    virUUIDFormat(def->uuid, uuidstr);
>> +    virObjectRef(obj);
>> +    virObjectRWUnlock(obj);
>> +    virObjectRWLockWrite(nwfilters);
>> +    virObjectRWLockWrite(obj);
>> +    virHashRemoveEntry(nwfilters->objs, uuidstr);
> Unref here after successful removal from hash bucket?
>> +    virHashRemoveEntry(nwfilters->objsName, def->name);
> Again unref here after successful removal ?

[1] So when we RemoveEntry, the data->tableFree (from our Create) is
called. So, yes, that Unref happens automagically.  Once the last ref is
removed the object's disposal API (virNWFilterObjDispose) is called
before being completely reaped.

>> +    virObjectRWUnlock(obj);
>> +    virObjectUnref(obj);
>> +    virObjectRWUnlock(nwfilters);
>>   }
>>
>>
>>   /**
>> - * virNWFilterObjListFindByUUID
>> + * virNWFilterObjListFindByUUID[Locked]
>>    * @nwfilters: Pointer to filter list
>> - * @uuid: UUID to use to lookup the object
>> + * @uuidstr: UUID to use to lookup the object
>> + *
>> + * The static [Locked] version would only be used when the Object
>> List is
>> + * already locked, such as is the case during
>> virNWFilterObjListAssignDef.
>> + * The caller is thus responsible for locking the object.
>>    *
>>    * Search for the object by uuidstr in the hash table and return a read
>>    * locked copy of the object.
>>    *
>> + * Returns: A reffed object or NULL on error
>> + */
>> +static virNWFilterObjPtr
>> +virNWFilterObjListFindByUUIDLocked(virNWFilterObjListPtr nwfilters,
>> +                                   const char *uuidstr)
>> +{
>> +    return virObjectRef(virHashLookup(nwfilters->objs, uuidstr));
>> +}
>> +
>> +
>> +/*
>>    * Returns: A reffed and read locked object or NULL on error
>>    */
>>   virNWFilterObjPtr
>>   virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters,
>> -                             const unsigned char *uuid)
>> +                             const char *uuidstr)
>>   {
>> -    size_t i;
>>       virNWFilterObjPtr obj;
>> -    virNWFilterDefPtr def;
>>
>> -    for (i = 0; i < nwfilters->count; i++) {
>> -        obj = nwfilters->objs[i];
>> +    virObjectRWLockRead(nwfilters);
>> +    obj = virNWFilterObjListFindByUUIDLocked(nwfilters, uuidstr);
>> +    virObjectRWUnlock(nwfilters);
>> +    if (obj)
>>           virObjectRWLockRead(obj);
>> -        def = obj->def;
>> -        if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN))
>> -            return virObjectRef(obj);
>> -        virObjectRWUnlock(obj);
>> -    }
>>
>> -    return NULL;
>> +    return obj;
>>   }
>>
>>
>>   /**
>> - * virNWFilterObjListFindByName
>> + * virNWFilterObjListFindByName[Locked]
>>    * @nwfilters: Pointer to filter list
>>    * @name: filter name to use to lookup the object
>>    *
>> + * The static [Locked] version would only be used when the Object
>> List is
>> + * already locked, such as is the case during
>> virNWFilterObjListAssignDef.
>> + * The caller is thus responsible for locking the object.
>> + *
>>    * Search for the object by name in the hash table and return a read
>>    * locked copy of the object.
>>    *
>> + * Returns: A reffed object or NULL on error
>> + */
>> +static virNWFilterObjPtr
>> +virNWFilterObjListFindByNameLocked(virNWFilterObjListPtr nwfilters,
>> +                                   const char *name)
>> +{
>> +    return virObjectRef(virHashLookup(nwfilters->objsName, name));
>> +}
>> +
>> +
>> +/*
>>    * Returns: A reffed and read locked object or NULL on error
>>    */
>>   virNWFilterObjPtr
>>   virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
>>                                const char *name)
>>   {
>> -    size_t i;
>>       virNWFilterObjPtr obj;
>> -    virNWFilterDefPtr def;
>>
>> -    for (i = 0; i < nwfilters->count; i++) {
>> -        obj = nwfilters->objs[i];
>> +    virObjectRWLockRead(nwfilters);
>> +    obj = virNWFilterObjListFindByNameLocked(nwfilters, name);
>> +    virObjectRWUnlock(nwfilters);
>> +    if (obj)
>>           virObjectRWLockRead(obj);
>> -        def = obj->def;
>> -        if (STREQ_NULLABLE(def->name, name))
>> -            return virObjectRef(obj);
>> -        virObjectRWUnlock(obj);
>> -    }
>>
>> -    return NULL;
>> +    return obj;
>>   }
>>
>>
>> @@ -391,8 +449,11 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr
>> nwfilters,
>>   {
>>       virNWFilterObjPtr obj;
>>       virNWFilterDefPtr objdef;
>> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
>>
>> -    if ((obj = virNWFilterObjListFindByUUID(nwfilters, def->uuid))) {
>> +    virUUIDFormat(def->uuid, uuidstr);
>> +
>> +    if ((obj = virNWFilterObjListFindByUUID(nwfilters, uuidstr))) {
>>           objdef = obj->def;
>>
>>           if (STRNEQ(def->name, objdef->name)) {
>> @@ -406,10 +467,7 @@ virNWFilterObjListAssignDef(virNWFilterObjListPtr
>> nwfilters,
>>           virNWFilterObjEndAPI(&obj);
>>       } else {
>>           if ((obj = virNWFilterObjListFindByName(nwfilters,
>> def->name))) {
>> -            char uuidstr[VIR_UUID_STRING_BUFLEN];
>> -
>>               objdef = obj->def;
>> -            virUUIDFormat(objdef->uuid, uuidstr);
>>               virReportError(VIR_ERR_OPERATION_FAILED,
>>                              _("filter '%s' already exists with uuid
>> %s"),
>>                              def->name, uuidstr);
>> @@ -424,11 +482,13 @@
>> virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
>>           return NULL;
>>       }
>>
>> -
>> -    /* Get a READ lock and immediately promote to WRITE while we adjust
>> -     * data within. */
>> -    if ((obj = virNWFilterObjListFindByName(nwfilters, def->name))) {
>> -        virNWFilterObjPromoteToWrite(obj);
>> +    /* We're about to make some changes to objects on the list - so get
>> +     * the list READ lock in order to Find the object and WRITE lock it
>> +     * while we adjust data within. */
>> +    virObjectRWLockRead(nwfilters);
>> +    if ((obj = virNWFilterObjListFindByNameLocked(nwfilters,
>> def->name))) {
>> +        virObjectRWUnlock(nwfilters);
>> +        virObjectRWLockWrite(obj);
>>
>>           objdef = obj->def;
>>           if (virNWFilterDefEqual(def, objdef)) {
> 
> I think you should have left the above PromoteToWrite(obj) rather than
> doing a virObjectRWLockWrite(obj) now and would then adapt the code here
> as mentioned in review of 2/4.
> 

Hmm... true...  I think that's because originally I had done the list
patch before the object patch... So it's probably one of those merge
things...

Good catch.

>> @@ -458,37 +518,112 @@
>> virNWFilterObjListAssignDef(virNWFilterObjListPtr nwfilters,
>>           return obj;
>>       }
>>
>> +    /* Promote the nwfilters to add a new object */
>> +    virObjectRWUnlock(nwfilters);
>> +    virObjectRWLockWrite(nwfilters);
>>       if (!(obj = virNWFilterObjNew()))
>> -        return NULL;
>> +        goto cleanup;
>>
>> -    if (VIR_APPEND_ELEMENT_COPY(nwfilters->objs,
>> -                                nwfilters->count, obj) < 0) {
>> -        virNWFilterObjEndAPI(&obj);
>> -        return NULL;
>> +    if (virHashAddEntry(nwfilters->objs, uuidstr, obj) < 0)
>> +        goto error;
>> +    virObjectRef(obj);
> 
> good, here you take a reference because obj ended in the hash bucket.
> Therefore, further above, you have to unref when taking it out of the
> hash bucket.
> 
>> +
>> +    if (virHashAddEntry(nwfilters->objsName, def->name, obj) < 0) {
>> +        virHashRemoveEntry(nwfilters->objs, uuidstr);
>> +        goto error;
>>       }
>> +    virObjectRef(obj);
>> +
> Same comment here. Looks also better to me since taking the reference is
> done 'closer' to virHashAddEntry() call.
> 

Right. That was the "end goal"...

>>       obj->def = def;
>>
>> -    return virObjectRef(obj);
>> + cleanup:
>> +    virObjectRWUnlock(nwfilters);
>> +    return obj;
>> +
>> + error:
>> +    virObjectRWUnlock(obj);
>> +    virObjectUnref(obj);
>> +    virObjectRWUnlock(nwfilters);
>> +    return NULL;
>>   }
>>
>>
>> +struct virNWFilterCountData {
>> +    virConnectPtr conn;
>> +    virNWFilterObjListFilter filter;
>> +    int nelems;
>> +};
>> +
>> +static int
>> +virNWFilterObjListNumOfNWFiltersCallback(void *payload,
>> +                                         const void *name
>> ATTRIBUTE_UNUSED,
>> +                                         void *opaque)
>> +{
>> +    virNWFilterObjPtr obj = payload;
>> +    struct virNWFilterCountData *data = opaque;
>> +
>> +    virObjectRWLockRead(obj);
>> +    if (!data->filter || data->filter(data->conn, obj->def))
>> +        data->nelems++;
>> +    virObjectRWUnlock(obj);
>> +    return 0;
>> +}
>> +
>>   int
>>   virNWFilterObjListNumOfNWFilters(virNWFilterObjListPtr nwfilters,
>>                                    virConnectPtr conn,
>>                                    virNWFilterObjListFilter filter)
>>   {
>> -    size_t i;
>> -    int nfilters = 0;
>> +    struct virNWFilterCountData data = { .conn = conn,
>> +        .filter = filter, .nelems = 0 };
> 
> style: one of these per line ?
> 

I've never seen a consistent style for these things, but I can move the
.conn to the same line as .filter which is done in other initializers
where there is only one line.

>>
>> -    for (i = 0; i < nwfilters->count; i++) {
>> -        virNWFilterObjPtr obj = nwfilters->objs[i];
>> -        virObjectRWLockRead(obj);
>> -        if (!filter || filter(conn, obj->def))
>> -            nfilters++;
>> -        virObjectRWUnlock(obj);
>> +    virObjectRWLockRead(nwfilters);
>> +    virHashForEach(nwfilters->objs,
>> virNWFilterObjListNumOfNWFiltersCallback,
>> +                   &data);
>> +    virObjectRWUnlock(nwfilters);
>> +
>> +    return data.nelems;
>> +}
>> +
>> +
>> +struct virNWFilterListData {
>> +    virConnectPtr conn;
>> +    virNWFilterObjListFilter filter;
>> +    int nelems;
>> +    char **elems;
>> +    int maxelems;
>> +    bool error;
>> +};
>> +
>> +static int
>> +virNWFilterObjListGetNamesCallback(void *payload,
>> +                                   const void *name ATTRIBUTE_UNUSED,
>> +                                   void *opaque)
>> +{
>> +    virNWFilterObjPtr obj = payload;
>> +    virNWFilterDefPtr def;
>> +    struct virNWFilterListData *data = opaque;
>> +
>> +    if (data->error)
>> +        return 0;
>> +
>> +    if (data->maxelems >= 0 && data->nelems == data->maxelems)
>> +        return 0;
>> +
>> +    virObjectRWLockRead(obj);
>> +    def = obj->def;
>> +
>> +    if (!data->filter || data->filter(data->conn, def)) {
>> +        if (VIR_STRDUP(data->elems[data->nelems], def->name) < 0) {
>> +            data->error = true;
>> +            goto cleanup;
>> +        }
>> +        data->nelems++;
>>       }
>>
>> -    return nfilters;
>> + cleanup:
>> +    virObjectRWUnlock(obj);
>> +    return 0;
>>   }
>>
>>
>> @@ -499,82 +634,103 @@
>> virNWFilterObjListGetNames(virNWFilterObjListPtr nwfilters,
>>                              char **const names,
>>                              int maxnames)
>>   {
>> -    int nnames = 0;
>> -    size_t i;
>> -    virNWFilterDefPtr def;
>> +    struct virNWFilterListData data = { .conn = conn, .filter = filter,
>> +        .nelems = 0, .elems = names, .maxelems = maxnames, .error =
>> false };
>>
>> -    for (i = 0; i < nwfilters->count && nnames < maxnames; i++) {
>> -        virNWFilterObjPtr obj = nwfilters->objs[i];
>> -        virObjectRWLockRead(obj);
>> -        def = obj->def;
>> -        if (!filter || filter(conn, def)) {
>> -            if (VIR_STRDUP(names[nnames], def->name) < 0) {
>> -                virObjectRWUnlock(obj);
>> -                goto failure;
>> -            }
>> -            nnames++;
>> -        }
>> -        virObjectRWUnlock(obj);
>> -    }
>> +    virObjectRWLockRead(nwfilters);
>> +    virHashForEach(nwfilters->objs,
>> virNWFilterObjListGetNamesCallback, &data);
>> +    virObjectRWUnlock(nwfilters);
>>
>> -    return nnames;
>> +    if (data.error)
>> +        goto error;
>>
>> - failure:
>> -    while (--nnames >= 0)
>> -        VIR_FREE(names[nnames]);
>> +    return data.nelems;
>>
>> + error:
>> +    while (--data.nelems >= 0)
>> +        VIR_FREE(data.elems[data.nelems]);
>>       return -1;
>>   }
>>
>>
>> +struct virNWFilterExportData {
>> +    virConnectPtr conn;
>> +    virNWFilterObjListFilter filter;
>> +    virNWFilterPtr *filters;
>> +    int nfilters;
>> +    bool error;
>> +};
>> +
>> +static int
>> +virNWFilterObjListExportCallback(void *payload,
>> +                                 const void *name ATTRIBUTE_UNUSED,
>> +                                 void *opaque)
>> +{
>> +    virNWFilterObjPtr obj = payload;
>> +     virNWFilterDefPtr def;
>> +
> nit: indentation error
> 

Thanks!

>> +    struct virNWFilterExportData *data = opaque;
>> +    virNWFilterPtr nwfilter;
>> +
>> +    if (data->error)
>> +        return 0;
>> +
>> +    virObjectRWLockRead(obj);
>> +    def = obj->def;
>> +
>> +    if (data->filter && !data->filter(data->conn, def))
>> +        goto cleanup;
>> +
>> +    if (!data->filters) {
>> +        data->nfilters++;
>> +        goto cleanup;
>> +    }
>> +
>> +    if (!(nwfilter = virGetNWFilter(data->conn, def->name,
>> def->uuid))) {
>> +        data->error = true;
>> +        goto cleanup;
>> +    }
>> +    data->filters[data->nfilters++] = nwfilter;
>> +
>> + cleanup:
>> +    virObjectRWUnlock(obj);
>> +    return 0;
>> +}
>> +
>> +
>>   int
>>   virNWFilterObjListExport(virConnectPtr conn,
>>                            virNWFilterObjListPtr nwfilters,
>>                            virNWFilterPtr **filters,
>>                            virNWFilterObjListFilter filter)
>>   {
>> -    virNWFilterPtr *tmp_filters = NULL;
>> -    int nfilters = 0;
>> -    virNWFilterPtr nwfilter = NULL;
>> -    virNWFilterObjPtr obj = NULL;
>> -    virNWFilterDefPtr def;
>> -    size_t i;
>> -    int ret = -1;
>> +    struct virNWFilterExportData data = { .conn = conn, .filter =
>> filter,
>> +        .filters = NULL, .nfilters = 0, .error = false };
>>
>> -    if (!filters) {
>> -        ret = nwfilters->count;
>> -        goto cleanup;
>> +    virObjectRWLockRead(nwfilters);
>> +    if (filters &&
>> +        VIR_ALLOC_N(data.filters, virHashSize(nwfilters->objs) + 1) <
>> 0) {
>> +        virObjectRWUnlock(nwfilters);
>> +        return -1;
>>       }
>>
>> -    if (VIR_ALLOC_N(tmp_filters, nwfilters->count + 1) < 0)
>> -        goto cleanup;
>> +    virHashForEach(nwfilters->objs, virNWFilterObjListExportCallback,
>> &data);
>> +    virObjectRWUnlock(nwfilters);
>>
>> -    for (i = 0; i < nwfilters->count; i++) {
>> -        obj = nwfilters->objs[i];
>> -        virObjectRWLockRead(obj);
>> -        def = obj->def;
>> -        if (!filter || filter(conn, def)) {
>> -            if (!(nwfilter = virGetNWFilter(conn, def->name,
>> def->uuid))) {
>> -                virObjectRWUnlock(obj);
>> -                goto cleanup;
>> -            }
>> -            tmp_filters[nfilters++] = nwfilter;
>> -        }
>> -        virObjectRWUnlock(obj);
>> +    if (data.error)
>> +         goto cleanup;
>> +
>> +    if (data.filters) {
>> +        /* trim the array to the final size */
>> +        ignore_value(VIR_REALLOC_N(data.filters, data.nfilters + 1));
> 
> can we ignore errors here and not return -1 in case an error occurs ?
> 

Yes, the error would be we cannot shrink... This is common between
vir.*ObjList.*Export.* functions

Thanks again for the detailed review.  After I push patch 1, I'll repost
the remaining ones.  Maybe Laine will also have some luck with his
nwfilter testing.  He's pursuing either a locking problem or a *severe*
performance problem with the libvirt-tck test.

John

>> +        *filters = data.filters;
>>       }
>>
>> -    *filters = tmp_filters;
>> -    tmp_filters = NULL;
>> -    ret = nfilters;
>> +    return data.nfilters;
>>
>>    cleanup:
>> -    if (tmp_filters) {
>> -        for (i = 0; i < nfilters; i ++)
>> -            virObjectUnref(tmp_filters[i]);
>> -    }
>> -    VIR_FREE(tmp_filters);
>> -
>> -    return ret;
>> +    virObjectListFree(data.filters);
>> +    return -1;
>>   }
>>
>>
>> diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
>> index 0281bc5f5..cabb42a71 100644
>> --- a/src/conf/virnwfilterobj.h
>> +++ b/src/conf/virnwfilterobj.h
>> @@ -65,7 +65,7 @@ virNWFilterObjListRemove(virNWFilterObjListPtr
>> nwfilters,
>>
>>   virNWFilterObjPtr
>>   virNWFilterObjListFindByUUID(virNWFilterObjListPtr nwfilters,
>> -                             const unsigned char *uuid);
>> +                             const char *uuidstr);
>>
>>   virNWFilterObjPtr
>>   virNWFilterObjListFindByName(virNWFilterObjListPtr nwfilters,
>> diff --git a/src/nwfilter/nwfilter_driver.c
>> b/src/nwfilter/nwfilter_driver.c
>> index b38740b15..b24f59379 100644
>> --- a/src/nwfilter/nwfilter_driver.c
>> +++ b/src/nwfilter/nwfilter_driver.c
>> @@ -369,11 +369,10 @@ nwfilterObjFromNWFilter(const unsigned char *uuid)
>>       virNWFilterObjPtr obj;
>>       char uuidstr[VIR_UUID_STRING_BUFLEN];
>>
>> -    if (!(obj = virNWFilterObjListFindByUUID(driver->nwfilters,
>> uuid))) {
>> -        virUUIDFormat(uuid, uuidstr);
>> +    virUUIDFormat(uuid, uuidstr);
>> +    if (!(obj = virNWFilterObjListFindByUUID(driver->nwfilters,
>> uuidstr)))
>>           virReportError(VIR_ERR_NO_NWFILTER,
>>                          _("no nwfilter with matching uuid '%s'"),
>> uuidstr);
>> -    }
>>       return obj;
>>   }
>>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to