9:35 a.m.
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(a)redhat.com>
---
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