[libvirt] [PATCH v2 08/25] backup: Parse and output backup XML

[Ján Tomko]

On Tue, Dec 03, 2019 at 06:17:30PM +0100, Peter Krempa wrote:
>From: Eric Blake <eblake at redhat.com>
>
>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.
>

A leftover from earlier versions? This essentially says: there might be
bugs

>Signed-off-by: Eric Blake <eblake at redhat.com>
>---
> po/POTFILES.in           |   1 +
> src/conf/Makefile.inc.am |   2 +
> src/conf/backup_conf.c   | 499 +++++++++++++++++++++++++++++++++++++++
> src/conf/backup_conf.h   | 101 ++++++++
> src/conf/virconftypes.h  |   3 +
> src/libvirt_private.syms |   8 +
> 6 files changed, 614 insertions(+)
> create mode 100644 src/conf/backup_conf.c
> create mode 100644 src/conf/backup_conf.h

[...]

>+static int
>+virDomainBackupDiskDefParseXML(xmlNodePtr node,
>+                               xmlXPathContextPtr ctxt,
>+                               virDomainBackupDiskDefPtr def,
>+                               bool push,
>+                               unsigned int flags,
>+                               virDomainXMLOptionPtr xmlopt)
>+{
>+    VIR_XPATH_NODE_AUTORESTORE(ctxt);
>+    g_autofree char *type = NULL;
>+    g_autofree char *driver = NULL;
>+    g_autofree char *backup = NULL;
>+    g_autofree char *state = NULL;
>+    int tmp;
>+    xmlNodePtr srcNode;
>+    unsigned int storageSourceParseFlags = 0;
>+    bool internal = flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL;
>+
>+    if (internal)
>+        storageSourceParseFlags = VIR_DOMAIN_DEF_PARSE_STATUS;
>+
>+    ctxt->node = node;
>+
>+    if (!(def->name = virXMLPropString(node, "name"))) {
>+        virReportError(VIR_ERR_XML_ERROR, "%s",
>+                       _("missing name from disk backup element"));
>+        return -1;
>+    }
>+
>+    def->backup = VIR_TRISTATE_BOOL_YES;
>+
>+    if ((backup = virXMLPropString(node, "backup"))) {
>+        if ((tmp = virTristateBoolTypeFromString(backup)) <= 0) {
>+            virReportError(VIR_ERR_XML_ERROR,
>+                           _("invalid disk 'backup' state '%s'"), backup);
>+            return -1;
>+        }
>+
>+        def->backup = tmp;
>+    }
>+
>+    /* don't parse anything else if backup is disabled */
>+    if (def->backup == VIR_TRISTATE_BOOL_NO)
>+        return 0;
>+
>+    if (internal) {
>+        tmp = 0;

This should not be necessary - either the condition below returns, or
tmp gets overwritten.

>+        if (!(state = virXMLPropString(node, "state")) ||
>+            (tmp = virDomainBackupDiskStateTypeFromString(state)) < 0) {
>+            virReportError(VIR_ERR_INTERNAL_ERROR,
>+                           _("disk '%s' backup state wrong or missing'"), def->name);
>+            return -1;
>+        }
>+
>+        def->state = tmp;
>+    }
>+
>+    if (!(def->store = virStorageSourceNew()))
>+        return -1;
>+
>+    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) {

The schema whitelists file and block, while a blacklist is used here.

>+            virReportError(VIR_ERR_XML_ERROR,
>+                           _("unknown disk backup type '%s'"), type);

Also, technically they are known, just unsupported.

>+            return -1;
>+        }
>+    } else {
>+        def->store->type = VIR_STORAGE_TYPE_FILE;
>+    }
>+
>+    if (push)
>+        srcNode = virXPathNode("./target", ctxt);
>+    else
>+        srcNode = virXPathNode("./scratch", ctxt);
>+
>+    if (srcNode &&
>+        virDomainStorageSourceParse(srcNode, ctxt, def->store,
>+                                    storageSourceParseFlags, xmlopt) < 0)
>+        return -1;
>+

If you make sure that this does not accept unwanted types:
Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191209/971cf26b/attachment-0001.sig>