On 26.11.2013 17:48, Peter Krempa wrote:
Disk source elements for snapshots were using separate code from our
config parser. As snapshots can be stored on more than just regular
files, we will need the universal parser to allow us to expose a variety
of snapshot disk targets. This patch reuses the config parsers and
formatters to do the job.
This initial support only changes the code without any visible XML
change.
---
src/conf/snapshot_conf.c | 70 +++++++++++++++++++++++++++++++++---------------
src/conf/snapshot_conf.h | 1 +
2 files changed, 49 insertions(+), 22 deletions(-)
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 94a74d2..7258daa 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -128,27 +128,42 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
}
}
- cur = node->children;
- while (cur) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (!def->file &&
- xmlStrEqual(cur->name, BAD_CAST "source")) {
- def->file = virXMLPropString(cur, "file");
- } else if (!def->format &&
- xmlStrEqual(cur->name, BAD_CAST "driver")) {
- char *driver = virXMLPropString(cur, "type");
- def->format = virStorageFileFormatTypeFromString(driver);
- if (def->format <= 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown disk snapshot driver
'%s'"),
- driver);
- VIR_FREE(driver);
- goto cleanup;
- }
+ def->type = -1;
+
+ for (cur = node->children; cur; cur = cur->next) {
+ if (cur->type != XML_ELEMENT_NODE)
+ continue;
+
+ if (!def->file &&
+ xmlStrEqual(cur->name, BAD_CAST "source")) {
+
+ int backingtype = def->type;
+
+ if (backingtype < 0)
+ backingtype = VIR_DOMAIN_DISK_TYPE_FILE;
So far, def->type is never parsed, so the @backingtype is always
VIR_DOMAIN_DISK_TYPE_FILE. Mmm, okay.
+
+ if (virDomainDiskSourceDefParse(cur,
+ backingtype,
+ &def->file,
+ NULL,
+ NULL,
+ NULL,
+ NULL) < 0)
+ goto cleanup;
+
+ } else if (!def->format &&
+ xmlStrEqual(cur->name, BAD_CAST "driver")) {
+ char *driver = virXMLPropString(cur, "type");
+ def->format = virStorageFileFormatTypeFromString(driver);
+ if (def->format <= 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("unknown disk snapshot driver
'%s'"),
+ driver);
VIR_FREE(driver);
+ goto cleanup;
}
+ VIR_FREE(driver);
}
- cur = cur->next;
}
if (!def->snapshot && (def->file || def->format))
@@ -577,6 +592,8 @@ static void
virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virDomainSnapshotDiskDefPtr disk)
{
+ int type = disk->type;
+
if (!disk->name)
return;
@@ -584,6 +601,13 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
if (disk->snapshot > 0)
virBufferAsprintf(buf, " snapshot='%s'",
virDomainSnapshotLocationTypeToString(disk->snapshot));
+
+ if (type < 0)
+ type = VIR_DOMAIN_DISK_TYPE_FILE;
+ else
+ virBufferAsprintf(buf, " type='%s'",
+ virDomainDiskTypeToString(type));
+
This is a very thin line between no XML change and outputting a new
'type' attribute into XML. Doesn't make much sense now, but I see this
is to be changed in 25/27. Anyway, I think it would be better to save
these small snippets up till then.
if (!disk->file && disk->format == 0) {
virBufferAddLit(buf, "/>\n");
return;
@@ -591,12 +615,14 @@ virDomainSnapshotDiskDefFormat(virBufferPtr buf,
virBufferAddLit(buf, ">\n");
- virBufferAdjustIndent(buf, 6);
if (disk->format > 0)
- virBufferEscapeString(buf, "<driver type='%s'/>\n",
+ virBufferEscapeString(buf, " <driver
type='%s'/>\n",
virStorageFileFormatTypeToString(disk->format));
- virBufferEscapeString(buf, "<source file='%s'/>\n",
disk->file);
- virBufferAdjustIndent(buf, -6);
+ virDomainDiskSourceDefFormatInternal(buf,
+ type,
+ disk->file,
+ 0, 0, 0, NULL, 0, NULL, NULL, 0);
+
virBufferAddLit(buf, " </disk>\n");
}
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ff3dca2..241d63c 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -51,6 +51,7 @@ struct _virDomainSnapshotDiskDef {
char *name; /* name matching the <target dev='...' of the domain */
int index; /* index within snapshot->dom->disks that matches name */
int snapshot; /* enum virDomainSnapshotLocation */
+ int type; /* enum virDomainDiskType */
char *file; /* new source file when snapshot is external */
int format; /* enum virStorageFileFormat */
};
ACK to the rest.
Michal