10:45 a.m.
On Wed, Nov 20, 2024 at 18:48:39 +0300, Nikolai Barybin via Devel wrote:
Data files are simple raw images. Thus, we don't need to parse
too much. The main objectives are:
- allow only RAW format
- forbid storage slices
- include this parsing/formatting into backing chain parse/format as
well as into top storage source parse/format because data file can
belong to backing image
Signed-off-by: Nikolai Barybin <nikolai.barybin(a)virtuozzo.com>
---
src/conf/domain_conf.c | 100 +++++++++++++++++++++++++++++++++++++++++
src/conf/domain_conf.h | 13 ++++++
2 files changed, 113 insertions(+)
As I've already mentioned before I think that it'll make much more sense
if the element is called <dataStore> (as it doesn't necessarily have to
be a file) and also that it's nested under:
<disk ...>
<source file=...>
<dataStore type='file'>
<source ...>
</dataStore>
</source>
</disk>
The data-file qcow2 feature ties the data very closely to the source so
I don't think the same treatment as we have with <backingStore> would
model this relationship correctly.
In fact I consider that <backingStore> should have been also nested as
above.
I'll do the appropriate adjustment.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 295707ec1f..9348c12725 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7670,6 +7670,62 @@ virDomainStorageSourceParse(xmlNodePtr node,
}
+int
+virDomainDiskDataFileStoreParse(xmlXPathContextPtr ctxt,
+ virStorageSource *src,
+ unsigned int flags,
+ virDomainXMLOption *xmlopt)
+{
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ xmlNodePtr source;
+ g_autofree char *type = NULL;
+ g_autofree char *format = NULL;
+ g_autoptr(virStorageSource) dataFileStore = NULL;
+
+ if (!(ctxt->node = virXPathNode("./dataFileStore", ctxt)))
+ return 0;
+
+ if (!(type = virXMLPropStringRequired(ctxt->node, "type")))
+ return -1;
+
+ if (!(format = virXPathString("string(./format/@type)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing data file store format"));
+ return -1;
+ }
+
+ if (!(source = virXPathNode("./source", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing disk data file store source"));
+ return -1;
+ }
+
+ if (virStorageFileFormatTypeFromString(format) != VIR_STORAGE_FILE_RAW) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unexpected disk data file format '%1$s', only raw
format is supported"),
+ format);
+ return -1;
+ }
+
+ if (!(dataFileStore = virDomainStorageSourceParseBase(type, format, NULL)))
+ return -1;
+
+ if (virDomainStorageSourceParse(source, ctxt, dataFileStore, flags, xmlopt) < 0)
+ return -1;
+
+ if (dataFileStore->sliceStorage) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("slices are not supported for fata file store
source"));
+ return -1;
+ }
Validation should be done in src/conf/domain_validate.c or the
hypervisor-driver specific code. I'll move this there.
This is also lacking validation that the disk source is QCOW2 as no
other source supports this. I'll add that there as well, as move the
format check above.
[...]
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a187ab4083..4d1939aa1b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3909,6 +3909,12 @@ int virDomainDiskSourceFormat(virBuffer *buf,
bool skipEnc,
virDomainXMLOption *xmlopt);
+int
+virDomainDiskDataFileStoreFormat(virBuffer *buf,
+ virStorageSource *src,
+ virDomainXMLOption *xmlopt,
+ unsigned int flags);
+
int
virDomainDiskBackingStoreFormat(virBuffer *buf,
virStorageSource *src,
@@ -4426,6 +4432,13 @@ int virDomainStorageSourceParse(xmlNodePtr node,
virDomainXMLOption *xmlopt)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+int
+virDomainDiskDataFileStoreParse(xmlXPathContextPtr ctxt,
+ virStorageSource *src,
+ unsigned int flags,
+ virDomainXMLOption *xmlopt)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
+
int
virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
virStorageSource *src,
Please do not unnecessarily export functions. Neither of these is being
used outside of the 'domain_conf.c' module.
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>