From nobody Thu May 15 22:35:30 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1500559719862143.2741132651205; Thu, 20 Jul 2017 07:08:39 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 98784C056827; Thu, 20 Jul 2017 14:08:33 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 69BB282712; Thu, 20 Jul 2017 14:08:33 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 209411853E31; Thu, 20 Jul 2017 14:08:33 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v6KE8J4Q023829 for ; Thu, 20 Jul 2017 10:08:19 -0400 Received: by smtp.corp.redhat.com (Postfix) id 219EE17F23; Thu, 20 Jul 2017 14:08:19 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-46.phx2.redhat.com [10.3.117.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id DA0D317F3A for ; Thu, 20 Jul 2017 14:08:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 98784C056827 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=libvir-list-bounces@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 98784C056827 From: John Ferlan To: libvir-list@redhat.com Date: Thu, 20 Jul 2017 10:08:12 -0400 Message-Id: <20170720140815.16411-2-jferlan@redhat.com> In-Reply-To: <20170720140815.16411-1-jferlan@redhat.com> References: <20170720140815.16411-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v6 1/4] nodedev: Alter node device deletion logic X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 20 Jul 2017 14:08:34 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Alter the node device deletion logic to make use of the parent field from the obj->def rather than call virNodeDeviceObjListGetParentHost. As it turns out the saved @def won't have parent_wwnn/wwpn or parent_fabric_wwn, so the only logical path would be to call virNodeDeviceObjListGetParentHostByParent which we can accomplish directly via virNodeDeviceObjListFindByName. Signed-off-by: John Ferlan --- src/node_device/node_device_driver.c | 26 +++++++++++++++++++------- src/test/test_driver.c | 26 +++++++++++++------------- 2 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_de= vice_driver.c index 0a19908..f56ff34 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -594,8 +594,9 @@ nodeDeviceDestroy(virNodeDevicePtr device) int ret =3D -1; virNodeDeviceObjPtr obj =3D NULL; virNodeDeviceDefPtr def; + char *parent =3D NULL; char *wwnn =3D NULL, *wwpn =3D NULL; - int parent_host =3D -1; + unsigned int parent_host; =20 if (!(obj =3D nodeDeviceObjFindByName(device->name))) return -1; @@ -609,13 +610,23 @@ nodeDeviceDestroy(virNodeDevicePtr device) if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) < 0) goto cleanup; =20 - /* virNodeDeviceGetParentHost will cause the device object's lock - * to be taken, so grab the object def which will have the various - * fields used to search (name, parent, parent_wwnn, parent_wwpn, - * or parent_fabric_wwn) and drop the object lock. */ + /* Because we're about to release the lock and thus run into a race + * possibility (however improbable) with a udevAddOneDevice change + * event which would essentially free the existing @def (obj->def) and + * replace it with something new, we need to grab the parent field + * and then find the parent obj in order to manage the vport */ + if (VIR_STRDUP(parent, def->parent) < 0) + goto cleanup; + virNodeDeviceObjEndAPI(&obj); - if ((parent_host =3D virNodeDeviceObjListGetParentHost(driver->devs, d= ef, - EXISTING_DEVICE))= < 0) + + if (!(obj =3D virNodeDeviceObjListFindByName(driver->devs, parent))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find parent '%s' definition"), parent); + goto cleanup; + } + + if (virSCSIHostGetNumber(parent, &parent_host) < 0) goto cleanup; =20 if (virVHBAManageVport(parent_host, wwpn, wwnn, VPORT_DELETE) < 0) @@ -626,6 +637,7 @@ nodeDeviceDestroy(virNodeDevicePtr device) cleanup: nodeDeviceUnlock(); virNodeDeviceObjEndAPI(&obj); + VIR_FREE(parent); VIR_FREE(wwnn); VIR_FREE(wwpn); return ret; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 83ab9cc..076b17a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5618,8 +5618,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) int ret =3D 0; testDriverPtr driver =3D dev->conn->privateData; virNodeDeviceObjPtr obj =3D NULL; + virNodeDeviceObjPtr parentobj =3D NULL; virNodeDeviceDefPtr def; - char *parent_name =3D NULL, *wwnn =3D NULL, *wwpn =3D NULL; + char *wwnn =3D NULL, *wwpn =3D NULL; virObjectEventPtr event =3D NULL; =20 if (!(obj =3D testNodeDeviceObjFindByName(driver, dev->name))) @@ -5629,22 +5630,22 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) if (virNodeDeviceGetWWNs(def, &wwnn, &wwpn) =3D=3D -1) goto cleanup; =20 - if (VIR_STRDUP(parent_name, def->parent) < 0) - goto cleanup; - - /* virNodeDeviceGetParentHost will cause the device object's lock to be - * taken, so we have to dup the parent's name and drop the lock - * before calling it. We don't need the reference to the object - * any more once we have the parent's name. */ + /* Unlike the real code we cannot run into the udevAddOneDevice race + * which would replace obj->def, so no need to save off the parent, + * but do need to drop the @obj lock so that the FindByName code doesn= 't + * deadlock on ourselves */ virObjectUnlock(obj); =20 - /* We do this just for basic validation, but also avoid finding a - * vport capable HBA if for some reason our vHBA doesn't exist */ - if (virNodeDeviceObjListGetParentHost(driver->devs, def, - EXISTING_DEVICE) < 0) { + /* We do this just for basic validation and throw away the parentobj + * since there's no vport_delete to be run */ + if (!(parentobj =3D virNodeDeviceObjListFindByName(driver->devs, + def->parent))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find parent '%s' definition"), def->paren= t); virObjectLock(obj); goto cleanup; } + virNodeDeviceObjEndAPI(&parentobj); =20 event =3D virNodeDeviceEventLifecycleNew(dev->name, VIR_NODE_DEVICE_EVENT_DELETED, @@ -5658,7 +5659,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) cleanup: virNodeDeviceObjEndAPI(&obj); testObjectEventQueue(driver, event); - VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); return ret; --=20 2.9.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list