From nobody Tue May 14 21:25:02 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1646907063; cv=none; d=zohomail.com; s=zohoarc; b=k109Gf5j6LqapcSlL4wU5D1i1jjY8HSx/H+JC8gYzvW+061d3dntMS7gXykWGXH7jGCzMmiSzsPxXGy7/QzMM01BhfSDdoAR0YJtPiT+NWcqzQ/mrb+ll6fz8PkPODRgznH/cppH/P27UV/5AEwrPIA6Y9bSv54Ar+7HxeGiVWk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1646907063; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=jankqV9Oxw+vIZR3G45guF5Yl4TJbPdW+qDJdf3C7bU=; b=SrP7mu1wHF2YOjkczxpckI6ygSZ+3j4uY91QDKs8r6GI/XEAtf+bxtL5lGZK3NqX6aP5o8eAbH9RRvm8uCEZ37D6sh7OCdme8trePcqopcUlVlle3Qi954qpm4bMXZ8utTIQ5RSQeVBMtMUXt9xx774Gz2F+OBtcspIiyvyBNsw= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1646907063392834.4728969520786; Thu, 10 Mar 2022 02:11:03 -0800 (PST) Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-85-_dbfZkpqMgqSAnWAuRXfVw-1; Thu, 10 Mar 2022 05:10:23 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id EAF7B3C01DAB; Thu, 10 Mar 2022 10:10:21 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id D7F191457F05; Thu, 10 Mar 2022 10:10:21 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id D6E58195FD4D; Thu, 10 Mar 2022 10:10:19 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 2F59B1953544 for ; Thu, 10 Mar 2022 10:10:19 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id E6D1973DA1; Thu, 10 Mar 2022 10:10:18 +0000 (UTC) Received: from domokun.gsslab.fab.redhat.com (gx270-2.gsslab.fab.redhat.com [10.33.8.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id 620377219B; Thu, 10 Mar 2022 10:10:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1646907062; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=jankqV9Oxw+vIZR3G45guF5Yl4TJbPdW+qDJdf3C7bU=; b=I3lRUc39Yl2SdiXMvcmREqwA0zJQ/+VfxQvLrxjX0huSHmwA+BgJvWkyu9Ac/ptSsZW8jr h/bJfR2OaHVsfUD0957pj9/4Qj1ZS5EA6IQ4Vz59AyfjSPHCZaKVx6EY17aZ3zZjj4X3tw o5KW3MZS1FwuakY8CQzDqaijj0qh/dM= X-MC-Unique: _dbfZkpqMgqSAnWAuRXfVw-1 X-Original-To: libvir-list@listman.corp.redhat.com From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH 1/2] nwfilter: update comment about locking filter updates Date: Thu, 10 Mar 2022 10:10:03 +0000 Message-Id: <20220310101004.1914477-2-berrange@redhat.com> In-Reply-To: <20220310101004.1914477-1-berrange@redhat.com> References: <20220310101004.1914477-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1646907063980100001 The comment against the 'updateMutex' refers to a problem with lock ordering when looking up filters in the virNWFilterObjList which uses an array. That problem does indeed exist. Unfortunately it claims that switching to a hash table would solve the lock ordering problems during instantiation. That is not correct because there is a second lock ordering problem related to how we traverse related filters when instantiating filters. Consider a set of filters: Filter A: Reference Filter C Reference Filter D Filter B: Reference Filter D Reference Filter C In one example, we lock A, C, D, in the other example we lock A, D, C. Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: J=C3=A1n Tomko --- src/nwfilter/nwfilter_gentech_driver.c | 57 ++++++++++++++++++++------ 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter= _gentech_driver.c index 7bbf1e12fb..9f4a755252 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -55,19 +55,52 @@ static virNWFilterTechDriver *filter_tech_drivers[] =3D= { NULL }; =20 -/* Serializes instantiation of filters. This is necessary - * to avoid lock ordering deadlocks. eg virNWFilterInstantiateFilterUpdate - * will hold a lock on a virNWFilterObj *. This in turn invokes - * virNWFilterDoInstantiate which invokes virNWFilterDetermineMissingVarsR= ec - * which invokes virNWFilterObjListFindInstantiateFilter. This iterates ov= er - * every single virNWFilterObj *in the list. So if 2 threads try to - * instantiate a filter in parallel, they'll both hold 1 lock at the top l= evel - * in virNWFilterInstantiateFilterUpdate which will cause the other thread - * to deadlock in virNWFilterObjListFindInstantiateFilter. +/* + * Serializes instantiation of filters. + * + * When instantiating a filter, we need to resolve references + * to other filters and acquire locks on them. The act of + * looking up a filter requires traversing an array, locking + * each filter in turn until we find the one we want. + * + * The mere act of finding a filter by name, while holding + * a lock on another filter, introduces deadlocks due to + * varying lock ordering. + * + * We retain a lock on the referenced filter once found. + * The order in which the locks are acquired depends on + * the order in which filters reference each other. + * + * Filter A: + * Reference Filter C + * Reference Filter D + * + * Filter B: + * Reference Filter D + * Reference Filter C + * + * In one example, we lock A, C, D, in the other example + * we lock A, D, C. + * + * Because C & D are locked in differing orders we are + * once again at risk of deadlocks. + * + * There can be multiple levels of recursion, so it is + * not viable to determine the lock order upfront, it + * has to be done as we traverse the tree. + * + * Thus we serialize any code that needs to traverse + * the filter references. + * + * This covers the following APIs: + * + * virNWFilterDefineXML + * virNWFilterUndefine + * virNWFilterBindingCreate + * virNWFilterBindingDelete * - * XXX better long term solution is to make virNWFilterObjList use a - * hash table as is done for virDomainObjList. You can then get - * lockless lookup of objects by name. + * In addition to the asynchronous filter instantiation + * triggered by the IP address learning backends. */ static virMutex updateMutex; =20 --=20 2.35.1 From nobody Tue May 14 21:25:02 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1646907062; cv=none; d=zohomail.com; s=zohoarc; b=nBi+PQIozBJmivwf9ozUSMRjcdQLOcT6/6Z0sJWfoLasWL7NDb5VbMcZvtUZKwbNpKTAa/8LAofsrT78//z2L9eQVnIrtDCFGpMsLPnZvAIah3NEW1dsbIUxPutv/erHGCO2E5MnbfW6lCNbrq79MhHYaRwnGMJsqCnVdeU2SR0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1646907062; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=Op9NqcwqZ9p1zf4uw9EIohc6TNUBNqir9l5zE8kobUY=; b=JEu7XP2UqZrlGHEm6cA5CtsTKg561xaipnozJ727hpkw7FRjy0yl2/OqIv1CttRo+YZJOE7pDowdpSqhSCBl60UQG3/t7HHXHXp+fVORXS7ewc5vnOTrs5EzKgoa1BQjzfvCGyULmaF1AHZKmpvNCTYwYcgLkOkRq5vX1gdmZ6c= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 1646907062569469.42294734461757; Thu, 10 Mar 2022 02:11:02 -0800 (PST) Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-73-UsR4lbAzNZyVS8jE0buKgg-1; Thu, 10 Mar 2022 05:10:25 -0500 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 0D4A1296A62E; Thu, 10 Mar 2022 10:10:23 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id EB775463EB2; Thu, 10 Mar 2022 10:10:22 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 9F5E91953573; Thu, 10 Mar 2022 10:10:22 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 3B3BE195357C for ; Thu, 10 Mar 2022 10:10:20 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id DA9B373DA1; Thu, 10 Mar 2022 10:10:19 +0000 (UTC) Received: from domokun.gsslab.fab.redhat.com (gx270-2.gsslab.fab.redhat.com [10.33.8.41]) by smtp.corp.redhat.com (Postfix) with ESMTP id 34B407219B; Thu, 10 Mar 2022 10:10:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1646907062; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=Op9NqcwqZ9p1zf4uw9EIohc6TNUBNqir9l5zE8kobUY=; b=cq0tiZoCSVDFnqYAG1JC7vzbOmAtyaCg06p0iTiw6lQyPX8BrOypRpDPic3dUrRJKIKXrb zJNwuY4gNCGKnN4fGK79YYHZ0LT+sKf5qo0fN3zLv9xSAMcF5rcF5q33wY6NOjIXQVTLzc XCy4xSOAYfLZ8/fNFhMBqEwjQMNCHzs= X-MC-Unique: UsR4lbAzNZyVS8jE0buKgg-1 X-Original-To: libvir-list@listman.corp.redhat.com From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH 2/2] conf: use a hash table for storing nwfilter object list Date: Thu, 10 Mar 2022 10:10:04 +0000 Message-Id: <20220310101004.1914477-3-berrange@redhat.com> In-Reply-To: <20220310101004.1914477-1-berrange@redhat.com> References: <20220310101004.1914477-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 2.85 on 10.11.54.10 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1646907086672100001 The current use of an array for nwfilter objects requires the caller to iterate over all elements to find a filter, and also requires locking each filter. Switching to a pair of hash tables enables O(1) lookups both by name and uuid, with no locking required. Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: J=C3=A1n Tomko --- src/conf/virnwfilterobj.c | 264 +++++++++++++++++-------- src/nwfilter/nwfilter_gentech_driver.c | 8 +- 2 files changed, 179 insertions(+), 93 deletions(-) diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 6bbdf6e6fa..808283e4ed 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -43,10 +43,14 @@ struct _virNWFilterObj { }; =20 struct _virNWFilterObjList { - size_t count; - virNWFilterObj **objs; -}; + /* uuid string -> virNWFilterObj mapping + * for O(1), lookup-by-uuid */ + GHashTable *objs; =20 + /* name -> virNWFilterObj mapping for O(1), + * lookup-by-name */ + GHashTable *objsName; +}; =20 static virNWFilterObj * virNWFilterObjNew(void) @@ -106,10 +110,8 @@ virNWFilterObjFree(virNWFilterObj *obj) void virNWFilterObjListFree(virNWFilterObjList *nwfilters) { - size_t i; - for (i =3D 0; i < nwfilters->count; i++) - virNWFilterObjFree(nwfilters->objs[i]); - g_free(nwfilters->objs); + g_hash_table_unref(nwfilters->objs); + g_hash_table_unref(nwfilters->objsName); g_free(nwfilters); } =20 @@ -117,7 +119,17 @@ virNWFilterObjListFree(virNWFilterObjList *nwfilters) virNWFilterObjList * virNWFilterObjListNew(void) { - return g_new0(virNWFilterObjList, 1); + virNWFilterObjList *nwfilters =3D g_new0(virNWFilterObjList, 1); + + /* virNWFilterObj is not ref counted, so we rely fact that + * an instance will always exist in both hash tables, or + * neither hash table. Thus we only need to have a destroy + * callback for one of the two hash tables. + */ + nwfilters->objs =3D virHashNew((GDestroyNotify)virNWFilterObjFree); + nwfilters->objsName =3D virHashNew(NULL); + + return nwfilters; } =20 =20 @@ -125,21 +137,14 @@ void virNWFilterObjListRemove(virNWFilterObjList *nwfilters, virNWFilterObj *obj) { - size_t i; + char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 - virNWFilterObjUnlock(obj); + virUUIDFormat(obj->def->uuid, uuidstr); =20 - for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObjLock(nwfilters->objs[i]); - if (nwfilters->objs[i] =3D=3D obj) { - virNWFilterObjUnlock(nwfilters->objs[i]); - virNWFilterObjFree(nwfilters->objs[i]); + virNWFilterObjUnlock(obj); =20 - VIR_DELETE_ELEMENT(nwfilters->objs, i, nwfilters->count); - break; - } - virNWFilterObjUnlock(nwfilters->objs[i]); - } + g_hash_table_remove(nwfilters->objsName, obj->def->name); + g_hash_table_remove(nwfilters->objs, uuidstr); } =20 =20 @@ -147,20 +152,16 @@ virNWFilterObj * virNWFilterObjListFindByUUID(virNWFilterObjList *nwfilters, const unsigned char *uuid) { - size_t i; + char uuidstr[VIR_UUID_STRING_BUFLEN]; virNWFilterObj *obj; - virNWFilterDef *def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; + virUUIDFormat(uuid, uuidstr); + + obj =3D g_hash_table_lookup(nwfilters->objs, uuidstr); + if (obj) virNWFilterObjLock(obj); - def =3D obj->def; - if (!memcmp(def->uuid, uuid, VIR_UUID_BUFLEN)) - return obj; - virNWFilterObjUnlock(obj); - } =20 - return NULL; + return obj; } =20 =20 @@ -168,20 +169,13 @@ virNWFilterObj * virNWFilterObjListFindByName(virNWFilterObjList *nwfilters, const char *name) { - size_t i; virNWFilterObj *obj; - virNWFilterDef *def; =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; + obj =3D g_hash_table_lookup(nwfilters->objsName, name); + if (obj) virNWFilterObjLock(obj); - def =3D obj->def; - if (STREQ_NULLABLE(def->name, name)) - return obj; - virNWFilterObjUnlock(obj); - } =20 - return NULL; + return obj; } =20 =20 @@ -308,6 +302,7 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfilte= rs, { virNWFilterObj *obj; virNWFilterDef *objdef; + char uuidstr[VIR_UUID_STRING_BUFLEN]; =20 if ((obj =3D virNWFilterObjListFindByUUID(nwfilters, def->uuid))) { objdef =3D obj->def; @@ -323,8 +318,6 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfilte= rs, virNWFilterObjUnlock(obj); } else { if ((obj =3D virNWFilterObjListFindByName(nwfilters, def->name))) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - objdef =3D obj->def; virUUIDFormat(objdef->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, @@ -368,7 +361,10 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfilt= ers, if (!(obj =3D virNWFilterObjNew())) return NULL; =20 - VIR_APPEND_ELEMENT_COPY(nwfilters->objs, nwfilters->count, obj); + virUUIDFormat(def->uuid, uuidstr); + + g_hash_table_insert(nwfilters->objs, g_strdup(uuidstr), obj); + g_hash_table_insert(nwfilters->objsName, g_strdup(def->name), obj); =20 obj->def =3D def; =20 @@ -376,26 +372,69 @@ virNWFilterObjListAssignDef(virNWFilterObjList *nwfil= ters, } =20 =20 +struct virNWFilterObjListData { + virNWFilterObjListFilter filter; + virConnectPtr conn; + int count; +}; + + +static void +virNWFilterObjListCount(void *payload, + void *key G_GNUC_UNUSED, + void *opaque) +{ + virNWFilterObj *obj =3D payload; + struct virNWFilterObjListData *data =3D opaque; + g_auto(virLockGuard) lock =3D virObjectLockGuard(obj); + + if (data->filter(data->conn, obj->def)) + data->count++; +} + + int virNWFilterObjListNumOfNWFilters(virNWFilterObjList *nwfilters, virConnectPtr conn, virNWFilterObjListFilter filter) { - size_t i; - int nfilters =3D 0; - - for (i =3D 0; i < nwfilters->count; i++) { - virNWFilterObj *obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); - if (!filter || filter(conn, obj->def)) - nfilters++; - virNWFilterObjUnlock(obj); - } + struct virNWFilterObjListData data =3D { filter, conn, 0 }; =20 - return nfilters; + g_hash_table_foreach(nwfilters->objs, + virNWFilterObjListCount, + &data); + return data.count; } =20 =20 +struct virNWFilterNameData { + virNWFilterObjListFilter filter; + virConnectPtr conn; + int numnames; + int maxnames; + char **const names; +}; + + +static void +virNWFilterObjListCopyNames(void *payload, + void *key G_GNUC_UNUSED, + void *opaque) +{ + virNWFilterObj *obj =3D payload; + struct virNWFilterNameData *data =3D opaque; + g_auto(virLockGuard) lock =3D virObjectLockGuard(obj); + + if (data->filter && + !data->filter(data->conn, obj->def)) + return; + + if (data->numnames < data->maxnames) { + data->names[data->numnames] =3D g_strdup(obj->def->name); + data->numnames++; + } +} + int virNWFilterObjListGetNames(virNWFilterObjList *nwfilters, virConnectPtr conn, @@ -403,22 +442,80 @@ virNWFilterObjListGetNames(virNWFilterObjList *nwfilt= ers, char **const names, int maxnames) { - int nnames =3D 0; - size_t i; - virNWFilterDef *def; + struct virNWFilterNameData data =3D + { filter, conn, 0, maxnames, names }; =20 - for (i =3D 0; i < nwfilters->count && nnames < maxnames; i++) { - virNWFilterObj *obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); - def =3D obj->def; - if (!filter || filter(conn, def)) { - names[nnames] =3D g_strdup(def->name); - nnames++; + g_hash_table_foreach(nwfilters->objs, + virNWFilterObjListCopyNames, + &data); + + return data.numnames; +} + + +struct virNWFilterListData { + virNWFilterObj **filters; + size_t nfilters; +}; + + +static void +virNWFilterObjListCollectIterator(void *payload, + void *key G_GNUC_UNUSED, + void *opaque) +{ + struct virNWFilterListData *data =3D opaque; + virNWFilterObj *obj =3D payload; + + virNWFilterObjLock(obj); + data->filters[data->nfilters++] =3D payload; +} + + +static void +virNWFilterObjListFilterApply(virNWFilterObj ***list, + size_t *nfilters, + virConnectPtr conn, + virNWFilterObjListFilter filter) +{ + size_t i =3D 0; + + while (i < *nfilters) { + virNWFilterObj *obj =3D (*list)[i]; + + if (filter && !filter(conn, obj->def)) { + VIR_DELETE_ELEMENT(*list, i, *nfilters); + virNWFilterObjUnlock(obj); + continue; } - virNWFilterObjUnlock(obj); + + i++; } +} + + +static int +virNWFilterObjListCollect(virNWFilterObjList *nwfilters, + virConnectPtr conn, + virNWFilterObj ***filters, + size_t *nfilters, + virNWFilterObjListFilter filter) +{ + struct virNWFilterListData data =3D { NULL, 0 }; + + data.filters =3D g_new0(virNWFilterObj *, + g_hash_table_size(nwfilters->objs)); + + g_hash_table_foreach(nwfilters->objs, + virNWFilterObjListCollectIterator, + &data); =20 - return nnames; + virNWFilterObjListFilterApply(&data.filters, &data.nfilters, conn, fil= ter); + + *nfilters =3D data.nfilters; + *filters =3D data.filters; + + return 0; } =20 =20 @@ -429,32 +526,26 @@ virNWFilterObjListExport(virConnectPtr conn, virNWFilterObjListFilter filter) { virNWFilterPtr *tmp_filters =3D NULL; - int nfilters =3D 0; - virNWFilterPtr nwfilter =3D NULL; - virNWFilterObj *obj =3D NULL; - virNWFilterDef *def; + virNWFilterObj **objs =3D NULL; + size_t nfilters =3D 0; size_t i; int ret =3D -1; =20 + if (virNWFilterObjListCollect(nwfilters, conn, &objs, &nfilters, filte= r) < 0) + return -1; + if (!filters) { - ret =3D nwfilters->count; + ret =3D nfilters; goto cleanup; } =20 - tmp_filters =3D g_new0(virNWFilterPtr, nwfilters->count + 1); + tmp_filters =3D g_new0(virNWFilterPtr, nfilters + 1); =20 - for (i =3D 0; i < nwfilters->count; i++) { - obj =3D nwfilters->objs[i]; - virNWFilterObjLock(obj); - def =3D obj->def; - if (!filter || filter(conn, def)) { - if (!(nwfilter =3D virGetNWFilter(conn, def->name, def->uuid))= ) { - virNWFilterObjUnlock(obj); - goto cleanup; - } - tmp_filters[nfilters++] =3D nwfilter; - } - virNWFilterObjUnlock(obj); + for (i =3D 0; i < nfilters; i++) { + tmp_filters[i] =3D virGetNWFilter(conn, objs[i]->def->name, objs[i= ]->def->uuid); + + if (!tmp_filters[i]) + goto cleanup; } =20 *filters =3D g_steal_pointer(&tmp_filters); @@ -462,11 +553,12 @@ virNWFilterObjListExport(virConnectPtr conn, =20 cleanup: if (tmp_filters) { - for (i =3D 0; i < nfilters; i ++) + for (i =3D 0; i < nfilters; i++) virObjectUnref(tmp_filters[i]); } VIR_FREE(tmp_filters); - + for (i =3D 0; i < nfilters; i++) + virNWFilterObjUnlock(objs[i]); return ret; } =20 diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter= _gentech_driver.c index 9f4a755252..c609405ac0 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -59,13 +59,7 @@ static virNWFilterTechDriver *filter_tech_drivers[] =3D { * Serializes instantiation of filters. * * When instantiating a filter, we need to resolve references - * to other filters and acquire locks on them. The act of - * looking up a filter requires traversing an array, locking - * each filter in turn until we find the one we want. - * - * The mere act of finding a filter by name, while holding - * a lock on another filter, introduces deadlocks due to - * varying lock ordering. + * to other filters and acquire locks on them. * * We retain a lock on the referenced filter once found. * The order in which the locks are acquired depends on --=20 2.35.1