7:05 a.m.
On Wed, Mar 27, 2019 at 05:10:45 -0500, Eric Blake wrote:
Accept XML describing a generic block job, and output it again as
needed. This may still need a few tweaks to match the documented XML
and RNG schema.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/conf/backup_conf.h | 96 +++++++
src/conf/virconftypes.h | 3 +
src/conf/Makefile.inc.am | 2 +
src/conf/backup_conf.c | 544 +++++++++++++++++++++++++++++++++++++++
src/libvirt_private.syms | 8 +-
5 files changed, 652 insertions(+), 1 deletion(-)
create mode 100644 src/conf/backup_conf.h
create mode 100644 src/conf/backup_conf.c
[...]
+static int
+virDomainBackupDiskDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainBackupDiskDefPtr def,
+ bool push, bool internal,
+ virDomainXMLOptionPtr xmlopt)
+{
+ int ret = -1;
+ // char *backup = NULL; /* backup="yes|no"? */
+ char *type = NULL;
+ char *driver = NULL;
+ xmlNodePtr cur;
+ xmlNodePtr saved = ctxt->node;
+
+ ctxt->node = node;
+
+ if (VIR_ALLOC(def->store) < 0)
+ goto cleanup;
+
+ def->name = virXMLPropString(node, "name");
+ if (!def->name) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing name from disk backup element"));
+ goto cleanup;
+ }
+
+ /* Needed? A way for users to list a disk and explicitly mark it
+ * as not participating, and then output shows all disks rather
+ * than just active disks */
+#if 0
+ backup = virXMLPropString(node, "backup");
+ if (backup) {
+ def->type = virDomainCheckpointTypeFromString(checkpoint);
+ if (def->type <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown disk checkpoint setting '%s'"),
+ checkpoint);
+ goto cleanup;
+ }
+ }
+#endif
Some leftovers?
+
+ if ((type = virXMLPropString(node, "type"))) {
+ if ((def->store->type = virStorageTypeFromString(type)) <= 0 ||
+ def->store->type == VIR_STORAGE_TYPE_VOLUME ||
+ def->store->type == VIR_STORAGE_TYPE_DIR) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("unknown disk backup type '%s'"), type);
+ goto cleanup;
+ }
+ } else {
+ def->store->type = VIR_STORAGE_TYPE_FILE;
+ }
+
+ if ((cur = virXPathNode(push ? "./target" : "./scratch", ctxt))
&&
+ virDomainDiskSourceParse(cur, ctxt, def->store, 0, xmlopt) < 0)
+ goto cleanup;
+
+ if (internal) {
+ int detected;
+ if (virXPathInt("string(./node/@detected)", ctxt, &detected) <
0)
+ goto cleanup;
+ def->store->detected = detected;
+ def->store->nodeformat = virXPathString("string(./node)", ctxt);
This should not be public name. Current design of nodenames does not
allow you to depend on the name.
In general, nodenames are a qemu-specific thing thus should not be
expressed nor documented in the public XML.
Same applies for the formatter below.
(yes, the original implementation is suboptimal as I've put
nodeformat/nodestorage into virStorageSource, but for domain stuff it's
never formatted using the code in src/conf)
+ }
+
+ if ((driver = virXPathString("string(./driver/@type)", ctxt))) {
+ def->store->format = virStorageFileFormatTypeFromString(driver);
+ if (def->store->format <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown disk backup driver '%s'"),
driver);
+ goto cleanup;
+ } else if (!push && def->store->format != VIR_STORAGE_FILE_QCOW2)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("pull mode requires qcow2 driver, not
'%s'"),
+ driver);
+ goto cleanup;
+ }
+ }
+
+ /* validate that the passed path is absolute */
+ if (virStorageSourceIsRelative(def->store)) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("disk backup image path '%s' must be
absolute"),
+ def->store->path);
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ ctxt->node = saved;
+
+ VIR_FREE(driver);
+// VIR_FREE(backup);
here too
+ VIR_FREE(type);
+ if (ret < 0)
+ virDomainBackupDiskDefClear(def);
+ return ret;
+}