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@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list