Re: [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 

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 
---
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 

Jano


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Daniel P . Berrangé]

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

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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

[Peter Krempa]

From: Eric Blake 

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 
---
 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

diff --git a/po/POTFILES.in b/po/POTFILES.in
index debb51cd70..0ff3beeb7e 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -19,6 +19,7 @@
 @SRCDIR@/src/bhyve/bhyve_monitor.c
 @SRCDIR@/src/bhyve/bhyve_parse_command.c
 @SRCDIR@/src/bhyve/bhyve_process.c
+@SRCDIR@/src/conf/backup_conf.c
 @SRCDIR@/src/conf/capabilities.c
 @SRCDIR@/src/conf/checkpoint_conf.c
 @SRCDIR@/src/conf/cpu_conf.c
diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am
index 5035b9b524..debc6f4eef 100644
--- a/src/conf/Makefile.inc.am
+++ b/src/conf/Makefile.inc.am
@@ -12,6 +12,8 @@ NETDEV_CONF_SOURCES = \
$(NULL)

 DOMAIN_CONF_SOURCES = \
+   conf/backup_conf.c \
+   conf/backup_conf.h \
conf/capabilities.c \
conf/capabilities.h \
conf/checkpoint_conf.c \
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
new file mode 100644
index 00..aaafdf12a2
--- /dev/null
+++ b/src/conf/backup_conf.c
@@ -0,0 +1,499 @@
+/*
+ * backup_conf.c: domain backup XML processing
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ */
+
+#include 
+
+#include "configmake.h"
+#include "internal.h"
+#include "virbuffer.h"
+#include "datatypes.h"
+#include "domain_conf.h"
+#include "virlog.h"
+#include "viralloc.h"
+#include "backup_conf.h"
+#include "virstoragefile.h"
+#include "virfile.h"
+#include "virerror.h"
+#include "virxml.h"
+#include "virstring.h"
+#include "virhash.h"
+#include "virenum.h"
+
+#define VIR_FROM_THIS VIR_FROM_DOMAIN
+
+VIR_LOG_INIT("conf.backup_conf");
+
+VIR_ENUM_DECL(virDomainBackup);
+VIR_ENUM_IMPL(virDomainBackup,
+  VIR_DOMAIN_BACKUP_TYPE_LAST,
+  "default",
+  "push",
+  "pull");
+
+/* following values appear in the status XML */
+VIR_ENUM_DECL(virDomainBackupDiskState);
+VIR_ENUM_IMPL(virDomainBackupDiskState,
+  VIR_DOMAIN_BACKUP_DISK_STATE_LAST,
+  "",
+  "running",
+  "complete",
+  "failed",
+  "cancelling",
+  "cancelled");
+
+void
+virDomainBackupDefFree(virDomainBackupDefPtr def)
+{
+size_t i;
+
+if (!def)
+return;
+
+g_free(def->incremental);
+virStorageNetHostDefFree(1, def->server);
+
+for (i = 0; i < def->ndisks; i++) {
+virDomainBackupDiskDefPtr disk = def->disks + i;
+
+g_free(disk->name);
+virObjectUnref(disk->store);
+}
+
+g_free(def->disks);
+g_free(def);
+}
+
+
+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;
+}
+
+