[libvirt] [PATCH v5 2/3] storage: optimize calls to virStorageFileInit and friends
Prasanna Kumar Kalever
prasanna.kalever at redhat.com
Thu Dec 15 08:07:04 UTC 2016
Currently, each among virStorageFileGetMetadataRecurse,
qemuSecurityChownCallback, qemuDomainSnapshotPrepareDiskExternal and
qemuDomainSnapshotCreateSingleDiskActive makes calls to virStorageFileInit
and friends for simple operations like stat, read headers, chown and etc.
This patch
1. optimize/unify calls to virStorageFileInit and virStorageFileDeinit.
2. addes virObject to virStorageDriverData to keep reference.
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever at redhat.com>
---
src/qemu/qemu_domain.c | 25 +-----------------
src/qemu/qemu_domain.h | 8 +++---
src/qemu/qemu_driver.c | 26 ++++++++++++-------
src/qemu/qemu_process.c | 49 +++++++++++++++++++++++++++++++++++
src/qemu/qemu_process.h | 4 +++
src/storage/storage_backend.h | 1 +
src/storage/storage_backend_fs.c | 5 ++++
src/storage/storage_backend_gluster.c | 6 ++++-
src/storage/storage_driver.c | 33 +++++++++++++++++++----
src/storage/storage_driver.h | 2 ++
src/util/virstoragefile.h | 1 +
11 files changed, 117 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4aae14d..fcdf717 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,
@@ -4869,29 +4869,6 @@ qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
}
-int
-qemuDomainStorageFileInit(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr src)
-{
- virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
- uid_t uid;
- gid_t gid;
- int ret = -1;
-
- qemuDomainGetImageIds(cfg, vm, src, &uid, &gid);
-
- if (virStorageFileInitAs(src, uid, gid) < 0)
- goto cleanup;
-
- ret = 0;
-
- cleanup:
- virObjectUnref(cfg);
- return ret;
-}
-
-
char *
qemuDomainStorageAlias(const char *device, int depth)
{
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 7650ff3..8beab7a 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -591,9 +591,11 @@ bool qemuDomainDiskSourceDiffers(virDomainDiskDefPtr disk,
bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
virDomainDiskDefPtr orig_disk);
-int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- virStorageSourcePtr src);
+void qemuDomainGetImageIds(virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm,
+ virStorageSourcePtr src,
+ uid_t *uid, gid_t *gid);
+
char *qemuDomainStorageAlias(const char *device, int depth);
void qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3517aa2..6761915 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13865,9 +13865,6 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
return -1;
}
- if (virStorageFileInit(snapdisk->src) < 0)
- return -1;
-
if (virStorageFileStat(snapdisk->src, &st) < 0) {
if (errno != ENOENT) {
virReportSystemError(errno,
@@ -13891,7 +13888,6 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn,
ret = 0;
cleanup:
- virStorageFileDeinit(snapdisk->src);
return ret;
}
@@ -13955,7 +13951,8 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn,
static int
-qemuDomainSnapshotPrepare(virConnectPtr conn,
+qemuDomainSnapshotPrepare(virQEMUDriverConfigPtr cfg,
+ virConnectPtr conn,
virDomainObjPtr vm,
virDomainSnapshotDefPtr def,
unsigned int *flags)
@@ -14026,6 +14023,9 @@ qemuDomainSnapshotPrepare(virConnectPtr conn,
goto cleanup;
}
+ if (qemuStorageVolumeRegister(cfg, vm, def->disks[i].src) < 0)
+ goto cleanup;
+
if (qemuDomainSnapshotPrepareDiskExternal(conn, dom_disk, disk,
active, reuse) < 0)
goto cleanup;
@@ -14139,10 +14139,9 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
if (!(newDiskSrc = virStorageSourceCopy(snap->src, false)))
goto cleanup;
- if (virStorageSourceInitChainElement(newDiskSrc, disk->src, false) < 0)
- goto cleanup;
+ newDiskSrc->drv = snap->src->drv;
- if (qemuDomainStorageFileInit(driver, vm, newDiskSrc) < 0)
+ if (virStorageSourceInitChainElement(newDiskSrc, disk->src, false) < 0)
goto cleanup;
if (qemuGetDriveSourceString(newDiskSrc, NULL, &source) < 0)
@@ -14211,7 +14210,6 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
cleanup:
if (need_unlink && virStorageFileUnlink(newDiskSrc))
VIR_WARN("unable to unlink just-created %s", source);
- virStorageFileDeinit(newDiskSrc);
virStorageSourceFree(newDiskSrc);
virStorageSourceFree(persistDiskSrc);
VIR_FREE(device);
@@ -14566,6 +14564,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 +14573,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
bool align_match = true;
virQEMUDriverConfigPtr cfg = NULL;
virCapsPtr caps = NULL;
+ size_t i;
virCheckFlags(VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE |
VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT |
@@ -14732,7 +14732,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
}
if (virDomainSnapshotAlignDisks(def, align_location,
align_match) < 0 ||
- qemuDomainSnapshotPrepare(conn, vm, def, &flags) < 0)
+ qemuDomainSnapshotPrepare(cfg, conn, vm, def, &flags) < 0)
goto endjob;
}
@@ -14800,6 +14800,12 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
snapshot = virGetDomainSnapshot(domain, snap->def->name);
endjob:
+ refDef = (!snap) ? def : snap->def;
+ for (i = 0; i < refDef->ndisks; i++) {
+ if (refDef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL)
+ qemuStorageVolumeUnRegister(refDef->disks[i].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 7f19c69..7bec469 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4590,6 +4590,35 @@ qemuProcessStartValidate(virQEMUDriverPtr driver,
}
+int
+qemuStorageVolumeRegister(virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm, virStorageSourcePtr src)
+{
+ uid_t uid;
+ gid_t gid;
+
+ if (virStorageSourceIsEmpty(src))
+ return 0;
+
+ qemuDomainGetImageIds(cfg, vm, src, &uid, &gid);
+
+ if (virStorageFileInitAs(src, uid, gid) < 0)
+ return -1;
+
+ return 0;
+}
+
+
+void
+qemuStorageVolumeUnRegister(virStorageSourcePtr src)
+{
+ if (virStorageSourceIsEmpty(src))
+ return;
+
+ virStorageFileDeinit(src);
+}
+
+
/**
* qemuProcessInit:
*
@@ -4614,6 +4643,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
qemuDomainObjPrivatePtr priv = vm->privateData;
int stopFlags;
int ret = -1;
+ size_t i;
VIR_DEBUG("vm=%p name=%s id=%d migration=%d",
vm, vm->def->name, vm->def->id, migration);
@@ -4666,6 +4696,12 @@ qemuProcessInit(virQEMUDriverPtr driver,
if (qemuDomainSetPrivatePaths(driver, vm) < 0)
goto cleanup;
+ if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) {
+ for (i = 0; i < vm->def->ndisks; i++)
+ if (qemuStorageVolumeRegister(cfg, vm, vm->def->disks[i]->src) < 0)
+ goto cleanup;
+ }
+
ret = 0;
cleanup:
@@ -5674,6 +5710,7 @@ qemuProcessFinishStartup(virConnectPtr conn,
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
int ret = -1;
+ size_t i;
if (startCPUs) {
VIR_DEBUG("Starting domain CPUs");
@@ -5698,6 +5735,9 @@ qemuProcessFinishStartup(virConnectPtr conn,
VIR_HOOK_SUBOP_BEGIN) < 0)
goto cleanup;
+ for (i = 0; i < vm->def->ndisks; i++)
+ qemuStorageVolumeUnRegister(vm->def->disks[i]->src);
+
ret = 0;
cleanup:
@@ -6047,6 +6087,12 @@ void qemuProcessStop(virQEMUDriverPtr driver,
VIR_FREE(xml);
}
+ for (i = 0; i < vm->def->ndisks; i++) {
+ vm->def->disks[i]->src->drv = NULL; /* FIXME: Brings in garbage */
+ if (qemuStorageVolumeRegister(cfg, vm, vm->def->disks[i]->src) < 0)
+ goto cleanup;
+ }
+
/* Reset Security Labels unless caller don't want us to */
if (!(flags & VIR_QEMU_PROCESS_STOP_NO_RELABEL))
virSecurityManagerRestoreAllLabel(driver->securityManager,
@@ -6210,6 +6256,9 @@ void qemuProcessStop(virQEMUDriverPtr driver,
VIR_FREE(xml);
}
+ for (i = 0; i < vm->def->ndisks; i++)
+ qemuStorageVolumeUnRegister(vm->def->disks[i]->src);
+
virDomainObjRemoveTransientDef(vm);
endjob:
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 21f3b0c..e53a154 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -90,6 +90,10 @@ virCommandPtr qemuProcessCreatePretendCmd(virConnectPtr conn,
bool standalone,
unsigned int flags);
+int qemuStorageVolumeRegister(virQEMUDriverConfigPtr cfg,
+ virDomainObjPtr vm, virStorageSourcePtr src);
+void qemuStorageVolumeUnRegister(virStorageSourcePtr src);
+
int qemuProcessInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 28e1a65..94fa118 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -249,6 +249,7 @@ typedef struct _virStorageFileBackend virStorageFileBackend;
typedef virStorageFileBackend *virStorageFileBackendPtr;
struct _virStorageDriverData {
+ virObject parent;
virStorageFileBackendPtr backend;
void *priv;
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de0e8d5..1fd134c 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1486,6 +1486,11 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
virStorageFileBackendFsPrivPtr priv = src->drv->priv;
+ if (virObjectUnref(src->drv))
+ return;
+
+ src->drv = NULL;
+
VIR_FREE(priv->canonpath);
VIR_FREE(priv);
}
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 0d5b7f6..779a3bd 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -557,12 +557,16 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
src->hosts->u.inet.port ? src->hosts->u.inet.port : "0",
src->volume, src->path);
+ if (virObjectUnref(src->drv))
+ return;
+
+ src->drv = NULL;
+
if (priv->vol)
glfs_fini(priv->vol);
VIR_FREE(priv->canonpath);
VIR_FREE(priv);
- src->drv->priv = NULL;
}
static int
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7c7fddd..39f38d3 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2831,7 +2831,7 @@ int storageRegister(void)
/* ----------- file handlers cooperating with storage driver --------------- */
-static bool
+bool
virStorageFileIsInitialized(virStorageSourcePtr src)
{
return src && src->drv;
@@ -2894,6 +2894,22 @@ virStorageFileSupportsSecurityDriver(virStorageSourcePtr src)
}
+static virClassPtr virStorageDriverDataClass;
+
+static int virStorageDriverDataOnceInit(void)
+{
+ if (!(virStorageDriverDataClass = virClassNew(virClassForObject(),
+ "virStorageDriverData",
+ sizeof(virStorageDriverData),
+ NULL)))
+ return -1;
+
+ return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virStorageDriverData)
+
+
void
virStorageFileDeinit(virStorageSourcePtr src)
{
@@ -2903,8 +2919,6 @@ virStorageFileDeinit(virStorageSourcePtr src)
if (src->drv->backend &&
src->drv->backend->backendDeinit)
src->drv->backend->backendDeinit(src);
-
- VIR_FREE(src->drv);
}
@@ -2926,7 +2940,16 @@ virStorageFileInitAs(virStorageSourcePtr src,
uid_t uid, gid_t gid)
{
int actualType = virStorageSourceGetActualType(src);
- if (VIR_ALLOC(src->drv) < 0)
+
+ if (virStorageFileIsInitialized(src)) {
+ virObjectRef(src->drv);
+ return 0;
+ }
+
+ if (virStorageDriverDataInitialize() < 0)
+ return -1;
+
+ if (!(src->drv = virObjectNew(virStorageDriverDataClass)))
return -1;
if (uid == (uid_t) -1)
@@ -2950,7 +2973,7 @@ virStorageFileInitAs(virStorageSourcePtr src,
return 0;
error:
- VIR_FREE(src->drv);
+ virObjectUnref(src->drv);
return -1;
}
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index 3f2549d..3fadb53 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -30,6 +30,8 @@
# include "storage_conf.h"
# include "virstoragefile.h"
+bool virStorageFileIsInitialized(virStorageSourcePtr src);
+
int virStorageFileInit(virStorageSourcePtr src);
int virStorageFileInitAs(virStorageSourcePtr src,
uid_t uid, gid_t gid);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 99b4a31..03308d4 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -29,6 +29,7 @@
# include "virstorageencryption.h"
# include "virutil.h"
# include "virsecret.h"
+# include "virobject.h"
/* Minimum header size required to probe all known formats with
* virStorageFileProbeFormat, or obtain metadata from a known format.
--
2.7.4
More information about the libvir-list
mailing list