From nobody Thu May 2 16:50:33 2024 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 1510841192746870.6814041314773; Thu, 16 Nov 2017 06:06:32 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6AF6A76552; Thu, 16 Nov 2017 14:06:30 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C3D635C1A3; Thu, 16 Nov 2017 14:06:29 +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 899473D380; Thu, 16 Nov 2017 14:06:28 +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 vAGE6Rph017543 for ; Thu, 16 Nov 2017 09:06:27 -0500 Received: by smtp.corp.redhat.com (Postfix) id 6A0CE6062D; Thu, 16 Nov 2017 14:06:27 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-167.phx2.redhat.com [10.3.117.167]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2DC7D60618 for ; Thu, 16 Nov 2017 14:06:25 +0000 (UTC) From: John Ferlan To: libvir-list@redhat.com Date: Thu, 16 Nov 2017 09:06:23 -0500 Message-Id: <20171116140623.13377-1-jferlan@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2] storage: Resolve storage driver crash 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 16 Nov 2017 14:06:31 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" Resolve a storage driver crash as a result of a long running storageVolCreateXML when the virStorageVolPoolRefreshThread is run as a result of when a storageVolUpload completed and ran the virStoragePoolObjClearVols without checking if the creation code was currently processing a buildVol after incrementing the driver->asyncjob count. The refreshThread will now check the pool asyncjob count before attempting to pursue the pool refresh. Adjust the documentation to describe the condition. Crash from valgrind is as follows (with a bit of editing): =3D=3D21309=3D=3D Invalid read of size 8 =3D=3D21309=3D=3D at 0x153E47AF: storageBackendUpdateVolTargetInfo =3D=3D21309=3D=3D by 0x153E4C30: virStorageBackendUpdateVolInfo =3D=3D21309=3D=3D by 0x153E52DE: virStorageBackendVolRefreshLocal =3D=3D21309=3D=3D by 0x153DE29E: storageVolCreateXML =3D=3D21309=3D=3D by 0x562035B: virStorageVolCreateXML =3D=3D21309=3D=3D by 0x147366: remoteDispatchStorageVolCreateXML ... =3D=3D21309=3D=3D Address 0x2590a720 is 64 bytes inside a block of size 33= 6 free'd =3D=3D21309=3D=3D at 0x4C2F2BB: free =3D=3D21309=3D=3D by 0x54CB9FA: virFree =3D=3D21309=3D=3D by 0x55BC800: virStorageVolDefFree =3D=3D21309=3D=3D by 0x55BF1D8: virStoragePoolObjClearVols =3D=3D21309=3D=3D by 0x153D967E: virStorageVolPoolRefreshThread ... =3D=3D21309=3D=3D Block was alloc'd at =3D=3D21309=3D=3D at 0x4C300A5: calloc =3D=3D21309=3D=3D by 0x54CB483: virAlloc =3D=3D21309=3D=3D by 0x55BDC1F: virStorageVolDefParseXML =3D=3D21309=3D=3D by 0x55BDC1F: virStorageVolDefParseNode =3D=3D21309=3D=3D by 0x55BE5A4: virStorageVolDefParse =3D=3D21309=3D=3D by 0x153DDFF1: storageVolCreateXML =3D=3D21309=3D=3D by 0x562035B: virStorageVolCreateXML =3D=3D21309=3D=3D by 0x147366: remoteDispatchStorageVolCreateXML ... Signed-off-by: John Ferlan --- v1: https://www.redhat.com/archives/libvir-list/2017-November/msg00198.html Changes since v1: - From review, remove the retry if Asyncjobs > 0 logic and replace with a VIR_DEBUG indicating not doing refresh due to Asyncjob running and just goto cleanup. - Adjust the virStorageVolUpload docs to note that an attempt will be made to refresh the pool. - Drop patch 2 which added the Asyncjobs > 0 check to the SCSI pool refresh thread. Since a SCSI pool doesn't support buildVol, the asyncjob count cannot be anything other than zero, so it's pointless. src/libvirt-storage.c | 3 ++- src/storage/storage_driver.c | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c index 9ab0fe28ea..e4646cb80f 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -1632,7 +1632,8 @@ virStorageVolDownload(virStorageVolPtr vol, * another active stream is writing to the storage volume. * * When the data stream is closed whether the upload is successful - * or not the target storage pool will be refreshed to reflect pool + * or not an attempt will be made to refresh the target storage pool + * if an asynchronous build is not running in order to reflect pool * and volume changes as a result of the upload. Depending on * the target volume storage backend and the source stream type * for a successful upload, the target volume may take on the diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 2b7a299706..d209f5afb8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2267,6 +2267,13 @@ virStorageVolPoolRefreshThread(void *opaque) goto cleanup; def =3D virStoragePoolObjGetDef(obj); =20 + /* If some thread is building a new volume in the pool, then we cannot + * clear out all vols and refresh the pool. So we'll just pass. */ + if (virStoragePoolObjGetAsyncjobs(obj) > 0) { + VIR_DEBUG("Asyncjob in process, cannot refresh storage pool"); + goto cleanup; + } + if (!(backend =3D virStorageBackendForType(def->type))) goto cleanup; =20 --=20 2.13.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list