From: Prasanna Kumar Kalever <prasanna.kalever(a)redhat.com>
This patch optimizes calls to glfs_init() and friends
Currently, a start of a VM will call 2 glfs_new/glfs_init (which will create
glfs object, once for stat, read headers and next to chown) and then will fork
qemu process which will call once again (for actual read write IO).
Not that all, in case if we are have 4 extra attached disks, then the total
calls to glfs_init() and friends will be (4+1)*2 in libvirt and (4+1)*1 in
qemu space i.e 15 calls. Since we don't have control over qemu process as that
executes in a different process environment, lets do not bother much about it.
This patch shrinks these 10 calls (i.e objects from above example) to just
one, by maintaining a cache of glfs objects.
Additionally snapshot(external) scenario will further complex the situation ...
The glfs object is shared across other only if volume name and all the
volfile servers match (includes hostname, transport and port number).
In case of hit glfs object takes a ref and on close unref happens.
Thanks to 'Peter Krempa' for all the inputs.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever(a)redhat.com>
---
v2: Address review comments from Peter on v1
* Rebased on latest master
* Changes to commit msg
* Introduce storage API's for Register and Unregister of volume
* During qemu process Start/Stop and snapshot create
* Check Transport and Port type
* Atomic element add/del to list and ref counting
Pending: Treating IP and FQDN belong to same host
v1: Initial patch
---
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_domain.h | 5 +
src/qemu/qemu_driver.c | 29 ++++
src/qemu/qemu_process.c | 25 +++
src/storage/storage_backend_fs.c | 2 +
src/storage/storage_backend_gluster.c | 295 +++++++++++++++++++++++++++++++---
src/storage/storage_driver.c | 23 ++-
src/storage/storage_driver.h | 3 +
8 files changed, 357 insertions(+), 27 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 47332a8..35914c5 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4837,7 +4837,7 @@ qemuDomainCleanupRun(virQEMUDriverPtr driver,
priv->ncleanupCallbacks_max = 0;
}
-static void
+void
qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
virDomainObjPtr vm,
virStorageSourcePtr src,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7650ff3..a9e38bd 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -591,6 +591,11 @@ bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
virDomainDiskDefPtr orig_disk);
+void qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ uid_t *uid, gid_t *gid);
+
int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3517aa2..7cae094 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14211,6 +14211,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
cleanup:
if (need_unlink && virStorageFileUnlink(newDiskSrc))
VIR_WARN("unable to unlink just-created %s", source);
+ virStorageFileDeinit(disk->src);
virStorageFileDeinit(newDiskSrc);
virStorageSourceFree(newDiskSrc);
virStorageSourceFree(persistDiskSrc);
@@ -14566,6 +14567,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
virDomainSnapshotObjPtr snap = NULL;
virDomainSnapshotPtr snapshot = NULL;
virDomainSnapshotDefPtr def = NULL;
+ virDomainSnapshotDefPtr refDef = NULL;
bool update_current = true;
bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE;
unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS;
@@ -14574,6 +14576,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
bool align_match = true;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
+ unsigned int dIndex;
+ uid_t uid;
+ gid_t gid;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
@@ -14690,6 +14695,19 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
qemuDomainObjSetAsyncJobMask(vm, QEMU_JOB_NONE);
+ for (dIndex = 0; dIndex < def->ndisks; dIndex++) {
+ virDomainSnapshotDiskDef disk = def->disks[dIndex];
+
+ if (virStorageSourceIsEmpty(disk.src))
+ continue;
+
+ qemuDomainGetImageIds(cfg, vm, disk.src, &uid, &gid);
+
+ if (virStorageVolumeRegister(disk.src, uid, gid) <0)
+ goto cleanup;
+ }
+
+
if (redefine) {
if (virDomainSnapshotRedefinePrep(domain, vm, &def, &snap,
&update_current, flags) < 0)
@@ -14800,6 +14818,17 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
snapshot = virGetDomainSnapshot(domain, snap->def->name);
endjob:
+ refDef = (!snap) ? def : snap->def;
+
+ for (dIndex = 0; dIndex < refDef->ndisks; dIndex++) {
+ virDomainSnapshotDiskDef disk = refDef->disks[dIndex];
+
+ if (virStorageSourceIsEmpty(disk.src))
+ continue;
+
+ virStorageVolumeUnRegister(disk.src);
+ }
+
if (snapshot && !(flags & VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA)) {
if (qemuDomainSnapshotWriteMetadata(vm, snap, driver->caps,
cfg->snapshotDir) < 0) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ecd7ded..20793cf 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4612,6 +4612,9 @@ qemuProcessInit(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
int stopFlags;
int ret = -1;
+ unsigned int dIndex;
+ uid_t uid;
+ gid_t gid;
VIR_DEBUG("vm=%p name=%s id=%d migration=%d",
vm, vm->def->name, vm->def->id, migration);
@@ -4664,6 +4667,18 @@ qemuProcessInit(virQEMUDriverPtr driver,
if (qemuDomainSetPrivatePaths(driver, vm) < 0)
goto cleanup;
+ for (dIndex = 0; dIndex < vm->def->ndisks; dIndex++) {
+ virDomainDiskDefPtr disk = vm->def->disks[dIndex];
+
+ if (virStorageSourceIsEmpty(disk->src))
+ continue;
+
+ qemuDomainGetImageIds(cfg, vm, disk->src, &uid, &gid);
+
+ if (virStorageVolumeRegister(disk->src, uid, gid) < 0)
+ goto cleanup;
+ }
+
ret = 0;
cleanup:
@@ -5671,6 +5686,7 @@ qemuProcessFinishStartup(virConnectPtr conn,
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
+ unsigned int dIndex;
if (startCPUs) {
VIR_DEBUG("Starting domain CPUs");
@@ -5695,6 +5711,15 @@ qemuProcessFinishStartup(virConnectPtr conn,
VIR_HOOK_SUBOP_BEGIN) < 0)
goto cleanup;
+ for (dIndex = 0; dIndex < vm->def->ndisks; dIndex++) {
+ virDomainDiskDefPtr disk = vm->def->disks[dIndex];
+
+ if (virStorageSourceIsEmpty(disk->src))
+ continue;
+
+ virStorageVolumeUnRegister(disk->src);
+ }
+
ret = 0;
cleanup:
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de0e8d5..0e03e06 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1488,6 +1488,8 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
VIR_FREE(priv->canonpath);
VIR_FREE(priv);
+
+ VIR_FREE(src->drv);
}
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 8e86704..18b7fc3 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -31,11 +31,34 @@
#include "virstoragefile.h"
#include "virstring.h"
#include "viruri.h"
+#include "viratomic.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
VIR_LOG_INIT("storage.storage_backend_gluster");
+static bool virGlusterGlobalError;
+static virOnceControl glusterConnOnce = VIR_ONCE_CONTROL_INITIALIZER;
+
+typedef struct _virStorageBackendGlusterState virStorageBackendGlusterState;
+typedef virStorageBackendGlusterState *virStorageBackendGlusterStatePtr;
+
+typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv;
+typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr;
+
+typedef struct _unixSocketAddress unixSocketAddress;
+
+typedef struct _inetSocketAddress inetSocketAddress;
+
+typedef struct _glusterServer glusterServer;
+typedef struct _glusterServer *glusterServerPtr;
+
+typedef struct _virStorageBackendGlusterStatePreopened virStorageBackendGlusterStatePreopened;
+typedef virStorageBackendGlusterStatePreopened *virStorageBackendGlusterStatePtrPreopened;
+
+typedef struct _virStorageBackendGlusterconnCache virStorageBackendGlusterconnCache;
+typedef virStorageBackendGlusterconnCache *virStorageBackendGlusterconnCachePtr;
+
struct _virStorageBackendGlusterState {
glfs_t *vol;
@@ -47,8 +70,241 @@ struct _virStorageBackendGlusterState {
char *dir; /* dir from URI, or "/"; always starts and ends in '/' */
};
-typedef struct _virStorageBackendGlusterState virStorageBackendGlusterState;
-typedef virStorageBackendGlusterState *virStorageBackendGlusterStatePtr;
+struct _virStorageFileBackendGlusterPriv {
+ glfs_t *vol;
+ char *canonpath;
+};
+
+struct _unixSocketAddress {
+ char *path;
+};
+
+struct _inetSocketAddress {
+ char *host;
+ char *port;
+};
+
+struct _glusterServer {
+ virStorageNetHostTransport type;
+ union { /* union tag is @type */
+ unixSocketAddress uds;
+ inetSocketAddress tcp;
+ } u;
+};
+
+struct _virStorageBackendGlusterStatePreopened {
+ virStorageFileBackendGlusterPrivPtr priv;
+ unsigned int nservers;
+ glusterServerPtr *hosts;
+ char *volname;
+ volatile int ref;
+};
+
+struct _virStorageBackendGlusterconnCache {
+ virMutex lock;
+ size_t nConn;
+ virStorageBackendGlusterStatePtrPreopened *conn;
+};
+
+virStorageBackendGlusterconnCachePtr connCache = {0,};
+
+
+static void
+virGlusterConnAlloc(void)
+{
+ if ((VIR_ALLOC(connCache) < 0))
+ virGlusterGlobalError = false;
+}
+
+static int
+virStorageBackendGlusterSetPreopened(virStorageSourcePtr src,
+ virStorageFileBackendGlusterPrivPtr priv)
+{
+ unsigned int idx;
+ virStorageBackendGlusterStatePtrPreopened entry = NULL;
+
+ if (connCache == NULL && (virOnce(&glusterConnOnce,
+ virGlusterConnAlloc) < 0))
+ return -1;
+
+ if (virGlusterGlobalError)
+ return -1;
+
+ if (!connCache->nConn) {
+ if (virMutexInit(&connCache->lock) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to initialize mutex"));
+ return -1;
+ }
+ }
+
+ for (idx = 0; idx < connCache->nConn; idx++) {
+ if (STREQ(connCache->conn[idx]->volname, src->volume))
+ return 0;
+ }
+
+ if (VIR_ALLOC(entry) < 0)
+ return -1;
+
+ if (VIR_STRDUP(entry->volname, src->volume) < 0)
+ goto error;
+
+ if (VIR_ALLOC_N(entry->hosts, entry->nservers) < 0)
+ goto error;
+
+ entry->nservers = src->nhosts;
+
+ for (idx = 0; idx < src->nhosts; idx++) {
+ if (VIR_ALLOC(entry->hosts[idx]) < 0)
+ goto error;
+ if (src->hosts[idx].transport == VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+ if (VIR_STRDUP(entry->hosts[idx]->u.uds.path,
+ src->hosts[idx].socket) < 0)
+ goto error;
+ entry->hosts[idx]->type = VIR_STORAGE_NET_HOST_TRANS_UNIX;
+ } else if (src->hosts[idx].transport ==
+ VIR_STORAGE_NET_HOST_TRANS_TCP) {
+ if (VIR_STRDUP(entry->hosts[idx]->u.tcp.host,
+ src->hosts[idx].name) < 0)
+ goto error;
+ if (VIR_STRDUP(entry->hosts[idx]->u.tcp.port,
+ src->hosts[idx].port) < 0)
+ goto error;
+ entry->hosts[idx]->type = VIR_STORAGE_NET_HOST_TRANS_TCP;
+ } else {
+ entry->hosts[idx]->type = VIR_STORAGE_NET_HOST_TRANS_LAST;
+ }
+ }
+ entry->priv = priv;
+
+ virMutexLock(&connCache->lock);
+ virAtomicIntSet(&entry->ref, 1);
+ if (VIR_INSERT_ELEMENT(connCache->conn, -1, connCache->nConn, entry) < 0) {
+ virMutexUnlock(&connCache->lock);
+ goto error;
+ }
+ virMutexUnlock(&connCache->lock);
+
+ return 0;
+
+ error:
+ for (idx = 0; idx < entry->nservers; idx++) {
+ if (entry->hosts[idx]->type == VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+ VIR_FREE(entry->hosts[idx]->u.uds.path);
+ } else {
+ VIR_FREE(entry->hosts[idx]->u.tcp.host);
+ VIR_FREE(entry->hosts[idx]->u.tcp.port);
+ }
+ VIR_FREE(entry->hosts[idx]);
+ }
+
+ VIR_FREE(entry->hosts);
+ VIR_FREE(entry->volname);
+ VIR_FREE(entry);
+
+ return -1;
+}
+
+static glfs_t *
+virStorageBackendGlusterFindPreopened(virStorageSourcePtr src)
+{
+ unsigned int cIdx, sIdx, nIdx; /* connectionIdx, savedIdx, newIdx */
+ bool flag = false;
+
+ if (connCache == NULL)
+ return NULL;
+
+ virStorageBackendGlusterStatePtrPreopened entry;
+
+ for (cIdx = 0; cIdx < connCache->nConn; cIdx++) {
+ entry = connCache->conn[cIdx];
+ if (STREQ(entry->volname, src->volume)) {
+ if (entry->nservers == src->nhosts) {
+ for (sIdx = 0; sIdx < entry->nservers; sIdx++) {
+ for (nIdx = 0; nIdx < src->nhosts; nIdx++) {
+ if (entry->hosts[sIdx]->type ==
+ src->hosts[nIdx].transport) {
+ if (entry->hosts[sIdx]->type
+ == VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+ if (STREQ(entry->hosts[sIdx]->u.uds.path,
+ src->hosts[nIdx].socket)) {
+ flag = true;
+ break;
+ }
+ } else {
+ if (STREQ(entry->hosts[sIdx]->u.tcp.host,
+ src->hosts[nIdx].name) &&
+ STREQ(entry->hosts[sIdx]->u.tcp.port,
+ src->hosts[nIdx].port)) {
+ flag = true;
+ break;
+ }
+ }
+ }
+ }
+ if (!flag)
+ return NULL;
+ flag = false;
+ }
+ virAtomicIntInc(&entry->ref);
+ return entry->priv->vol;
+ } else {
+ return NULL;
+ }
+ }
+ }
+ return NULL;
+}
+
+static void
+virStorageBackendGlusterClosePreopened(virStorageSourcePtr src)
+{
+ virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
+ unsigned int cIdx, hIdx; /* connectionIdx, hostIdx */
+
+ for (cIdx = 0; cIdx < connCache->nConn; cIdx++) {
+ if (STREQ(connCache->conn[cIdx]->volname, src->volume)) {
+ virAtomicIntDecAndTest(&connCache->conn[cIdx]->ref);
+ if (virAtomicIntGet(&connCache->conn[cIdx]->ref) != 0) {
+ if (priv && priv->canonpath) {
+ VIR_FREE(priv->canonpath);
+ VIR_FREE(priv);
+ src->drv->priv = NULL;
+ }
+ return;
+ }
+
+ glfs_fini(connCache->conn[cIdx]->priv->vol);
+
+ VIR_FREE(connCache->conn[cIdx]->priv->canonpath);
+ VIR_FREE(connCache->conn[cIdx]->priv);
+
+ for (hIdx = 0; hIdx < connCache->conn[cIdx]->nservers; hIdx++) {
+ if (connCache->conn[cIdx]->hosts[hIdx]->type ==
+ VIR_STORAGE_NET_HOST_TRANS_UNIX) {
+ VIR_FREE(connCache->conn[cIdx]->hosts[hIdx]->u.uds.path);
+ } else {
+ VIR_FREE(connCache->conn[cIdx]->hosts[hIdx]->u.tcp.host);
+ VIR_FREE(connCache->conn[cIdx]->hosts[hIdx]->u.tcp.port);
+ }
+ VIR_FREE(connCache->conn[cIdx]->hosts[hIdx]);
+ }
+
+ VIR_FREE(connCache->conn[cIdx]->hosts);
+ VIR_FREE(connCache->conn[cIdx]->volname);
+ VIR_FREE(connCache->conn[cIdx]);
+
+ virMutexLock(&connCache->lock);
+ VIR_DELETE_ELEMENT(connCache->conn, cIdx, connCache->nConn);
+ virMutexUnlock(&connCache->lock);
+
+ VIR_FREE(src->drv);
+ }
+ }
+
+ if (!connCache->conn)
+ virMutexDestroy(&connCache->lock);
+}
static void
virStorageBackendGlusterClose(virStorageBackendGlusterStatePtr state)
@@ -538,30 +794,14 @@ virStorageBackend virStorageBackendGluster = {
};
-typedef struct _virStorageFileBackendGlusterPriv virStorageFileBackendGlusterPriv;
-typedef virStorageFileBackendGlusterPriv *virStorageFileBackendGlusterPrivPtr;
-
-struct _virStorageFileBackendGlusterPriv {
- glfs_t *vol;
- char *canonpath;
-};
-
-
static void
virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
{
- virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
-
VIR_DEBUG("deinitializing gluster storage file %p (gluster://%s:%s/%s%s)",
src, src->hosts->name, src->hosts->port ? src->hosts->port : "0",
src->volume, src->path);
- if (priv->vol)
- glfs_fini(priv->vol);
- VIR_FREE(priv->canonpath);
-
- VIR_FREE(priv);
- src->drv->priv = NULL;
+ virStorageBackendGlusterClosePreopened(src);
}
static int
@@ -612,6 +852,7 @@ virStorageFileBackendGlusterInitServer(virStorageFileBackendGlusterPrivPtr priv,
static int
virStorageFileBackendGlusterInit(virStorageSourcePtr src)
{
+ int ret = 0;
virStorageFileBackendGlusterPrivPtr priv = NULL;
size_t i;
@@ -625,6 +866,12 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
if (VIR_ALLOC(priv) < 0)
return -1;
+ priv->vol = virStorageBackendGlusterFindPreopened(src);
+ if (priv->vol) {
+ src->drv->priv = priv;
+ return ret;
+ }
+
VIR_DEBUG("initializing gluster storage file %p "
"(priv='%p' volume='%s' path='%s') as [%u:%u]",
src, priv, src->volume, src->path,
@@ -635,6 +882,10 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
goto error;
}
+ ret = virStorageBackendGlusterSetPreopened(src, priv);
+ if (ret < 0)
+ goto error;
+
for (i = 0; i < src->nhosts; i++) {
if (virStorageFileBackendGlusterInitServer(priv, src->hosts + i) < 0)
goto error;
@@ -648,13 +899,13 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
}
src->drv->priv = priv;
+ ret = 0;
- return 0;
+ return ret;
error:
- if (priv->vol)
- glfs_fini(priv->vol);
VIR_FREE(priv);
+ virStorageBackendGlusterClosePreopened(src);
return -1;
}
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index df65807..9c3bffb 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2900,11 +2900,8 @@ virStorageFileDeinit(virStorageSourcePtr src)
if (!virStorageFileIsInitialized(src))
return;
- if (src->drv->backend &&
- src->drv->backend->backendDeinit)
+ if (src->drv->backend && src->drv->backend->backendDeinit)
src->drv->backend->backendDeinit(src);
-
- VIR_FREE(src->drv);
}
@@ -2967,6 +2964,24 @@ virStorageFileInit(virStorageSourcePtr src)
return virStorageFileInitAs(src, -1, -1);
}
+int
+virStorageVolumeRegister(virStorageSourcePtr src,
+ uid_t uid, gid_t gid)
+{
+ if (virStorageFileInitAs(src, uid, gid) < 0)
+ return -1;
+
+ src->drv->priv = NULL;
+
+ return 0;
+}
+
+void
+virStorageVolumeUnRegister(virStorageSourcePtr src)
+{
+ virStorageFileDeinit(src);
+}
+
/**
* virStorageFileCreate: Creates an empty storage file via storage driver
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index 3f2549d..89a3d55 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -71,4 +71,7 @@ char *virStoragePoolObjBuildTempFilePath(virStoragePoolObjPtr pool,
int storageRegister(void);
+int virStorageVolumeRegister(virStorageSourcePtr src, uid_t uid, gid_t gid);
+void virStorageVolumeUnRegister(virStorageSourcePtr src);
+
#endif /* __VIR_STORAGE_DRIVER_H__ */
--
2.7.4
