[libvirt] [PATCH 4/6] storage: Add support for storage pool state XML
John Ferlan
jferlan at redhat.com
Thu Apr 2 14:29:12 UTC 2015
On 04/02/2015 06:10 AM, Erik Skultety wrote:
> This patch introduces new virStorageDriverState element stateDir.
> Also adds necessary changes to storageStateInitialize, so that
> directories initialization becomes more generic.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
^^ don't forget to remove...
> ---
> src/conf/storage_conf.h | 1 +
> src/storage/storage_driver.c | 109 +++++++++++++++++++++++++++++++------------
> 2 files changed, 81 insertions(+), 29 deletions(-)
>
> diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
> index da378b7..8d43019 100644
> --- a/src/conf/storage_conf.h
> +++ b/src/conf/storage_conf.h
> @@ -293,6 +293,7 @@ struct _virStorageDriverState {
>
> char *configDir;
> char *autostartDir;
> + char *stateDir;
> bool privileged;
> };
>
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 64ea770..0180fd7 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -78,6 +78,7 @@ static void
> storageDriverAutostart(void)
> {
> size_t i;
> + char *stateFile = NULL;
> virConnectPtr conn = NULL;
>
> /* XXX Remove hardcoding of QEMU URI */
> @@ -136,7 +137,14 @@ storageDriverAutostart(void)
> virStoragePoolObjUnlock(pool);
> continue;
> }
> +
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml")))
> + continue;
> +
> + ignore_value(virStoragePoolSaveState(stateFile, pool->def));
> pool->active = 1;
> + VIR_FREE(stateFile);
I didn't quite pick up on this the first time, but with everything
together now it was more obvious... If we cannot build the stateFile
path, then pool->active = 1 never happens, but the pool is started... So
it seems we'll have to stop the pool here as well and tell why (rather
than continue)...
Perhaps follow how you did things in storagePoolCreate[XML] with the
extra caveats noted below...
> }
> virStoragePoolObjUnlock(pool);
> }
> @@ -147,61 +155,67 @@ storageDriverAutostart(void)
> /**
> * virStorageStartup:
> *
> - * Initialization function for the QEmu daemon
> + * Initialization function for the Storage Driver
Yay! thanks ;-)
> */
> static int
> storageStateInitialize(bool privileged,
> virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> void *opaque ATTRIBUTE_UNUSED)
> {
> - char *base = NULL;
> + int ret = -1;
> + char *configdir = NULL;
> + char *rundir = NULL;
>
> if (VIR_ALLOC(driver) < 0)
> - return -1;
> + return ret;
>
> if (virMutexInit(&driver->lock) < 0) {
> VIR_FREE(driver);
> - return -1;
> + return ret;
> }
> storageDriverLock();
>
> if (privileged) {
> - if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0)
> + if (VIR_STRDUP(driver->configDir,
> + SYSCONFDIR "/libvirt/storage") < 0 ||
> + VIR_STRDUP(driver->autostartDir,
> + SYSCONFDIR "/libvirt/storage/autostart") < 0 ||
> + VIR_STRDUP(driver->stateDir,
> + LOCALSTATEDIR "/run/libvirt/storage") < 0)
> goto error;
> } else {
> - base = virGetUserConfigDirectory();
> - if (!base)
> + configdir = virGetUserConfigDirectory();
> + rundir = virGetUserRuntimeDirectory();
> + if (!(configdir && rundir))
> + goto error;
> +
> + if ((virAsprintf(&driver->configDir,
> + "%s/storage", configdir) < 0) ||
> + (virAsprintf(&driver->autostartDir,
> + "%s/storage", configdir) < 0) ||
> + (virAsprintf(&driver->stateDir,
> + "%s/storage/run", rundir) < 0))
> goto error;
> }
> driver->privileged = privileged;
>
> - /*
> - * Configuration paths are either $USER_CONFIG_HOME/libvirt/storage/...
> - * (session) or /etc/libvirt/storage/... (system).
> - */
> - if (virAsprintf(&driver->configDir,
> - "%s/storage", base) == -1)
> - goto error;
> -
> - if (virAsprintf(&driver->autostartDir,
> - "%s/storage/autostart", base) == -1)
> - goto error;
> -
> - VIR_FREE(base);
> -
> if (virStoragePoolLoadAllConfigs(&driver->pools,
> driver->configDir,
> driver->autostartDir) < 0)
> goto error;
>
> storageDriverUnlock();
> - return 0;
> +
> + ret = 0;
> + cleanup:
> + VIR_FREE(configdir);
> + VIR_FREE(rundir);
> + return ret;
>
> error:
> - VIR_FREE(base);
> storageDriverUnlock();
> storageStateCleanup();
> - return -1;
> + goto cleanup;
> }
>
> /**
> @@ -261,6 +275,7 @@ storageStateCleanup(void)
>
> VIR_FREE(driver->configDir);
> VIR_FREE(driver->autostartDir);
> + VIR_FREE(driver->stateDir);
> storageDriverUnlock();
> virMutexDestroy(&driver->lock);
> VIR_FREE(driver);
> @@ -579,6 +594,7 @@ storagePoolCreateXML(virConnectPtr conn,
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> virStorageBackendPtr backend;
> + char *stateFile;
>
> virCheckFlags(0, NULL);
>
> @@ -609,7 +625,12 @@ storagePoolCreateXML(virConnectPtr conn,
> goto cleanup;
> }
>
> - if (backend->refreshPool(conn, pool) < 0) {
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml")))
> + goto cleanup;
At this point the pool is started, so rather than goto cleanup, we'll
need to stop the pool
> +
> + if (virStoragePoolSaveState(stateFile, pool->def) < 0 ||
> + backend->refreshPool(conn, pool) < 0) {
> if (backend->stopPool)
> backend->stopPool(conn, pool);
> virStoragePoolObjRemove(&driver->pools, pool);
> @@ -745,6 +766,7 @@ storagePoolCreate(virStoragePoolPtr obj,
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> int ret = -1;
> + char *stateFile = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -763,21 +785,28 @@ storagePoolCreate(virStoragePoolPtr obj,
> pool->def->name);
> goto cleanup;
> }
> +
> + VIR_INFO("Starting up storage pool '%s'", pool->def->name);
> if (backend->startPool &&
> backend->startPool(obj->conn, pool) < 0)
> goto cleanup;
>
> - if (backend->refreshPool(obj->conn, pool) < 0) {
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml")))
> + goto cleanup;
Same start and stop on goto cleanup issue.
> +
> + if (virStoragePoolSaveState(stateFile, pool->def) ||
> + backend->refreshPool(obj->conn, pool) < 0) {
> if (backend->stopPool)
> backend->stopPool(obj->conn, pool);
> goto cleanup;
> }
>
> - VIR_INFO("Starting up storage pool '%s'", pool->def->name);
> pool->active = 1;
> ret = 0;
>
> cleanup:
> + VIR_FREE(stateFile);
> virStoragePoolObjUnlock(pool);
> return ret;
> }
> @@ -822,6 +851,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + char *stateFile = NULL;
> int ret = -1;
>
> storageDriverLock();
> @@ -840,12 +870,22 @@ storagePoolDestroy(virStoragePoolPtr obj)
> if ((backend = virStorageBackendForType(pool->def->type)) == NULL)
> goto cleanup;
>
> + VIR_INFO("Destroying storage pool '%s'", pool->def->name);
> +
> if (!virStoragePoolObjIsActive(pool)) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> _("storage pool '%s' is not active"), pool->def->name);
> goto cleanup;
> }
>
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name,
> + ".xml")))
> + goto cleanup;
> +
> + unlink(stateFile);
> + VIR_FREE(stateFile);
> +
Perhaps should occur after the next check
> if (pool->asyncjobs > 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("pool '%s' has asynchronous jobs running."),
> @@ -860,7 +900,6 @@ storagePoolDestroy(virStoragePoolPtr obj)
> virStoragePoolObjClearVols(pool);
>
> pool->active = 0;
> - VIR_INFO("Shutting down storage pool '%s'", pool->def->name);
>
> if (pool->configFile == NULL) {
> virStoragePoolObjRemove(&driver->pools, pool);
> @@ -870,6 +909,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
> pool->def = pool->newDef;
> pool->newDef = NULL;
> }
> +
> ret = 0;
>
> cleanup:
> @@ -885,6 +925,7 @@ storagePoolDelete(virStoragePoolPtr obj,
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + char *stateFile = NULL;
> int ret = -1;
>
> if (!(pool = virStoragePoolObjFromStoragePool(obj)))
> @@ -896,6 +937,8 @@ storagePoolDelete(virStoragePoolPtr obj,
> if ((backend = virStorageBackendForType(pool->def->type)) == NULL)
> goto cleanup;
>
> + VIR_INFO("Deleting storage pool '%s'", pool->def->name);
> +
> if (virStoragePoolObjIsActive(pool)) {
> virReportError(VIR_ERR_OPERATION_INVALID,
> _("storage pool '%s' is still active"),
> @@ -903,6 +946,14 @@ storagePoolDelete(virStoragePoolPtr obj,
> goto cleanup;
> }
>
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name,
> + ".xml")))
> + goto cleanup;
> +
> + unlink(stateFile);
> + VIR_FREE(stateFile);
> +
Perhaps should occur after the next check...
I think these are easily resolvable, but a v3 (sic) should be seen
John
> if (pool->asyncjobs > 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("pool '%s' has asynchronous jobs running."),
> @@ -917,7 +968,7 @@ storagePoolDelete(virStoragePoolPtr obj,
> }
> if (backend->deletePool(obj->conn, pool, flags) < 0)
> goto cleanup;
> - VIR_INFO("Deleting storage pool '%s'", pool->def->name);
> +
> ret = 0;
>
> cleanup:
>
More information about the libvir-list
mailing list