9:44 a.m.
First patch fixes a regression introduced with pool state patches
Last 3 patches help fix other issues with the state tracking
Cole Robinson (4):
storage: Fix autostart dir for qemu:///session
storage: Don't leave stale state file if pool startup fails
storage: Break out storageDriverLoadPoolState
storage: If driver startup state syncing fails, delete statefile
src/storage/storage_driver.c | 109 ++++++++++++++++++++++++++-----------------
1 file changed, 67 insertions(+), 42 deletions(-)
--
2.3.6
9:44 a.m.
New subject: [libvirt] [PATCH 1/4] storage: Fix autostart dir for qemu:///session
---
src/storage/storage_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 306d98e..3b7746b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -237,7 +237,7 @@ storageStateInitialize(bool privileged,
if ((virAsprintf(&driver->configDir,
"%s/storage", configdir) < 0) ||
(virAsprintf(&driver->autostartDir,
- "%s/storage", configdir) < 0) ||
+ "%s/storage/autostart", configdir) < 0) ||
(virAsprintf(&driver->stateDir,
"%s/storage/run", rundir) < 0))
goto error;
--
2.3.6
9:44 a.m.
New subject: [libvirt] [PATCH 2/4] storage: Don't leave stale state file if pool startup fails
After pool startup we call refreshPool(). If that fails, we leave
a stale pool state file hanging around.
Hit this trying to create a pool with qemu:///session containing
root owned files.
---
src/storage/storage_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 3b7746b..06686bf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -180,6 +180,8 @@ storageDriverAutostart(void)
virStoragePoolSaveState(stateFile, pool->def) < 0 ||
backend->refreshPool(conn, pool) < 0) {
virErrorPtr err = virGetLastError();
+ if (stateFile)
+ unlink(stateFile);
if (backend->stopPool)
backend->stopPool(conn, pool);
VIR_ERROR(_("Failed to autostart storage pool '%s':
%s"),
@@ -690,6 +692,8 @@ storagePoolCreateXML(virConnectPtr conn,
if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
backend->refreshPool(conn, pool) < 0) {
+ if (stateFile)
+ unlink(stateFile);
if (backend->stopPool)
backend->stopPool(conn, pool);
virStoragePoolObjRemove(&driver->pools, pool);
@@ -856,6 +860,8 @@ storagePoolCreate(virStoragePoolPtr obj,
if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
backend->refreshPool(obj->conn, pool) < 0) {
+ if (stateFile)
+ unlink(stateFile);
if (backend->stopPool)
backend->stopPool(obj->conn, pool);
goto cleanup;
--
2.3.6
9:44 a.m.
New subject: [libvirt] [PATCH 3/4] storage: Break out storageDriverLoadPoolState
Will simplify a future patch
---
src/storage/storage_driver.c | 88 +++++++++++++++++++++++---------------------
1 file changed, 47 insertions(+), 41 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 06686bf..9abdc68 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -75,14 +75,58 @@ static void storageDriverUnlock(void)
}
static void
+storagePoolUpdateState(virStoragePoolObjPtr pool)
+{
+ bool active;
+ virStorageBackendPtr backend;
+
+ if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
+ VIR_ERROR(_("Missing backend %d"), pool->def->type);
+ goto error;
+ }
+
+ /* Backends which do not support 'checkPool' are considered
+ * inactive by default.
+ */
+ active = false;
+ if (backend->checkPool &&
+ backend->checkPool(pool, &active) < 0) {
+ virErrorPtr err = virGetLastError();
+ VIR_ERROR(_("Failed to initialize storage pool '%s': %s"),
+ pool->def->name, err ? err->message :
+ _("no error message found"));
+ goto error;
+ }
+
+ /* We can pass NULL as connection, most backends do not use
+ * it anyway, but if they do and fail, we want to log error and
+ * continue with other pools.
+ */
+ if (active) {
+ virStoragePoolObjClearVols(pool);
+ if (backend->refreshPool(NULL, pool) < 0) {
+ virErrorPtr err = virGetLastError();
+ if (backend->stopPool)
+ backend->stopPool(NULL, pool);
+ VIR_ERROR(_("Failed to restart storage pool '%s': %s"),
+ pool->def->name, err ? err->message :
+ _("no error message found"));
+ goto error;
+ }
+ }
+
+ pool->active = active;
+ error:
+ return;
+}
+
+static void
storagePoolUpdateAllState(void)
{
size_t i;
- bool active;
for (i = 0; i < driver->pools.count; i++) {
virStoragePoolObjPtr pool = driver->pools.objs[i];
- virStorageBackendPtr backend;
virStoragePoolObjLock(pool);
if (!virStoragePoolObjIsActive(pool)) {
@@ -90,45 +134,7 @@ storagePoolUpdateAllState(void)
continue;
}
- if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
- VIR_ERROR(_("Missing backend %d"), pool->def->type);
- virStoragePoolObjUnlock(pool);
- continue;
- }
-
- /* Backends which do not support 'checkPool' are considered
- * inactive by default.
- */
- active = false;
- if (backend->checkPool &&
- backend->checkPool(pool, &active) < 0) {
- virErrorPtr err = virGetLastError();
- VIR_ERROR(_("Failed to initialize storage pool '%s': %s"),
- pool->def->name, err ? err->message :
- _("no error message found"));
- virStoragePoolObjUnlock(pool);
- continue;
- }
-
- /* We can pass NULL as connection, most backends do not use
- * it anyway, but if they do and fail, we want to log error and
- * continue with other pools.
- */
- if (active) {
- virStoragePoolObjClearVols(pool);
- if (backend->refreshPool(NULL, pool) < 0) {
- virErrorPtr err = virGetLastError();
- if (backend->stopPool)
- backend->stopPool(NULL, pool);
- VIR_ERROR(_("Failed to restart storage pool '%s':
%s"),
- pool->def->name, err ? err->message :
- _("no error message found"));
- virStoragePoolObjUnlock(pool);
- continue;
- }
- }
-
- pool->active = active;
+ storagePoolUpdateState(pool);
virStoragePoolObjUnlock(pool);
}
}
--
2.3.6
9:44 a.m.
New subject: [libvirt] [PATCH 4/4] storage: If driver startup state syncing fails, delete statefile
If you end up with a state file for a pool that no longer starts up
or refreshes correctly, the state file is never removed and adds
noise to the logs everytime libvirtd is started.
If the initial state syncing fails, delete the statefile.
---
src/storage/storage_driver.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9abdc68..ac4a74a 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -79,6 +79,12 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
{
bool active;
virStorageBackendPtr backend;
+ int ret = -1;
+ char *stateFile;
+
+ if (!(stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml")))
+ goto error;
if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
VIR_ERROR(_("Missing backend %d"), pool->def->type);
@@ -116,7 +122,14 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
}
pool->active = active;
+ ret = 0;
error:
+ if (ret < 0) {
+ if (stateFile)
+ unlink(stateFile);
+ }
+ VIR_FREE(stateFile);
+
return;
}
--
2.3.6
7:07 a.m.
On Mon, Apr 27, 2015 at 10:44:54AM -0400, Cole Robinson wrote:
First patch fixes a regression introduced with pool state patches
Last 3 patches help fix other issues with the state tracking
Cole Robinson (4):
storage: Fix autostart dir for qemu:///session
storage: Don't leave stale state file if pool startup fails
storage: Break out storageDriverLoadPoolState
storage: If driver startup state syncing fails, delete statefile
src/storage/storage_driver.c | 109 ++++++++++++++++++++++++++-----------------
1 file changed, 67 insertions(+), 42 deletions(-)
ACK series
Jan
8:40 a.m.
On 04/28/2015 08:07 AM, Ján Tomko wrote:
On Mon, Apr 27, 2015 at 10:44:54AM -0400, Cole Robinson wrote:
> First patch fixes a regression introduced with pool state patches
> Last 3 patches help fix other issues with the state tracking
>
> Cole Robinson (4):
> storage: Fix autostart dir for qemu:///session
> storage: Don't leave stale state file if pool startup fails
> storage: Break out storageDriverLoadPoolState
> storage: If driver startup state syncing fails, delete statefile
>
> src/storage/storage_driver.c | 109 ++++++++++++++++++++++++++-----------------
> 1 file changed, 67 insertions(+), 42 deletions(-)
>
ACK series
Thanks, pushed now
- Cole
3496
days inactive
3497
days old
6 comments
2 participants
participants (2)
-
Cole Robinson -
Ján Tomko