On Sat, Sep 07, 2024 at 17:15:40 +0300, Nikolai Barybin via Devel wrote: As noted separately, tests and documentation is missing.
Signed-off-by: Nikolai Barybin <nikolai.barybin@virtuozzo.com> --- src/conf/domain_conf.c | 98 ++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 13 ++++++ 2 files changed, 111 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a263612ef7..cfd25032b3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7654,6 +7654,52 @@ 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 = virXMLPropString(ctxt->node, "type"))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing data file store type")); + return -1; + }
You can use virXMLPropStringRequired() which also reports error.
+ + if (!(format = virXPathString("string(./format/@type)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing data file store format")); + return -1; + }
IMO we shouldn't even have a 'format' field here as this doesn't make sense to have with anything non-raw.
+ + if (!(source = virXPathNode("./source", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing disk data file store source")); + return -1; + } + + if (!(dataFileStore = virDomainStorageSourceParseBase(type, format, NULL))) + return -1; + + if (virDomainStorageSourceParse(source, ctxt, dataFileStore, flags, xmlopt) < 0) + return -1;
This parses too much, some of which we don't want to support with the data file, such as 'slice'. Make sure to forbid slices if they are parsed.
+ + dataFileStore->readonly = src->readonly; + src->dataFileStore = g_steal_pointer(&dataFileStore); + + return 0; +} + + int virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, virStorageSource *src,
[...]
@@ -22819,6 +22869,46 @@ virDomainDiskSourceFormat(virBuffer *buf, return 0; }
+int +virDomainDiskDataFileStoreFormat(virBuffer *buf, + virStorageSource *src, + virDomainXMLOption *xmlopt, + unsigned int flags) +{ + g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) formatAttrBuf = VIR_BUFFER_INITIALIZER; + g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); + virStorageSource *dataFileStore = src->dataFileStore; + bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE; + + if (!dataFileStore) + return 0; + + /* don't write detected data file member to inactive xml */ + if (inactive && dataFileStore->detected) + return 0; + + if (dataFileStore->format != VIR_STORAGE_FILE_RAW) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected disk data file format '%1$d', only raw format is supported"), + dataFileStore->format);
This definitely doesn't belong to the formatter, but rather to the parser, where such a check is missing.
+ return -1; + } + + virBufferAsprintf(&attrBuf, " type='%s'", + virStorageTypeToString(dataFileStore->type)); + + virBufferAsprintf(&formatAttrBuf, " type='%s'", + virStorageFileFormatTypeToString(dataFileStore->format));
I wonder wheter we should even parse it if this only ever makes sense with raw images.
+ virXMLFormatElement(&childBuf, "format", &formatAttrBuf, NULL); + + if (virDomainDiskSourceFormat(&childBuf, dataFileStore, "source", 0, false, + flags, false, false, xmlopt) < 0) + return -1; + + virXMLFormatElement(buf, "dataFileStore", &attrBuf, &childBuf); + return 0; +}
int virDomainDiskBackingStoreFormat(virBuffer *buf,