Implement non-pool-blocking volume allocation for FS volumes.
We setup the volume object, then drop the pool lock, and start the
actual storage allocation. Certain pool operations will be blocked
(start, stop, destroy, refresh), but the basic things like volume
listings will still work.
This also allows storage allocation progress reporting: the API user can
watch the volume object in a separate thread, polling it's 'info' for
update to date capacity/allocation reporting.
- Cole
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 350a931..b6ac8e0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -46,6 +46,7 @@ virGetDomain;
virGetNetwork;
virGetStoragePool;
virGetStorageVol;
+virUnrefStorageVol;
virGetNodeDevice;
virUnrefDomain;
virUnrefConnect;
diff --git a/src/storage_backend.h b/src/storage_backend.h
index 7fab384..c9c1e35 100644
--- a/src/storage_backend.h
+++ b/src/storage_backend.h
@@ -34,6 +34,7 @@ typedef int (*virStorageBackendRefreshPool)(virConnectPtr conn, virStoragePoolOb
typedef int (*virStorageBackendStopPool)(virConnectPtr conn, virStoragePoolObjPtr pool);
typedef int (*virStorageBackendDeletePool)(virConnectPtr conn, virStoragePoolObjPtr pool, unsigned int flags);
+typedef int (*virStorageBackendBuildVol)(virConnectPtr conn, virStorageVolDefPtr vol);
typedef int (*virStorageBackendCreateVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol);
typedef int (*virStorageBackendRefreshVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol);
typedef int (*virStorageBackendDeleteVol)(virConnectPtr conn, virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags);
@@ -52,6 +53,7 @@ struct _virStorageBackend {
virStorageBackendStopPool stopPool;
virStorageBackendDeletePool deletePool;
+ virStorageBackendBuildVol buildVol;
virStorageBackendCreateVol createVol;
virStorageBackendRefreshVol refreshVol;
virStorageBackendDeleteVol deleteVol;
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 5000f43..7a1bba8 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -982,16 +982,16 @@ virStorageBackendFileSystemDelete(virConnectPtr conn,
/**
- * Allocate a new file as a volume. This is either done directly
- * for raw/sparse files, or by calling qemu-img/qcow-create for
- * special kinds of files
+ * Set up a volume definition to be added to a pool's volume list, but
+ * don't do any file creation or allocation. By separating the two processes,
+ * we allow allocation progress reporting (by polling the volume's 'info'
+ * function), and can drop the parent pool lock during the (slow) allocation.
*/
static int
virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virStoragePoolObjPtr pool,
virStorageVolDefPtr vol)
{
- int fd;
if (VIR_ALLOC_N(vol->target.path, strlen(pool->def->target.path) +
1 + strlen(vol->name) + 1) < 0) {
@@ -1008,6 +1008,20 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
return -1;
}
+ return 0;
+}
+
+/**
+ * Allocate a new file as a volume. This is either done directly
+ * for raw/sparse files, or by calling qemu-img/qcow-create for
+ * special kinds of files
+ */
+static int
+virStorageBackendFileSystemVolBuild(virConnectPtr conn,
+ virStorageVolDefPtr vol)
+{
+ int fd;
+
if (vol->target.format == VIR_STORAGE_VOL_FILE_RAW) {
if ((fd = open(vol->target.path, O_RDWR | O_CREAT | O_EXCL,
vol->target.perms.mode)) < 0) {
@@ -1017,6 +1031,16 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
return -1;
}
+ /* Seek to the final size, so the capacity is available upfront
+ * for progress reporting */
+ if (ftruncate(fd, vol->capacity) < 0) {
+ virReportSystemError(conn, errno,
+ _("cannot extend file '%s'"),
+ vol->target.path);
+ close(fd);
+ return -1;
+ }
+
/* Pre-allocate any data if requested */
/* XXX slooooooooooooooooow on non-extents-based file systems */
/* FIXME: Add in progress bars & bg thread if progress bar requested */
@@ -1039,7 +1063,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, r,
_("cannot fill file '%s'"),
vol->target.path);
- unlink(vol->target.path);
close(fd);
return -1;
}
@@ -1052,22 +1075,12 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, r,
_("cannot fill file '%s'"),
vol->target.path);
- unlink(vol->target.path);
close(fd);
return -1;
}
}
}
- /* Now seek to final size, possibly making the file sparse */
- if (ftruncate(fd, vol->capacity) < 0) {
- virReportSystemError(conn, errno,
- _("cannot extend file '%s'"),
- vol->target.path);
- unlink(vol->target.path);
- close(fd);
- return -1;
- }
} else if (vol->target.format == VIR_STORAGE_VOL_FILE_DIR) {
if (mkdir(vol->target.path, vol->target.perms.mode) < 0) {
virReportSystemError(conn, errno,
@@ -1127,7 +1140,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
snprintf(size, sizeof(size), "%llu", vol->capacity/1024);
if (virRun(conn, imgargv, NULL) < 0) {
- unlink(vol->target.path);
return -1;
}
@@ -1135,7 +1147,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot read path '%s'"),
vol->target.path);
- unlink(vol->target.path);
return -1;
}
#elif HAVE_QCOW_CREATE
@@ -1168,7 +1179,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
imgargv[3] = NULL;
if (virRun(conn, imgargv, NULL) < 0) {
- unlink(vol->target.path);
return -1;
}
@@ -1176,7 +1186,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot read path '%s'"),
vol->target.path);
- unlink(vol->target.path);
return -1;
}
#else
@@ -1193,7 +1202,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot set file owner '%s'"),
vol->target.path);
- unlink(vol->target.path);
close(fd);
return -1;
}
@@ -1202,7 +1210,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot set file mode '%s'"),
vol->target.path);
- unlink(vol->target.path);
close(fd);
return -1;
}
@@ -1211,7 +1218,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
if (virStorageBackendUpdateVolTargetInfoFD(conn, &vol->target, fd,
&vol->allocation,
NULL) < 0) {
- unlink(vol->target.path);
close(fd);
return -1;
}
@@ -1220,7 +1226,6 @@ virStorageBackendFileSystemVolCreate(virConnectPtr conn,
virReportSystemError(conn, errno,
_("cannot close file '%s'"),
vol->target.path);
- unlink(vol->target.path);
return -1;
}
@@ -1268,6 +1273,7 @@ virStorageBackend virStorageBackendDirectory = {
.buildPool = virStorageBackendFileSystemBuild,
.refreshPool = virStorageBackendFileSystemRefresh,
.deletePool = virStorageBackendFileSystemDelete,
+ .buildVol = virStorageBackendFileSystemVolBuild,
.createVol = virStorageBackendFileSystemVolCreate,
.refreshVol = virStorageBackendFileSystemVolRefresh,
.deleteVol = virStorageBackendFileSystemVolDelete,
@@ -1282,6 +1288,7 @@ virStorageBackend virStorageBackendFileSystem = {
.refreshPool = virStorageBackendFileSystemRefresh,
.stopPool = virStorageBackendFileSystemStop,
.deletePool = virStorageBackendFileSystemDelete,
+ .buildVol = virStorageBackendFileSystemVolBuild,
.createVol = virStorageBackendFileSystemVolCreate,
.refreshVol = virStorageBackendFileSystemVolRefresh,
.deleteVol = virStorageBackendFileSystemVolDelete,
@@ -1295,6 +1302,7 @@ virStorageBackend virStorageBackendNetFileSystem = {
.refreshPool = virStorageBackendFileSystemRefresh,
.stopPool = virStorageBackendFileSystemStop,
.deletePool = virStorageBackendFileSystemDelete,
+ .buildVol = virStorageBackendFileSystemVolBuild,
.createVol = virStorageBackendFileSystemVolCreate,
.refreshVol = virStorageBackendFileSystemVolRefresh,
.deleteVol = virStorageBackendFileSystemVolDelete,
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 8cb8d4b..3803474 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -1189,6 +1189,8 @@ cleanup:
return ret;
}
+static int storageVolumeDelete(virStorageVolPtr obj, unsigned int flags);
+
static virStorageVolPtr
storageVolumeCreateXML(virStoragePoolPtr obj,
const char *xmldesc,
@@ -1196,8 +1198,8 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
virStorageDriverStatePtr driver = obj->conn->storagePrivateData;
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
- virStorageVolDefPtr vol = NULL;
- virStorageVolPtr ret = NULL;
+ virStorageVolDefPtr voldef = NULL;
+ virStorageVolPtr ret = NULL, volobj = NULL;
storageDriverLock(driver);
pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
@@ -1218,11 +1220,11 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
if ((backend = virStorageBackendForType(pool->def->type)) == NULL)
goto cleanup;
- vol = virStorageVolDefParse(obj->conn, pool->def, xmldesc, NULL);
- if (vol == NULL)
+ voldef = virStorageVolDefParse(obj->conn, pool->def, xmldesc, NULL);
+ if (voldef == NULL)
goto cleanup;
- if (virStorageVolDefFindByName(pool, vol->name)) {
+ if (virStorageVolDefFindByName(pool, voldef->name)) {
virStorageReportError(obj->conn, VIR_ERR_INVALID_STORAGE_POOL,
"%s", _("storage vol already exists"));
goto cleanup;
@@ -1236,22 +1238,72 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
if (!backend->createVol) {
virStorageReportError(obj->conn, VIR_ERR_NO_SUPPORT,
- "%s", _("storage pool does not support volume creation"));
+ "%s", _("storage pool does not support volume "
+ "creation"));
goto cleanup;
}
- /* XXX ought to drop pool lock while creating instance */
- if (backend->createVol(obj->conn, pool, vol) < 0) {
+ if (backend->createVol(obj->conn, pool, voldef) < 0) {
goto cleanup;
}
- pool->volumes.objs[pool->volumes.count++] = vol;
+ pool->volumes.objs[pool->volumes.count++] = voldef;
+ volobj = virGetStorageVol(obj->conn, pool->def->name, voldef->name,
+ voldef->key);
- ret = virGetStorageVol(obj->conn, pool->def->name, vol->name, vol->key);
- vol = NULL;
+ if (0) {
+ printf("after vol lookup.\n");
+ virReportOOMError(obj->conn);
+ goto cleanup;
+ }
+
+ if (volobj && backend->buildVol) {
+ int buildret;
+ virStorageVolDefPtr buildvoldef = NULL;
+
+ if (VIR_ALLOC(buildvoldef) < 0) {
+ virReportOOMError(obj->conn);
+ voldef = NULL;
+ goto cleanup;
+ }
+
+ /* Make a shallow copy of the 'defined' volume definition, since the
+ * original allocation value will change as the user polls 'info',
+ * but we only need the initial requested values
+ */
+ memcpy(buildvoldef, voldef, sizeof(*voldef));
+ voldef = NULL;
+
+ /* Drop the pool lock during volume allocation */
+ pool->asyncjobs++;
+ virStoragePoolObjUnlock(pool);
+
+ buildvoldef->building = 1;
+ buildret = backend->buildVol(obj->conn, buildvoldef);
+ buildvoldef->building = 0;
+
+ virStoragePoolObjLock(pool);
+ pool->asyncjobs--;
+
+ VIR_FREE(buildvoldef);
+
+ if (buildret < 0) {
+ virStoragePoolObjUnlock(pool);
+ storageVolumeDelete(volobj, 0);
+ pool = NULL;
+ goto cleanup;
+ }
+
+ }
+
+ ret = volobj;
+ volobj = NULL;
+ voldef = NULL;
cleanup:
- virStorageVolDefFree(vol);
+ if (volobj)
+ virUnrefStorageVol(volobj);
+ virStorageVolDefFree(voldef);
if (pool)
virStoragePoolObjUnlock(pool);
return ret;
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list