From nobody Wed Nov 27 04:34:29 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; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1558708685; cv=none; d=zoho.com; s=zohoarc; b=VF7gjnwEaeoL+zHNvkmDYkMB8rxWJJ7YXbtIX/4J8SG9Mmwk4DCOWPRWL9mGeNb5vb9DfiWDv+F8jB9xjFXPvHKFyDOYof4qkyqUNAhzVfXEqa7hb4sHqspwLEUxfdRq+YXLOB3ift2YsS6qIkWEeuJhs9T/KHtclWdiDeAoYh8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1558708685; 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:ARC-Authentication-Results; bh=/7ToHjMw5zDOtpmsMk7Y5FvG1X8Bw1S1VWjdSonydZM=; b=DgsitIXl0bJol/snofo6p/qXvMzKUiWMZUKdlL91XtA8z7iA6PnUGYtR6omBsD4Bqy3pndrjdyXkyu5rA6UP2UUrCrG3eXovv1dfKFRJiYWnOUv7XsRyoGMdv6l347fhm/4QLYVe42bHTuN8yGj8+Lzix2lx+PaGKT/CDhhAbe0= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1558708685134616.3006588876167; Fri, 24 May 2019 07:38:05 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 54D36B0CFB; Fri, 24 May 2019 14:38:03 +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 1C8D25B2EB; Fri, 24 May 2019 14:38:02 +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 D32D01806B11; Fri, 24 May 2019 14:38:01 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x4OEaKEb011427 for ; Fri, 24 May 2019 10:36:20 -0400 Received: by smtp.corp.redhat.com (Postfix) id 17FB769287; Fri, 24 May 2019 14:36:20 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9396D692A1 for ; Fri, 24 May 2019 14:36:17 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 24 May 2019 16:35:47 +0200 Message-Id: <1ffb6e8cd780a57122d1633e68df282e5056673d.1558707820.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v1 11/11] storage: Drop and reacquire pool obj lock in some backends 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: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 24 May 2019 14:38:04 +0000 (UTC) Content-Type: text/plain; charset="utf-8" https://bugzilla.redhat.com/show_bug.cgi?id=3D1711789 Starting up or building some types of pools may take a very long time (e.g. a misconfigured NFS). Holding the pool object locked throughout the whole time hurts concurrency, e.g. if there's another thread that is listing all the pools. Signed-off-by: Michal Privoznik Reviewed-by: J=C3=A1n Tomko --- src/storage/storage_backend_disk.c | 11 ++++++++++- src/storage/storage_backend_fs.c | 13 +++++++++++-- src/storage/storage_backend_logical.c | 15 ++++++++++----- src/storage/storage_backend_vstorage.c | 9 ++++++++- src/storage/storage_backend_zfs.c | 7 ++++++- 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backe= nd_disk.c index 9b94d26cf9..f58b7b294c 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -461,7 +461,10 @@ virStorageBackendDiskStartPool(virStoragePoolObjPtr po= ol) const char *format; const char *path =3D def->source.devices[0].path; =20 + /* This can take a significant amount of time. */ + virObjectUnlock(pool); virWaitForDevices(); + virObjectLock(pool); =20 if (!virFileExists(path)) { virReportError(VIR_ERR_INVALID_ARG, @@ -490,6 +493,7 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr poo= l, int format =3D def->source.format; const char *fmt; VIR_AUTOPTR(virCommand) cmd =3D NULL; + int ret =3D -1; =20 virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, -1); @@ -523,7 +527,12 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr po= ol, "--script", fmt, NULL); - return virCommandRun(cmd, NULL); + + virObjectUnlock(pool); + ret =3D virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } =20 =20 diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend= _fs.c index ae4e9a03a3..1257419760 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -318,7 +318,14 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr = pool) return -1; =20 cmd =3D virStorageBackendFileSystemMountCmd(MOUNT, def, src); - return virCommandRun(cmd, NULL); + + /* Mounting a shared FS might take a long time. Don't hold + * the pool locked meanwhile. */ + virObjectUnlock(pool); + rc =3D virCommandRun(cmd, NULL); + virObjectLock(pool); + + return rc; } =20 =20 @@ -457,13 +464,14 @@ virStorageBackendMakeFileSystem(virStoragePoolObjPtr = pool, virReportError(VIR_ERR_OPERATION_INVALID, _("No source device specified when formatting pool = '%s'"), def->name); - goto error; + return -1; } =20 device =3D def->source.devices[0].path; format =3D virStoragePoolFormatFileSystemTypeToString(def->source.form= at); VIR_DEBUG("source device: '%s' format: '%s'", device, format); =20 + virObjectUnlock(pool); if (!virFileExists(device)) { virReportError(VIR_ERR_OPERATION_INVALID, _("Source device does not exist when formatting poo= l '%s'"), @@ -482,6 +490,7 @@ virStorageBackendMakeFileSystem(virStoragePoolObjPtr po= ol, ret =3D virStorageBackendExecuteMKFS(device, format); =20 error: + virObjectLock(pool); return ret; } =20 diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_ba= ckend_logical.c index 83b5f27151..603a3f9a42 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -50,9 +50,15 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr p= ool, { virStoragePoolDefPtr def =3D virStoragePoolObjGetDef(pool); VIR_AUTOPTR(virCommand) cmd =3D NULL; + int ret; =20 cmd =3D virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); - return virCommandRun(cmd, NULL); + + virObjectUnlock(pool); + ret =3D virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } =20 =20 @@ -723,11 +729,10 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPt= r pool, virCommandAddArg(vgcmd, path); } =20 + virObjectUnlock(pool); /* Now create the volume group itself */ - if (virCommandRun(vgcmd, NULL) < 0) - goto cleanup; - - ret =3D 0; + ret =3D virCommandRun(vgcmd, NULL); + virObjectLock(pool); =20 cleanup: /* On any failure, run through the devices that had pvcreate run in diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_b= ackend_vstorage.c index d446aa2726..cec21dccbf 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -42,6 +42,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) VIR_AUTOFREE(char *) usr_name =3D NULL; VIR_AUTOFREE(char *) mode =3D NULL; VIR_AUTOPTR(virCommand) cmd =3D NULL; + int ret; =20 /* Check the permissions */ if (def->target.perms.mode =3D=3D (mode_t)-1) @@ -69,7 +70,13 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) "-g", grp_name, "-u", usr_name, NULL); =20 - return virCommandRun(cmd, NULL); + /* Mounting a shared FS might take a long time. Don't hold + * the pool locked meanwhile. */ + virObjectUnlock(pool); + ret =3D virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } =20 =20 diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backen= d_zfs.c index 826a95538e..d172a5a165 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -385,6 +385,7 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, virStoragePoolDefPtr def =3D virStoragePoolObjGetDef(pool); size_t i; VIR_AUTOPTR(virCommand) cmd =3D NULL; + int ret =3D -1; =20 virCheckFlags(0, -1); =20 @@ -400,7 +401,11 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr poo= l, for (i =3D 0; i < def->source.ndevice; i++) virCommandAddArg(cmd, def->source.devices[i].path); =20 - return virCommandRun(cmd, NULL); + virObjectUnlock(pool); + ret =3D virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } =20 static int --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list