https://bugzilla.redhat.com/show_bug.cgi?id=1711789 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 <mprivozn@xxxxxxxxxx> --- 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_backend_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 pool) const char *format; const char *path = def->source.devices[0].path; + /* This can take a significant amount of time. */ + virObjectUnlock(pool); virWaitForDevices(); + virObjectLock(pool); if (!virFileExists(path)) { virReportError(VIR_ERR_INVALID_ARG, @@ -490,6 +493,7 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr pool, int format = def->source.format; const char *fmt; VIR_AUTOPTR(virCommand) cmd = NULL; + int ret = -1; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, -1); @@ -523,7 +527,12 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr pool, "--script", fmt, NULL); - return virCommandRun(cmd, NULL); + + virObjectUnlock(pool); + ret = virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } 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; cmd = 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 = virCommandRun(cmd, NULL); + virObjectLock(pool); + + return rc; } @@ -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; } device = def->source.devices[0].path; format = virStoragePoolFormatFileSystemTypeToString(def->source.format); VIR_DEBUG("source device: '%s' format: '%s'", device, format); + virObjectUnlock(pool); if (!virFileExists(device)) { virReportError(VIR_ERR_OPERATION_INVALID, _("Source device does not exist when formatting pool '%s'"), @@ -482,6 +490,7 @@ virStorageBackendMakeFileSystem(virStoragePoolObjPtr pool, ret = virStorageBackendExecuteMKFS(device, format); error: + virObjectLock(pool); return ret; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_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 pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); VIR_AUTOPTR(virCommand) cmd = NULL; + int ret; cmd = virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); - return virCommandRun(cmd, NULL); + + virObjectUnlock(pool); + ret = virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } @@ -723,11 +729,10 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, virCommandAddArg(vgcmd, path); } + virObjectUnlock(pool); /* Now create the volume group itself */ - if (virCommandRun(vgcmd, NULL) < 0) - goto cleanup; - - ret = 0; + ret = virCommandRun(vgcmd, NULL); + virObjectLock(pool); 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_backend_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 = NULL; VIR_AUTOFREE(char *) mode = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; + int ret; /* Check the permissions */ if (def->target.perms.mode == (mode_t)-1) @@ -69,7 +70,13 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) "-g", grp_name, "-u", usr_name, NULL); - return virCommandRun(cmd, NULL); + /* Mounting a shared FS might take a long time. Don't hold + * the pool locked meanwhile. */ + virObjectUnlock(pool); + ret = virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_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 = virStoragePoolObjGetDef(pool); size_t i; VIR_AUTOPTR(virCommand) cmd = NULL; + int ret = -1; virCheckFlags(0, -1); @@ -400,7 +401,11 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, for (i = 0; i < def->source.ndevice; i++) virCommandAddArg(cmd, def->source.devices[i].path); - return virCommandRun(cmd, NULL); + virObjectUnlock(pool); + ret = virCommandRun(cmd, NULL); + virObjectLock(pool); + + return ret; } static int -- 2.21.0 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
