7:03 a.m.
Peter Krempa (3):
storage: backend: Use correct stringifier for pool type
storage: driver: Split out code fixing pool state after deactivation
storage: driver: Remove unavailable transient pools after restart
src/storage/storage_backend.c | 2 +-
src/storage/storage_driver.c | 68 ++++++++++++++++++++++++++-----------------
2 files changed, 42 insertions(+), 28 deletions(-)
--
2.12.1
7:03 a.m.
New subject: [libvirt] [PATCH 1/3] storage: backend: Use correct stringifier for pool type
When registering a storage poll backend, the code would use
virStorageTypeToString instead of virStoragePoolTypeToString. The
following message would be logged:
virDriverLoadModuleFunc:71 : Lookup function 'virStorageBackendSCSIRegister'
virStorageBackendRegister:174 : Registering storage backend '(null)'
---
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ce278b99c..7f892df7e 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -171,7 +171,7 @@ int
virStorageBackendRegister(virStorageBackendPtr backend)
{
VIR_DEBUG("Registering storage backend '%s'",
- virStorageTypeToString(backend->type));
+ NULLSTR(virStoragePoolTypeToString(backend->type)));
if (virStorageBackendsCount >= VIR_STORAGE_BACKENDS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.12.1
7:48 a.m.
New subject: [libvirt] [PATCH 1/3] storage: backend: Use correct stringifier for pool type
On Thu, Mar 30, 2017 at 02:03:45PM +0200, Peter Krempa wrote:
When registering a storage poll backend, the code would use
virStorageTypeToString instead of virStoragePoolTypeToString. The
following message would be logged:
virDriverLoadModuleFunc:71 : Lookup function 'virStorageBackendSCSIRegister'
virStorageBackendRegister:174 : Registering storage backend '(null)'
---
src/storage/storage_backend.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index ce278b99c..7f892df7e 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -171,7 +171,7 @@ int
virStorageBackendRegister(virStorageBackendPtr backend)
{
VIR_DEBUG("Registering storage backend '%s'",
- virStorageTypeToString(backend->type));
+ NULLSTR(virStoragePoolTypeToString(backend->type)));
The NULLSTR is unnecessary, since this is only being called from the backends
themselves and each backend sets the its type appropriately.
if (virStorageBackendsCount >= VIR_STORAGE_BACKENDS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR,
^This virReportError also uses the incorrect stringifier and needs to be fixed.
ACK with adjustments.
Erik
--
2.12.1
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:03 a.m.
New subject: [libvirt] [PATCH 2/3] storage: driver: Split out code fixing pool state after deactivation
After a pool is made inactive the definition objects need to be updated
(if a new definition is prepared) and transient pools need to be
completely removed. Split out the code doing these steps into a separate
function for later reuse.
---
src/storage/storage_driver.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 61c5e7eff..618b640a2 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -77,6 +77,31 @@ static void storageDriverUnlock(void)
virMutexUnlock(&driver->lock);
}
+
+/**
+ * virStoragePoolUpdateInactive:
+ * @poolptr: pointer to a variable holding the pool object pointer
+ *
+ * This function is supposed to be called after a pool becomes inactive. The
+ * function switches to the new config object for persistent pools. Inactive
+ * pools are removed.
+ */
+static void
+virStoragePoolUpdateInactive(virStoragePoolObjPtr *poolptr)
+{
+ virStoragePoolObjPtr pool = *poolptr;
+
+ if (pool->configFile == NULL) {
+ virStoragePoolObjRemove(&driver->pools, pool);
+ *poolptr = NULL;
+ } else if (pool->newDef) {
+ virStoragePoolDefFree(pool->def);
+ pool->def = pool->newDef;
+ pool->newDef = NULL;
+ }
+}
+
+
static void
storagePoolUpdateState(virStoragePoolObjPtr pool)
{
@@ -1071,14 +1096,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
pool->active = false;
- if (pool->configFile == NULL) {
- virStoragePoolObjRemove(&driver->pools, pool);
- pool = NULL;
- } else if (pool->newDef) {
- virStoragePoolDefFree(pool->def);
- pool->def = pool->newDef;
- pool->newDef = NULL;
- }
+ virStoragePoolUpdateInactive(&pool);
ret = 0;
@@ -1200,10 +1218,8 @@ storagePoolRefresh(virStoragePoolPtr obj,
0);
pool->active = false;
- if (pool->configFile == NULL) {
- virStoragePoolObjRemove(&driver->pools, pool);
- pool = NULL;
- }
+ virStoragePoolUpdateInactive(&pool);
+
goto cleanup;
}
--
2.12.1
7:03 a.m.
New subject: [libvirt] [PATCH 3/3] storage: driver: Remove unavailable transient pools after restart
If a transient storage pool is deemed inactive after libvirtd restart it
would not be deleted from the list. Reuse virStoragePoolUpdateInactive
along with a refactor necessary to properly update the state.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1242801
---
src/storage/storage_driver.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 618b640a2..fea769887 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -105,31 +105,28 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *poolptr)
static void
storagePoolUpdateState(virStoragePoolObjPtr pool)
{
- bool active;
+ bool active = false;
virStorageBackendPtr backend;
- int ret = -1;
char *stateFile;
if (!(stateFile = virFileBuildPath(driver->stateDir,
pool->def->name, ".xml")))
- goto error;
+ goto cleanup;
if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Missing backend %d"), pool->def->type);
- goto error;
+ goto cleanup;
}
/* Backends which do not support 'checkPool' are considered
- * inactive by default.
- */
- active = false;
+ * inactive by default. */
if (backend->checkPool &&
backend->checkPool(pool, &active) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to initialize storage pool '%s':
%s"),
pool->def->name, virGetLastErrorMessage());
- goto error;
+ active = false;
}
/* We can pass NULL as connection, most backends do not use
@@ -144,17 +141,18 @@ storagePoolUpdateState(virStoragePoolObjPtr pool)
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to restart storage pool '%s':
%s"),
pool->def->name, virGetLastErrorMessage());
- goto error;
+ active = false;
}
}
pool->active = active;
- ret = 0;
- error:
- if (ret < 0) {
- if (stateFile)
- unlink(stateFile);
- }
+
+ if (!pool->active)
+ virStoragePoolUpdateInactive(&pool);
+
+ cleanup:
+ if (!active && stateFile)
+ ignore_value(unlink(stateFile));
VIR_FREE(stateFile);
return;
--
2.12.1
9:01 a.m.
On Thu, Mar 30, 2017 at 02:03:44PM +0200, Peter Krempa wrote:
Peter Krempa (3):
storage: backend: Use correct stringifier for pool type
storage: driver: Split out code fixing pool state after deactivation
storage: driver: Remove unavailable transient pools after restart
src/storage/storage_backend.c | 2 +-
src/storage/storage_driver.c | 68 ++++++++++++++++++++++++++-----------------
2 files changed, 42 insertions(+), 28 deletions(-)
--
2.12.1
ACK series, just see my comment on 1/3.
Erik
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2824
days inactive
2824
days old
5 comments
2 participants
participants (2)
-
Erik Skultety -
Peter Krempa