5:10 a.m.
Patches add necessary changes to add support for storage pool state XMLs
and thus resolve https://bugzilla.redhat.com/show_bug.cgi?id=1177733
Erik Skultety (6):
conf: Introduce virStoragePoolDefFormatBuf
conf: Introduce virStoragePoolSaveXML
conf: Introduce virStoragePoolSaveState
storage: Add support for storage pool state XML
conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
storage: Introduce storagePoolUpdateAllState function
src/conf/storage_conf.c | 233 ++++++++++++++++++++++++++++++++++++-------
src/conf/storage_conf.h | 12 ++-
src/libvirt_private.syms | 2 +
src/storage/storage_driver.c | 188 +++++++++++++++++++++++++++-------
4 files changed, 358 insertions(+), 77 deletions(-)
--
1.9.3
5:10 a.m.
New subject: [libvirt] [PATCH 1/6] conf: Introduce virStoragePoolDefFormatBuf
When modifying config/status XML, it might be handy to include some
additional XML elements (e.g. <poolstatus>). In order to do so,
introduce new formatting function virStoragePoolDefFormatBuf and make
virStoragePoolDefFormat call it.
---
src/conf/storage_conf.c | 78 +++++++++++++++++++++++++++++--------------------
1 file changed, 46 insertions(+), 32 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index b070448..a8e9876 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf,
}
-char *
-virStoragePoolDefFormat(virStoragePoolDefPtr def)
+static int
+virStoragePoolDefFormatBuf(virBufferPtr buf,
+ virStoragePoolDefPtr def)
{
virStoragePoolOptionsPtr options;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
- const char *type;
char uuid[VIR_UUID_STRING_BUFLEN];
+ const char *type;
options = virStoragePoolOptionsForPoolType(def->type);
if (options == NULL)
- return NULL;
+ goto error;
type = virStoragePoolTypeToString(def->type);
if (!type) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("unexpected pool type"));
- goto cleanup;
+ goto error;
}
- virBufferAsprintf(&buf, "<pool type='%s'>\n", type);
- virBufferAdjustIndent(&buf, 2);
- virBufferEscapeString(&buf, "<name>%s</name>\n",
def->name);
+ virBufferAsprintf(buf, "<pool type='%s'>\n", type);
+ virBufferAdjustIndent(buf, 2);
+ virBufferEscapeString(buf, "<name>%s</name>\n", def->name);
virUUIDFormat(def->uuid, uuid);
- virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuid);
+ virBufferAsprintf(buf, "<uuid>%s</uuid>\n", uuid);
- virBufferAsprintf(&buf, "<capacity
unit='bytes'>%llu</capacity>\n",
+ virBufferAsprintf(buf, "<capacity
unit='bytes'>%llu</capacity>\n",
def->capacity);
- virBufferAsprintf(&buf, "<allocation
unit='bytes'>%llu</allocation>\n",
+ virBufferAsprintf(buf, "<allocation
unit='bytes'>%llu</allocation>\n",
def->allocation);
- virBufferAsprintf(&buf, "<available
unit='bytes'>%llu</available>\n",
+ virBufferAsprintf(buf, "<available
unit='bytes'>%llu</available>\n",
def->available);
- if (virStoragePoolSourceFormat(&buf, options, &def->source) < 0)
- goto cleanup;
+ if (virStoragePoolSourceFormat(buf, options, &def->source) < 0)
+ goto error;
/* RBD, Sheepdog, and Gluster devices are not local block devs nor
* files, so they don't have a target */
if (def->type != VIR_STORAGE_POOL_RBD &&
def->type != VIR_STORAGE_POOL_SHEEPDOG &&
def->type != VIR_STORAGE_POOL_GLUSTER) {
- virBufferAddLit(&buf, "<target>\n");
- virBufferAdjustIndent(&buf, 2);
+ virBufferAddLit(buf, "<target>\n");
+ virBufferAdjustIndent(buf, 2);
- virBufferEscapeString(&buf, "<path>%s</path>\n",
def->target.path);
+ virBufferEscapeString(buf, "<path>%s</path>\n",
def->target.path);
- virBufferAddLit(&buf, "<permissions>\n");
- virBufferAdjustIndent(&buf, 2);
- virBufferAsprintf(&buf, "<mode>0%o</mode>\n",
+ virBufferAddLit(buf, "<permissions>\n");
+ virBufferAdjustIndent(buf, 2);
+ virBufferAsprintf(buf, "<mode>0%o</mode>\n",
def->target.perms.mode);
- virBufferAsprintf(&buf, "<owner>%d</owner>\n",
+ virBufferAsprintf(buf, "<owner>%d</owner>\n",
(int) def->target.perms.uid);
- virBufferAsprintf(&buf, "<group>%d</group>\n",
+ virBufferAsprintf(buf, "<group>%d</group>\n",
(int) def->target.perms.gid);
- virBufferEscapeString(&buf, "<label>%s</label>\n",
+ virBufferEscapeString(buf, "<label>%s</label>\n",
def->target.perms.label);
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</permissions>\n");
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</target>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</permissions>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</target>\n");
}
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</pool>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</pool>\n");
+
+ return 0;
+
+ error:
+ return -1;
+}
+
+char *
+virStoragePoolDefFormat(virStoragePoolDefPtr def)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (virStoragePoolDefFormatBuf(&buf, def) < 0)
+ goto error;
if (virBufferCheckError(&buf) < 0)
- goto cleanup;
+ goto error;
return virBufferContentAndReset(&buf);
- cleanup:
+ error:
virBufferFreeAndReset(&buf);
return NULL;
}
--
1.9.3
3:07 a.m.
New subject: [libvirt] [PATCH 1/6] conf: Introduce virStoragePoolDefFormatBuf
On Thu, Apr 02, 2015 at 12:10:35PM +0200, Erik Skultety wrote:
When modifying config/status XML, it might be handy to include some
additional XML elements (e.g. <poolstatus>). In order to do so,
introduce new formatting function virStoragePoolDefFormatBuf and make
virStoragePoolDefFormat call it.
---
src/conf/storage_conf.c | 78 +++++++++++++++++++++++++++++--------------------
1 file changed, 46 insertions(+), 32 deletions(-)
ACK with one nit:
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index b070448..a8e9876 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf,
--- 8< ---
- virBufferAdjustIndent(&buf, -2);
- virBufferAddLit(&buf, "</pool>\n");
+ virBufferAdjustIndent(buf, -2);
+ virBufferAddLit(buf, "</pool>\n");
+
+ return 0;
+
+ error:
+ return -1;
The error label is not necessary - just return -1; directly (unless you
plan to add some cleanup code there in the near future).
Jan
3:31 a.m.
New subject: [libvirt] [PATCH 1/6] conf: Introduce virStoragePoolDefFormatBuf
ACK with one nit:
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index b070448..a8e9876 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1150,76 +1150,90 @@ virStoragePoolSourceFormat(virBufferPtr buf,
--- 8< ---
> - virBufferAdjustIndent(&buf, -2);
> - virBufferAddLit(&buf, "</pool>\n");
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</pool>\n");
> +
> + return 0;
> +
> + error:
> + return -1;
The error label is not necessary - just return -1; directly (unless you
plan to add some cleanup code there in the near future).
Jan
Thanks, now pushed.
Erik
5:10 a.m.
New subject: [libvirt] [PATCH 2/6] conf: Introduce virStoragePoolSaveXML
Make XML definition saving more generic by moving the common code into
virStoragePoolSaveXML and leave case specific code to
PoolSave{Status,Config,...} functions.
---
src/conf/storage_conf.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index a8e9876..73b937e 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1913,11 +1913,25 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
return ret;
}
+
+static int virStoragePoolSaveXML(const char *path,
+ virStoragePoolDefPtr def,
+ const char *xml)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ int ret = -1;
+
+ virUUIDFormat(def->uuid, uuidstr);
+ ret = virXMLSaveFile(path,
+ virXMLPickShellSafeComment(def->name, uuidstr),
+ "pool-edit", xml);
+
+ return ret;
+}
int
virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
char *xml;
int ret = -1;
@@ -1927,12 +1941,12 @@ virStoragePoolSaveConfig(const char *configFile,
return -1;
}
- virUUIDFormat(def->uuid, uuidstr);
- ret = virXMLSaveFile(configFile,
- virXMLPickShellSafeComment(def->name, uuidstr),
- "pool-edit", xml);
- VIR_FREE(xml);
+ if (virStoragePoolSaveXML(configFile, def, xml))
+ goto cleanup;
+ ret = 0;
+ cleanup:
+ VIR_FREE(xml);
return ret;
}
--
1.9.3
6:34 a.m.
New subject: [libvirt] [PATCH 2/6] conf: Introduce virStoragePoolSaveXML
On Thu, Apr 02, 2015 at 12:10:36 +0200, Erik Skultety wrote:
Make XML definition saving more generic by moving the common code
into
virStoragePoolSaveXML and leave case specific code to
PoolSave{Status,Config,...} functions.
---
src/conf/storage_conf.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index a8e9876..73b937e 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1913,11 +1913,25 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
return ret;
}
+
+static int virStoragePoolSaveXML(const char *path,
+ virStoragePoolDefPtr def,
+ const char *xml)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ int ret = -1;
+
+ virUUIDFormat(def->uuid, uuidstr);
+ ret = virXMLSaveFile(path,
+ virXMLPickShellSafeComment(def->name, uuidstr),
+ "pool-edit", xml);
+
+ return ret;
+}
int
Missing empty lines between functions.
virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
char *xml;
int ret = -1;
@@ -1927,12 +1941,12 @@ virStoragePoolSaveConfig(const char *configFile,
return -1;
}
- virUUIDFormat(def->uuid, uuidstr);
- ret = virXMLSaveFile(configFile,
- virXMLPickShellSafeComment(def->name, uuidstr),
- "pool-edit", xml);
- VIR_FREE(xml);
+ if (virStoragePoolSaveXML(configFile, def, xml))
+ goto cleanup;
+ ret = 0;
+ cleanup:
+ VIR_FREE(xml);
return ret;
}
ACK if you add at least one new line between virStoragePoolSaveConfig
and virStoragePoolSaveXML
Peter
5:10 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
Properly format storage pool state XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.c | 35 +++++++++++++++++++++++++++++++++++
src/conf/storage_conf.h | 4 +++-
src/libvirt_private.syms | 1 +
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 73b937e..ee564f2 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path,
return ret;
}
+
+
+int virStoragePoolSaveState(const char *stateFile,
+ virStoragePoolDefPtr def)
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
+ char *xml;
+
+ virBufferAddLit(&buf, "<poolstatus>\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ if (virStoragePoolDefFormatBuf(&buf, def) < 0)
+ goto error;
+
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</poolstatus>\n");
+
+ if (virBufferCheckError(&buf) < 0)
+ goto error;
+
+ if (!(xml = virBufferContentAndReset(&buf)))
+ goto error;
+
+ if (virStoragePoolSaveXML(stateFile, def, xml))
+ goto error;
+
+ ret = 0;
+
+ error:
+ VIR_FREE(xml);
+ return ret;
+}
+
+
int
virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 4584075..da378b7 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -371,7 +371,9 @@ virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
-int virStoragePoolSaveConfig(const char *configDir,
+int virStoragePoolSaveState(const char *stateFile,
+ virStoragePoolDefPtr def);
+int virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def);
int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
virStoragePoolObjPtr pool,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9e71b1a..56acb01 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -813,6 +813,7 @@ virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjUnlock;
virStoragePoolSaveConfig;
+virStoragePoolSaveState;
virStoragePoolSourceAdapterTypeFromString;
virStoragePoolSourceAdapterTypeToString;
virStoragePoolSourceClear;
--
1.9.3
9:14 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
On 04/02/2015 06:10 AM, Erik Skultety wrote:
Properly format storage pool state XML.
Your previous patch 3/7 had a better commit message:
Introduce virStoragePoolSaveStatus to properly format the status XML in
the same manner as virStoragePoolDefFormat, except for adding a
<poolstatus> ... </poolstatus> around the definition. This is similar to
virNetworkObjFormat used to save the live/active network information.
By itself - no it doesn't solve the bug (same for 4 and 5)
---
src/conf/storage_conf.c | 35 +++++++++++++++++++++++++++++++++++
src/conf/storage_conf.h | 4 +++-
src/libvirt_private.syms | 1 +
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 73b937e..ee564f2 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path,
return ret;
}
+
+
+int virStoragePoolSaveState(const char *stateFile,
+ virStoragePoolDefPtr def)
Again it's
int
virStorage...
ACK with those adjustments.
John
FYI: Coverity is happy with all 6 patches...
+{
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+ int ret = -1;
+ char *xml;
+
+ virBufferAddLit(&buf, "<poolstatus>\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ if (virStoragePoolDefFormatBuf(&buf, def) < 0)
+ goto error;
+
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</poolstatus>\n");
+
+ if (virBufferCheckError(&buf) < 0)
+ goto error;
+
+ if (!(xml = virBufferContentAndReset(&buf)))
+ goto error;
+
+ if (virStoragePoolSaveXML(stateFile, def, xml))
+ goto error;
+
+ ret = 0;
+
+ error:
+ VIR_FREE(xml);
+ return ret;
+}
+
+
int
virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 4584075..da378b7 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -371,7 +371,9 @@ virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
-int virStoragePoolSaveConfig(const char *configDir,
+int virStoragePoolSaveState(const char *stateFile,
+ virStoragePoolDefPtr def);
+int virStoragePoolSaveConfig(const char *configFile,
virStoragePoolDefPtr def);
int virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
virStoragePoolObjPtr pool,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9e71b1a..56acb01 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -813,6 +813,7 @@ virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjUnlock;
virStoragePoolSaveConfig;
+virStoragePoolSaveState;
virStoragePoolSourceAdapterTypeFromString;
virStoragePoolSourceAdapterTypeToString;
virStoragePoolSourceClear;
3:39 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
Your previous patch 3/7 had a better commit message:
Introduce virStoragePoolSaveStatus to properly format the status XML in
the same manner as virStoragePoolDefFormat, except for adding a
<poolstatus> ... </poolstatus> around the definition. This is similar to
virNetworkObjFormat used to save the live/active network information.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
By itself - no it doesn't solve the bug (same for 4 and 5)
> ---
> src/conf/storage_conf.c | 35 +++++++++++++++++++++++++++++++++++
> src/conf/storage_conf.h | 4 +++-
> src/libvirt_private.syms | 1 +
> 3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 73b937e..ee564f2 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1928,6 +1928,41 @@ static int virStoragePoolSaveXML(const char *path,
>
> return ret;
> }
> +
> +
> +int virStoragePoolSaveState(const char *stateFile,
> + virStoragePoolDefPtr def)
Again it's
int
virStorage...
ACK with those adjustments.
John
FYI: Coverity is happy with all 6 patches...
Thank you, I changed the commit message, fixed the issues and pushed
with Jan's little note in mind (I'll push that one as trivial).
Erik
3:17 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
On Thu, Apr 02, 2015 at 12:10:37PM +0200, Erik Skultety wrote:
Properly format storage pool state XML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.c | 35 +++++++++++++++++++++++++++++++++++
src/conf/storage_conf.h | 4 +++-
src/libvirt_private.syms | 1 +
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 4584075..da378b7 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -371,7 +371,9 @@ virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
-int virStoragePoolSaveConfig(const char *configDir,
This change from configDir
+int virStoragePoolSaveState(const char *stateFile,
+ virStoragePoolDefPtr def);
+int virStoragePoolSaveConfig(const char *configFile,
to configFile should be in a separate commit.
(You can just push it under the trivial rule)
Jan
3:52 a.m.
New subject: [libvirt] [PATCH 3/6] conf: Introduce virStoragePoolSaveState
This change from configDir
> +int virStoragePoolSaveState(const char *stateFile,
> + virStoragePoolDefPtr def);
> +int virStoragePoolSaveConfig(const char *configFile,
to configFile should be in a separate commit.
(You can just push it under the trivial rule)
Jan
Thanks for notice, I sent a patch to list and pushed it as trivial.
Erik
5:10 a.m.
New subject: [libvirt] [PATCH 4/6] storage: Add support for storage pool state XML
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
---
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);
}
virStoragePoolObjUnlock(pool);
}
@@ -147,61 +155,67 @@ storageDriverAutostart(void)
/**
* virStorageStartup:
*
- * Initialization function for the QEmu daemon
+ * Initialization function for the Storage Driver
*/
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;
+
+ 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;
+
+ 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);
+
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);
+
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:
--
1.9.3
9:29 a.m.
New subject: [libvirt] [PATCH 4/6] storage: Add support for storage pool state XML
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:
6:03 a.m.
New subject: [libvirt] [PATCH v3] storage: Add support for storage pool state XML
This patch introduces new virStorageDriverState element stateDir.
Also adds necessary changes to storageStateInitialize, so that
directories initialization becomes more generic.
---
src/conf/storage_conf.h | 1 +
src/storage/storage_driver.c | 108 +++++++++++++++++++++++++++++++------------
2 files changed, 79 insertions(+), 30 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..b350ee6 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 */
@@ -126,7 +127,11 @@ storageDriverAutostart(void)
if (started) {
virStoragePoolObjClearVols(pool);
- if (backend->refreshPool(conn, pool) < 0) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+ if (!stateFile ||
+ virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ backend->refreshPool(conn, pool) < 0) {
virErrorPtr err = virGetLastError();
if (backend->stopPool)
backend->stopPool(conn, pool);
@@ -136,7 +141,9 @@ storageDriverAutostart(void)
virStoragePoolObjUnlock(pool);
continue;
}
+
pool->active = 1;
+ VIR_FREE(stateFile);
}
virStoragePoolObjUnlock(pool);
}
@@ -147,61 +154,67 @@ storageDriverAutostart(void)
/**
* virStorageStartup:
*
- * Initialization function for the QEmu daemon
+ * Initialization function for the Storage Driver
*/
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 +274,7 @@ storageStateCleanup(void)
VIR_FREE(driver->configDir);
VIR_FREE(driver->autostartDir);
+ VIR_FREE(driver->stateDir);
storageDriverUnlock();
virMutexDestroy(&driver->lock);
VIR_FREE(driver);
@@ -579,6 +593,7 @@ storagePoolCreateXML(virConnectPtr conn,
virStoragePoolObjPtr pool = NULL;
virStoragePoolPtr ret = NULL;
virStorageBackendPtr backend;
+ char *stateFile;
virCheckFlags(0, NULL);
@@ -609,7 +624,11 @@ storagePoolCreateXML(virConnectPtr conn,
goto cleanup;
}
- if (backend->refreshPool(conn, pool) < 0) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+
+ if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ backend->refreshPool(conn, pool) < 0) {
if (backend->stopPool)
backend->stopPool(conn, pool);
virStoragePoolObjRemove(&driver->pools, pool);
@@ -745,6 +764,7 @@ storagePoolCreate(virStoragePoolPtr obj,
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
int ret = -1;
+ char *stateFile = NULL;
virCheckFlags(0, -1);
@@ -763,21 +783,27 @@ 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) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+
+ if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ 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 +848,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+ char *stateFile = NULL;
int ret = -1;
storageDriverLock();
@@ -840,6 +867,8 @@ 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);
@@ -853,6 +882,14 @@ storagePoolDestroy(virStoragePoolPtr obj)
goto cleanup;
}
+ if (!(stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name,
+ ".xml")))
+ goto cleanup;
+
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+
if (backend->stopPool &&
backend->stopPool(obj->conn, pool) < 0)
goto cleanup;
@@ -860,7 +897,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 +906,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
pool->def = pool->newDef;
pool->newDef = NULL;
}
+
ret = 0;
cleanup:
@@ -885,6 +922,7 @@ storagePoolDelete(virStoragePoolPtr obj,
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+ char *stateFile = NULL;
int ret = -1;
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
@@ -896,6 +934,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"),
@@ -910,6 +950,14 @@ storagePoolDelete(virStoragePoolPtr obj,
goto cleanup;
}
+ if (!(stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name,
+ ".xml")))
+ goto cleanup;
+
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+
if (!backend->deletePool) {
virReportError(VIR_ERR_NO_SUPPORT,
"%s", _("pool does not support pool
deletion"));
@@ -917,7 +965,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:
--
1.9.3
8:55 a.m.
New subject: [libvirt] [PATCH v3] storage: Add support for storage pool state XML
On 04/03/2015 07:03 AM, Erik Skultety wrote:
This patch introduces new virStorageDriverState element stateDir.
Also adds necessary changes to storageStateInitialize, so that
directories initialization becomes more generic.
---
src/conf/storage_conf.h | 1 +
src/storage/storage_driver.c | 108 +++++++++++++++++++++++++++++++------------
2 files changed, 79 insertions(+), 30 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..b350ee6 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 */
@@ -126,7 +127,11 @@ storageDriverAutostart(void)
if (started) {
virStoragePoolObjClearVols(pool);
- if (backend->refreshPool(conn, pool) < 0) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+ if (!stateFile ||
+ virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ backend->refreshPool(conn, pool) < 0) {
virErrorPtr err = virGetLastError();
if (backend->stopPool)
backend->stopPool(conn, pool);
@@ -136,7 +141,9 @@ storageDriverAutostart(void)
virStoragePoolObjUnlock(pool);
Add a VIR_FREE(stateFile); before continue
continue;
}
+
pool->active = 1;
+ VIR_FREE(stateFile);
}
virStoragePoolObjUnlock(pool);
}
@@ -147,61 +154,67 @@ storageDriverAutostart(void)
/**
* virStorageStartup:
*
- * Initialization function for the QEmu daemon
+ * Initialization function for the Storage Driver
*/
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 +274,7 @@ storageStateCleanup(void)
VIR_FREE(driver->configDir);
VIR_FREE(driver->autostartDir);
+ VIR_FREE(driver->stateDir);
storageDriverUnlock();
virMutexDestroy(&driver->lock);
VIR_FREE(driver);
@@ -579,6 +593,7 @@ storagePoolCreateXML(virConnectPtr conn,
virStoragePoolObjPtr pool = NULL;
virStoragePoolPtr ret = NULL;
virStorageBackendPtr backend;
+ char *stateFile;
s/;/ = NULL;
virCheckFlags(0, NULL);
@@ -609,7 +624,11 @@ storagePoolCreateXML(virConnectPtr conn,
goto cleanup;
}
- if (backend->refreshPool(conn, pool) < 0) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+
+ if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ backend->refreshPool(conn, pool) < 0) {
if (backend->stopPool)
backend->stopPool(conn, pool);
virStoragePoolObjRemove(&driver->pools, pool);
Then in the cleanup:
VIR_FREE(stateFile);
ACK - with the adjustments
John
@@ -745,6 +764,7 @@ storagePoolCreate(virStoragePoolPtr obj,
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
int ret = -1;
+ char *stateFile = NULL;
virCheckFlags(0, -1);
@@ -763,21 +783,27 @@ 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) {
+ stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name, ".xml");
+
+ if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
+ 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 +848,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+ char *stateFile = NULL;
int ret = -1;
storageDriverLock();
@@ -840,6 +867,8 @@ 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);
@@ -853,6 +882,14 @@ storagePoolDestroy(virStoragePoolPtr obj)
goto cleanup;
}
+ if (!(stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name,
+ ".xml")))
+ goto cleanup;
+
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+
if (backend->stopPool &&
backend->stopPool(obj->conn, pool) < 0)
goto cleanup;
@@ -860,7 +897,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 +906,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
pool->def = pool->newDef;
pool->newDef = NULL;
}
+
ret = 0;
cleanup:
@@ -885,6 +922,7 @@ storagePoolDelete(virStoragePoolPtr obj,
{
virStoragePoolObjPtr pool;
virStorageBackendPtr backend;
+ char *stateFile = NULL;
int ret = -1;
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
@@ -896,6 +934,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"),
@@ -910,6 +950,14 @@ storagePoolDelete(virStoragePoolPtr obj,
goto cleanup;
}
+ if (!(stateFile = virFileBuildPath(driver->stateDir,
+ pool->def->name,
+ ".xml")))
+ goto cleanup;
+
+ unlink(stateFile);
+ VIR_FREE(stateFile);
+
if (!backend->deletePool) {
virReportError(VIR_ERR_NO_SUPPORT,
"%s", _("pool does not support pool
deletion"));
@@ -917,7 +965,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:
9:28 a.m.
New subject: [libvirt] [PATCH v3] storage: Add support for storage pool state XML
Thanks, fixed and pushed with remaining ACKed patches 5 and 6.
Erik
On 04/07/2015 03:55 PM, John Ferlan wrote:
On 04/03/2015 07:03 AM, Erik Skultety wrote:
> This patch introduces new virStorageDriverState element stateDir.
> Also adds necessary changes to storageStateInitialize, so that
> directories initialization becomes more generic.
> ---
> src/conf/storage_conf.h | 1 +
> src/storage/storage_driver.c | 108 +++++++++++++++++++++++++++++++------------
> 2 files changed, 79 insertions(+), 30 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..b350ee6 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 */
> @@ -126,7 +127,11 @@ storageDriverAutostart(void)
>
> if (started) {
> virStoragePoolObjClearVols(pool);
> - if (backend->refreshPool(conn, pool) < 0) {
> + stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml");
> + if (!stateFile ||
> + virStoragePoolSaveState(stateFile, pool->def) < 0 ||
> + backend->refreshPool(conn, pool) < 0) {
> virErrorPtr err = virGetLastError();
> if (backend->stopPool)
> backend->stopPool(conn, pool);
> @@ -136,7 +141,9 @@ storageDriverAutostart(void)
> virStoragePoolObjUnlock(pool);
Add a VIR_FREE(stateFile); before continue
> continue;
> }
> +
> pool->active = 1;
> + VIR_FREE(stateFile);
> }
> virStoragePoolObjUnlock(pool);
> }
> @@ -147,61 +154,67 @@ storageDriverAutostart(void)
> /**
> * virStorageStartup:
> *
> - * Initialization function for the QEmu daemon
> + * Initialization function for the Storage Driver
> */
> 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 +274,7 @@ storageStateCleanup(void)
>
> VIR_FREE(driver->configDir);
> VIR_FREE(driver->autostartDir);
> + VIR_FREE(driver->stateDir);
> storageDriverUnlock();
> virMutexDestroy(&driver->lock);
> VIR_FREE(driver);
> @@ -579,6 +593,7 @@ storagePoolCreateXML(virConnectPtr conn,
> virStoragePoolObjPtr pool = NULL;
> virStoragePoolPtr ret = NULL;
> virStorageBackendPtr backend;
> + char *stateFile;
s/;/ = NULL;
>
> virCheckFlags(0, NULL);
>
> @@ -609,7 +624,11 @@ storagePoolCreateXML(virConnectPtr conn,
> goto cleanup;
> }
>
> - if (backend->refreshPool(conn, pool) < 0) {
> + stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml");
> +
> + if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
> + backend->refreshPool(conn, pool) < 0) {
> if (backend->stopPool)
> backend->stopPool(conn, pool);
> virStoragePoolObjRemove(&driver->pools, pool);
Then in the cleanup:
VIR_FREE(stateFile);
ACK - with the adjustments
John
> @@ -745,6 +764,7 @@ storagePoolCreate(virStoragePoolPtr obj,
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> int ret = -1;
> + char *stateFile = NULL;
>
> virCheckFlags(0, -1);
>
> @@ -763,21 +783,27 @@ 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) {
> + stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name, ".xml");
> +
> + if (!stateFile || virStoragePoolSaveState(stateFile, pool->def) < 0 ||
> + 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 +848,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + char *stateFile = NULL;
> int ret = -1;
>
> storageDriverLock();
> @@ -840,6 +867,8 @@ 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);
> @@ -853,6 +882,14 @@ storagePoolDestroy(virStoragePoolPtr obj)
> goto cleanup;
> }
>
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name,
> + ".xml")))
> + goto cleanup;
> +
> + unlink(stateFile);
> + VIR_FREE(stateFile);
> +
> if (backend->stopPool &&
> backend->stopPool(obj->conn, pool) < 0)
> goto cleanup;
> @@ -860,7 +897,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 +906,7 @@ storagePoolDestroy(virStoragePoolPtr obj)
> pool->def = pool->newDef;
> pool->newDef = NULL;
> }
> +
> ret = 0;
>
> cleanup:
> @@ -885,6 +922,7 @@ storagePoolDelete(virStoragePoolPtr obj,
> {
> virStoragePoolObjPtr pool;
> virStorageBackendPtr backend;
> + char *stateFile = NULL;
> int ret = -1;
>
> if (!(pool = virStoragePoolObjFromStoragePool(obj)))
> @@ -896,6 +934,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"),
> @@ -910,6 +950,14 @@ storagePoolDelete(virStoragePoolPtr obj,
> goto cleanup;
> }
>
> + if (!(stateFile = virFileBuildPath(driver->stateDir,
> + pool->def->name,
> + ".xml")))
> + goto cleanup;
> +
> + unlink(stateFile);
> + VIR_FREE(stateFile);
> +
> if (!backend->deletePool) {
> virReportError(VIR_ERR_NO_SUPPORT,
> "%s", _("pool does not support pool
deletion"));
> @@ -917,7 +965,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:
>
5:10 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
These functions operate exactly the same as their network equivalents
virNetworkLoadAllState, virNetworkLoadState.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/conf/storage_conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
src/conf/storage_conf.h | 7 ++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 13 ++++++
4 files changed, 115 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ee564f2..e7d6e6b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1863,6 +1863,100 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
}
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+ const char *stateDir,
+ const char *name)
+{
+ char *stateFile = NULL;
+ virStoragePoolDefPtr def = NULL;
+ virStoragePoolObjPtr pool = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr node = NULL;
+
+ if (!(stateFile = virFileBuildPath(stateDir, name, ".xml")))
+ goto error;
+
+ if (!(xml = virXMLParseCtxt(stateFile, NULL, _("(pool state)"),
&ctxt)))
+ goto error;
+
+ if (!(node = virXPathNode("//pool", ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not find any 'pool' element in state
file"));
+ goto error;
+ }
+
+ ctxt->node = node;
+ if (!(def = virStoragePoolDefParseXML(ctxt)))
+ goto error;
+
+ if (!STREQ(name, def->name)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Storage pool state file '%s' does not match
"
+ "pool name '%s'"),
+ stateFile, def->name);
+ goto error;
+ }
+
+ /* create the object */
+ if (!(pool = virStoragePoolObjAssignDef(pools, def)))
+ goto error;
+
+ /* XXX: future handling of some additional usefull status data,
+ * for now, if a status file for a pool exists, the pool will be marked
+ * as active
+ */
+
+ pool->active = 1;
+
+ cleanup:
+ VIR_FREE(stateFile);
+ xmlFree(xml);
+ xmlXPathFreeContext(ctxt);
+ return pool;
+
+ error:
+ virStoragePoolDefFree(def);
+ goto cleanup;
+}
+
+
+int
+virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir)
+{
+ DIR *dir;
+ struct dirent *entry;
+ int ret = -1;
+
+ if (!(dir = opendir(stateDir))) {
+ if (errno == ENOENT)
+ return 0;
+
+ virReportSystemError(errno, _("Failed to open dir '%s'"),
stateDir);
+ return -1;
+ }
+
+ while ((ret = virDirRead(dir, &entry, stateDir)) > 0) {
+ virStoragePoolObjPtr pool;
+
+ if (entry->d_name[0] == '.')
+ continue;
+
+ if (!virFileStripSuffix(entry->d_name, ".xml"))
+ continue;
+
+ if (!(pool = virStoragePoolLoadState(pools, stateDir, entry->d_name)))
+ continue;
+ virStoragePoolObjUnlock(pool);
+ }
+
+ closedir(dir);
+ return ret;
+}
+
+
int
virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
const char *configDir,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 8d43019..7471006 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -318,6 +318,13 @@ int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
const char *configDir,
const char *autostartDir);
+int virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir);
+
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+ const char *stateDir,
+ const char *name);
virStoragePoolObjPtr
virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools,
const unsigned char *uuid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 56acb01..6b95dea 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -800,6 +800,7 @@ virStoragePoolFormatFileSystemNetTypeToString;
virStoragePoolFormatFileSystemTypeToString;
virStoragePoolGetVhbaSCSIHostParent;
virStoragePoolLoadAllConfigs;
+virStoragePoolLoadAllState;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
virStoragePoolObjDeleteDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0180fd7..36c05b3 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -199,6 +199,17 @@ storageStateInitialize(bool privileged,
}
driver->privileged = privileged;
+ if (virFileMakePath(driver->stateDir) < 0) {
+ virReportError(errno,
+ _("cannot create directory %s"),
+ driver->stateDir);
+ goto error;
+ }
+
+ if (virStoragePoolLoadAllState(&driver->pools,
+ driver->stateDir) < 0)
+ goto error;
+
if (virStoragePoolLoadAllConfigs(&driver->pools,
driver->configDir,
driver->autostartDir) < 0)
@@ -247,6 +258,8 @@ storageStateReload(void)
return -1;
storageDriverLock();
+ virStoragePoolLoadAllState(&driver->pools,
+ driver->stateDir);
virStoragePoolLoadAllConfigs(&driver->pools,
driver->configDir,
driver->autostartDir);
--
1.9.3
9:40 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
On 04/02/2015 06:10 AM, Erik Skultety wrote:
These functions operate exactly the same as their network
equivalents
virNetworkLoadAllState, virNetworkLoadState.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
^^ Don't forget to remove...
---
src/conf/storage_conf.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
src/conf/storage_conf.h | 7 ++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 13 ++++++
4 files changed, 115 insertions(+)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index ee564f2..e7d6e6b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1863,6 +1863,100 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
}
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+ const char *stateDir,
+ const char *name)
+{
+ char *stateFile = NULL;
+ virStoragePoolDefPtr def = NULL;
+ virStoragePoolObjPtr pool = NULL;
+ xmlDocPtr xml = NULL;
+ xmlXPathContextPtr ctxt = NULL;
+ xmlNodePtr node = NULL;
+
+ if (!(stateFile = virFileBuildPath(stateDir, name, ".xml")))
+ goto error;
+
+ if (!(xml = virXMLParseCtxt(stateFile, NULL, _("(pool state)"),
&ctxt)))
+ goto error;
+
+ if (!(node = virXPathNode("//pool", ctxt))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Could not find any 'pool' element in state
file"));
+ goto error;
+ }
+
+ ctxt->node = node;
+ if (!(def = virStoragePoolDefParseXML(ctxt)))
+ goto error;
+
+ if (!STREQ(name, def->name)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Storage pool state file '%s' does not match
"
+ "pool name '%s'"),
+ stateFile, def->name);
+ goto error;
+ }
+
+ /* create the object */
+ if (!(pool = virStoragePoolObjAssignDef(pools, def)))
+ goto error;
+
+ /* XXX: future handling of some additional usefull status data,
s/usefull/useful
ACK with the slight adjustments.
If you want to post a v3 "inline"/as a response to patch 4 - that's fine
John
+ * for now, if a status file for a pool exists, the pool will be
marked
+ * as active
+ */
+
+ pool->active = 1;
+
+ cleanup:
+ VIR_FREE(stateFile);
+ xmlFree(xml);
+ xmlXPathFreeContext(ctxt);
+ return pool;
+
+ error:
+ virStoragePoolDefFree(def);
+ goto cleanup;
+}
+
+
+int
+virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir)
+{
+ DIR *dir;
+ struct dirent *entry;
+ int ret = -1;
+
+ if (!(dir = opendir(stateDir))) {
+ if (errno == ENOENT)
+ return 0;
+
+ virReportSystemError(errno, _("Failed to open dir '%s'"),
stateDir);
+ return -1;
+ }
+
+ while ((ret = virDirRead(dir, &entry, stateDir)) > 0) {
+ virStoragePoolObjPtr pool;
+
+ if (entry->d_name[0] == '.')
+ continue;
+
+ if (!virFileStripSuffix(entry->d_name, ".xml"))
+ continue;
+
+ if (!(pool = virStoragePoolLoadState(pools, stateDir, entry->d_name)))
+ continue;
+ virStoragePoolObjUnlock(pool);
+ }
+
+ closedir(dir);
+ return ret;
+}
+
+
int
virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
const char *configDir,
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 8d43019..7471006 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -318,6 +318,13 @@ int virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
const char *configDir,
const char *autostartDir);
+int virStoragePoolLoadAllState(virStoragePoolObjListPtr pools,
+ const char *stateDir);
+
+virStoragePoolObjPtr
+virStoragePoolLoadState(virStoragePoolObjListPtr pools,
+ const char *stateDir,
+ const char *name);
virStoragePoolObjPtr
virStoragePoolObjFindByUUID(virStoragePoolObjListPtr pools,
const unsigned char *uuid);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 56acb01..6b95dea 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -800,6 +800,7 @@ virStoragePoolFormatFileSystemNetTypeToString;
virStoragePoolFormatFileSystemTypeToString;
virStoragePoolGetVhbaSCSIHostParent;
virStoragePoolLoadAllConfigs;
+virStoragePoolLoadAllState;
virStoragePoolObjAssignDef;
virStoragePoolObjClearVols;
virStoragePoolObjDeleteDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0180fd7..36c05b3 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -199,6 +199,17 @@ storageStateInitialize(bool privileged,
}
driver->privileged = privileged;
+ if (virFileMakePath(driver->stateDir) < 0) {
+ virReportError(errno,
+ _("cannot create directory %s"),
+ driver->stateDir);
+ goto error;
+ }
+
+ if (virStoragePoolLoadAllState(&driver->pools,
+ driver->stateDir) < 0)
+ goto error;
+
if (virStoragePoolLoadAllConfigs(&driver->pools,
driver->configDir,
driver->autostartDir) < 0)
@@ -247,6 +258,8 @@ storageStateReload(void)
return -1;
storageDriverLock();
+ virStoragePoolLoadAllState(&driver->pools,
+ driver->stateDir);
virStoragePoolLoadAllConfigs(&driver->pools,
driver->configDir,
driver->autostartDir);
6:07 a.m.
New subject: [libvirt] [PATCH 5/6] conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
ACK with the slight adjustments.
If you want to post a v3 "inline"/as a response to patch 4 - that's fine
John
Thank you, I sent v3 of patch 4/6, fixed the slight adjustments in 5 and
6. I'll push them once I have an ACK for 4/6.
Erik
5:10 a.m.
New subject: [libvirt] [PATCH 6/6] storage: Introduce storagePoolUpdateAllState function
The 'checkPool' callback was originally part of the storageDriverAutostart
function,
but the pools need to be checked earlier during initialization phase,
otherwise we can't start a domain which mountsa volume after daemon's
restarted. This is because qemuProcessReconnect is called earlier than
storageDriverAutostart. Therefore the 'checkPool' logic has been moved to
storagePoolUpdateAllState which is called inside storageDriverInitialize.
We also need a valid 'conn' reference to be able to execute 'refreshPool'
during initialization phase. Though it isn't available until storageDriverAutostart
all of our storage backends do ignore 'conn' pointer, except for RBD,
but RBD doesn't support 'checkPool' callback, so it's safe to pass
conn = NULL in this case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
---
src/storage/storage_driver.c | 74 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 61 insertions(+), 13 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 36c05b3..12e94ad 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -75,6 +75,64 @@ static void storageDriverUnlock(void)
}
static void
+storagePoolUpdateAllState(void)
+{
+ size_t i;
+ bool active = false;
+
+ for (i = 0; i < driver->pools.count; i++) {
+ virStoragePoolObjPtr pool = driver->pools.objs[i];
+ virStorageBackendPtr backend;
+
+ virStoragePoolObjLock(pool);
+ if (!virStoragePoolObjIsActive(pool)) {
+ virStoragePoolObjUnlock(pool);
+ 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.
+ */
+ 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;
+ virStoragePoolObjUnlock(pool);
+ }
+}
+
+static void
storageDriverAutostart(void)
{
size_t i;
@@ -95,23 +153,11 @@ storageDriverAutostart(void)
virStoragePoolObjLock(pool);
if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
- VIR_ERROR(_("Missing backend %d"), pool->def->type);
virStoragePoolObjUnlock(pool);
continue;
}
- if (backend->checkPool &&
- backend->checkPool(pool, &started) < 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;
- }
-
- if (!started &&
- pool->autostart &&
+ if (pool->autostart &&
!virStoragePoolObjIsActive(pool)) {
if (backend->startPool &&
backend->startPool(conn, pool) < 0) {
@@ -215,6 +261,8 @@ storageStateInitialize(bool privileged,
driver->autostartDir) < 0)
goto error;
+ storagePoolUpdateAllState();
+
storageDriverUnlock();
ret = 0;
--
1.9.3
9:51 a.m.
New subject: [libvirt] [PATCH 6/6] storage: Introduce storagePoolUpdateAllState function
On 04/02/2015 06:10 AM, Erik Skultety wrote:
The 'checkPool' callback was originally part of the
storageDriverAutostart function,
but the pools need to be checked earlier during initialization phase,
otherwise we can't start a domain which mountsa volume after daemon's
s/mountsa/mounts a/
s/after daemon's/after the libvirtd daemon/
restarted. This is because qemuProcessReconnect is called earlier
than
storageDriverAutostart. Therefore the 'checkPool' logic has been moved to
storagePoolUpdateAllState which is called inside storageDriverInitialize.
We also need a valid 'conn' reference to be able to execute
'refreshPool'
during initialization phase. Though it isn't available until storageDriverAutostart
all of our storage backends do ignore 'conn' pointer, except for RBD,
but RBD doesn't support 'checkPool' callback, so it's safe to pass
conn = NULL in this case.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1177733
yes, this does belong in this one
Also, as I'm going through my list of backlog bugzillas, I think that
https://bugzilla.redhat.com/show_bug.cgi?id=1125805 can be duplicated to
this bz.
---
src/storage/storage_driver.c | 74 ++++++++++++++++++++++++++++++++++++--------
1 file changed, 61 insertions(+), 13 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 36c05b3..12e94ad 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -75,6 +75,64 @@ static void storageDriverUnlock(void)
}
static void
+storagePoolUpdateAllState(void)
+{
+ size_t i;
+ bool active = false;
Instead of initializing it here - do it in the code...
+
+ for (i = 0; i < driver->pools.count; i++) {
+ virStoragePoolObjPtr pool = driver->pools.objs[i];
+ virStorageBackendPtr backend;
+
+ virStoragePoolObjLock(pool);
+ if (!virStoragePoolObjIsActive(pool)) {
+ virStoragePoolObjUnlock(pool);
+ 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;
So here's my reasoning for resetting active each time through the
pool... What happens if pool [1] has a check and is determined to be
active... then pool [2] doesn't have a check function, but active is
still true from [1] - thus the refreshPool will happen.
ACK with that adjustment.
John
+ 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;
+ virStoragePoolObjUnlock(pool);
+ }
+}
+
+static void
storageDriverAutostart(void)
{
size_t i;
@@ -95,23 +153,11 @@ storageDriverAutostart(void)
virStoragePoolObjLock(pool);
if ((backend = virStorageBackendForType(pool->def->type)) == NULL) {
- VIR_ERROR(_("Missing backend %d"), pool->def->type);
virStoragePoolObjUnlock(pool);
continue;
}
- if (backend->checkPool &&
- backend->checkPool(pool, &started) < 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;
- }
-
- if (!started &&
- pool->autostart &&
+ if (pool->autostart &&
!virStoragePoolObjIsActive(pool)) {
if (backend->startPool &&
backend->startPool(conn, pool) < 0) {
@@ -215,6 +261,8 @@ storageStateInitialize(bool privileged,
driver->autostartDir) < 0)
goto error;
+ storagePoolUpdateAllState();
+
storageDriverUnlock();
ret = 0;
5:28 a.m.
On 04/02/2015 12:10 PM, Erik Skultety wrote:
Patches add necessary changes to add support for storage pool state
XMLs
and thus resolve https://bugzilla.redhat.com/show_bug.cgi?id=1177733
Erik Skultety (6):
conf: Introduce virStoragePoolDefFormatBuf
conf: Introduce virStoragePoolSaveXML
conf: Introduce virStoragePoolSaveState
storage: Add support for storage pool state XML
conf: Introduce virStoragePoolLoadAllState && virStoragePoolLoadState
storage: Introduce storagePoolUpdateAllState function
src/conf/storage_conf.c | 233 ++++++++++++++++++++++++++++++++++++-------
src/conf/storage_conf.h | 12 ++-
src/libvirt_private.syms | 2 +
src/storage/storage_driver.c | 188 +++++++++++++++++++++++++++-------
4 files changed, 358 insertions(+), 77 deletions(-)
These are v2 patches, forgot the --subject-prefix option with
format-patch.
Erik
3517
days inactive
3522
days old
22 comments
4 participants
participants (4)
-
Erik Skultety -
John Ferlan -
Ján Tomko -
Peter Krempa