[libvirt] [PATCH v4 4/4] nwfilter: Remove need for nwfilterDriverLock in some API's

[Stefan Berger]

On 12/08/2017 09:01 AM, John Ferlan wrote:
> Now that nwfilters object list is self locking, it's no longer
> necessary to hold the driver level lock for certain API's.
>
> However, for the DefineXML, Undefine, and Reload processing keeping
> that lock ensures for serialization required in order to process
> the filter Instantiation properly.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>   src/nwfilter/nwfilter_driver.c | 51 +++++++++++++++++-------------------------
>   1 file changed, 20 insertions(+), 31 deletions(-)
>
> diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
> index b24f59379..5b7ba1fcd 100644
> --- a/src/nwfilter/nwfilter_driver.c
> +++ b/src/nwfilter/nwfilter_driver.c
> @@ -295,6 +295,10 @@ nwfilterStateReload(void)
>       /* shut down all threads -- they will be restarted if necessary */
>       virNWFilterLearnThreadsTerminate(true);
>
> +    /* Serialization of virNWFilterObjListLoadAllConfigs is extremely
> +     * important as it relates to virNWFilterObjListFindInstantiateFilter
> +     * processing via virNWFilterTriggerVMFilterRebuild that occurs during
> +     * virNWFilterObjListAssignDef */
>       nwfilterDriverLock();
>       virNWFilterWriteLockFilterUpdates();
>       virNWFilterCallbackDriversLock();
> @@ -385,11 +389,7 @@ nwfilterLookupByUUID(virConnectPtr conn,
>       virNWFilterDefPtr def;
>       virNWFilterPtr nwfilter = NULL;
>
> -    nwfilterDriverLock();
> -    obj = nwfilterObjFromNWFilter(uuid);
> -    nwfilterDriverUnlock();
> -
> -    if (!obj)
> +    if (!(obj = nwfilterObjFromNWFilter(uuid)))
>           return NULL;
>       def = virNWFilterObjGetDef(obj);
>
> @@ -412,11 +412,7 @@ nwfilterLookupByName(virConnectPtr conn,
>       virNWFilterDefPtr def;
>       virNWFilterPtr nwfilter = NULL;
>
> -    nwfilterDriverLock();
> -    obj = virNWFilterObjListFindByName(driver->nwfilters, name);
> -    nwfilterDriverUnlock();
> -
> -    if (!obj) {
> +    if (!(obj = virNWFilterObjListFindByName(driver->nwfilters, name))) {
>           virReportError(VIR_ERR_NO_NWFILTER,
>                          _("no nwfilter with matching name '%s'"), name);
>           return NULL;
> @@ -450,17 +446,12 @@ nwfilterConnectListNWFilters(virConnectPtr conn,
>                                char **const names,
>                                int maxnames)
>   {
> -    int nnames;
> -
>       if (virConnectListNWFiltersEnsureACL(conn) < 0)
>           return -1;
>
> -    nwfilterDriverLock();
> -    nnames = virNWFilterObjListGetNames(driver->nwfilters, conn,
> -                                    virConnectListNWFiltersCheckACL,
> -                                    names, maxnames);
> -    nwfilterDriverUnlock();
> -    return nnames;
> +    return virNWFilterObjListGetNames(driver->nwfilters, conn,
> +                                      virConnectListNWFiltersCheckACL,
> +                                      names, maxnames);
>   }
>
>
> @@ -469,19 +460,13 @@ nwfilterConnectListAllNWFilters(virConnectPtr conn,
>                                   virNWFilterPtr **nwfilters,
>                                   unsigned int flags)
>   {
> -    int ret;
> -
>       virCheckFlags(0, -1);
>
>       if (virConnectListAllNWFiltersEnsureACL(conn) < 0)
>           return -1;
>
> -    nwfilterDriverLock();
> -    ret = virNWFilterObjListExport(conn, driver->nwfilters, nwfilters,
> -                                   virConnectListAllNWFiltersCheckACL);
> -    nwfilterDriverUnlock();
> -
> -    return ret;
> +    return virNWFilterObjListExport(conn, driver->nwfilters, nwfilters,
> +                                    virConnectListAllNWFiltersCheckACL);
>   }
>
>   static virNWFilterPtr
> @@ -499,6 +484,10 @@ nwfilterDefineXML(virConnectPtr conn,
>           return NULL;
>       }
>
> +    /* Serialization of *one* DefineXML consumer is extremely important
> +     * as it relates to virNWFilterObjListFindInstantiateFilter processing
> +     * via virNWFilterTriggerVMFilterRebuild that occurs during
> +     * virNWFilterObjListAssignDef */
>       nwfilterDriverLock();
>       virNWFilterWriteLockFilterUpdates();
>       virNWFilterCallbackDriversLock();
> @@ -541,6 +530,10 @@ nwfilterUndefine(virNWFilterPtr nwfilter)
>       virNWFilterDefPtr def;
>       int ret = -1;
>
> +    /* Serialization of *one* Undefine consumer is extremely important
> +     * as it relates to virNWFilterObjListFindInstantiateFilter processing
> +     * via virNWFilterTriggerVMFilterRebuild that occurs during
> +     * virNWFilterObjTestUnassignDef */
>       nwfilterDriverLock();
>       virNWFilterWriteLockFilterUpdates();
>       virNWFilterCallbackDriversLock();
> @@ -587,11 +580,7 @@ nwfilterGetXMLDesc(virNWFilterPtr nwfilter,
>
>       virCheckFlags(0, NULL);
>
> -    nwfilterDriverLock();
> -    obj = nwfilterObjFromNWFilter(nwfilter->uuid);
> -    nwfilterDriverUnlock();
> -
> -    if (!obj)
> +    if (!(obj = nwfilterObjFromNWFilter(nwfilter->uuid)))
>           return NULL;
>       def = virNWFilterObjGetDef(obj);
>

This looks good.