[libvirt] [PATCH 5/8] conf: Optionally keep domains with invalid XML, but don't allow starting them

Martin Kletzander mkletzan at redhat.com
Thu Nov 26 13:21:50 UTC 2015


Add new parameter to virDomainObjListLoadConfig() and
virDomainObjListLoadAllConfigs() that controls whether domains with
invalid XML (which could not be parsed) should be kept in order not to
lose track of them.  For now, the parameter is set to false in all
callers.  Each driver can switch it to true when it is prepared to deal
with such domains.

For the domain object to be created add virDomainDefParseMinimal() that
parses only name and UUID from the XML definition.  UUID must be
present, it will not be generated.  The purpose of this function is to
be used when all else fails, but we still want a domain object to work
with.

Also explicitly disable adding the invalid domain into the list of
active ones, as that would render our internal structures inconsistent.

Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
---
 src/bhyve/bhyve_driver.c |   2 +
 src/conf/domain_conf.c   | 106 +++++++++++++++++++++++++++++++++++++++++++++--
 src/conf/domain_conf.h   |   5 +++
 src/libxl/libxl_driver.c |   3 ++
 src/lxc/lxc_driver.c     |   3 ++
 src/qemu/qemu_driver.c   |   3 ++
 src/uml/uml_driver.c     |   2 +
 7 files changed, 120 insertions(+), 4 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 4840a0eb8c53..11b75569c004 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -1241,6 +1241,7 @@ bhyveStateInitialize(bool privileged,
                                        NULL, 1,
                                        bhyve_driver->caps,
                                        bhyve_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto cleanup;

@@ -1249,6 +1250,7 @@ bhyveStateInitialize(bool privileged,
                                        BHYVE_AUTOSTART_DIR, 0,
                                        bhyve_driver->caps,
                                        bhyve_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto cleanup;

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9bce34b73905..c8291be2b7b5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2618,6 +2618,9 @@ void virDomainDefFree(virDomainDefPtr def)

     xmlFreeNode(def->metadata);

+    VIR_FREE(def->xmlStr);
+    VIR_FREE(def->parseError);
+
     VIR_FREE(def);
 }

@@ -2872,6 +2875,14 @@ virDomainObjListAddLocked(virDomainObjListPtr doms,
             }
         }

+        if (vm->def->parseError) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("domain '%s' was not loaded due to an XML error "
+                             "(%s), please redefine it"),
+                           vm->def->name, vm->def->parseError);
+            virDomainObjEndAPI(&vm);
+        }
+
         virDomainObjAssignDef(vm,
                               def,
                               !!(flags & VIR_DOMAIN_OBJ_LIST_ADD_LIVE),
@@ -22834,6 +22845,39 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
     return ret;
 }

+static virDomainDefPtr
+virDomainDefParseMinimal(const char *filename,
+                         const char *xmlStr)
+{
+    xmlXPathContextPtr ctxt = NULL;
+    virDomainDefPtr def = NULL;
+    xmlDocPtr xml = NULL;
+
+    if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"))))
+        goto cleanup;
+
+    ctxt = xmlXPathNewContext(xml);
+    if (ctxt == NULL) {
+        virReportOOMError();
+        goto cleanup;
+    }
+
+    ctxt->node = xmlDocGetRootElement(xml);
+
+    if (!(def = virDomainDefNew()))
+        goto cleanup;
+
+    def->id = -1;
+
+    if (virDomainDefParseName(def, ctxt) < 0 ||
+        virDomainDefParseUUID(def, ctxt, true, NULL) < 0)
+        virDomainDefFree(def);
+
+ cleanup:
+    xmlFreeDoc(xml);
+    xmlXPathFreeContext(ctxt);
+    return def;
+}

 static virDomainObjPtr
 virDomainObjListLoadConfig(virDomainObjListPtr doms,
@@ -22842,6 +22886,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
                            const char *configDir,
                            const char *autostartDir,
                            const char *name,
+                           bool keep_invalid,
                            virDomainLoadConfigNotify notify,
                            void *opaque)
 {
@@ -22850,13 +22895,57 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
     virDomainObjPtr dom;
     int autostart;
     virDomainDefPtr oldDef = NULL;
+    char *xmlStr = NULL;
+    char *xmlErr = NULL;

     if ((configFile = virDomainConfigFile(configDir, name)) == NULL)
         goto error;
-    if (!(def = virDomainDefParseFile(configFile, caps, xmlopt,
-                                      VIR_DOMAIN_DEF_PARSE_INACTIVE |
-                                      VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS)))
-        goto error;
+
+    def = virDomainDefParseFile(configFile, caps, xmlopt,
+                                VIR_DOMAIN_DEF_PARSE_INACTIVE |
+                                VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS);
+    if (!def) {
+        char *tmp = NULL;
+
+        if (!keep_invalid)
+            goto error;
+
+        if (VIR_STRDUP(xmlErr, virGetLastErrorMessage()) < 0)
+            goto error;
+
+        if (virFileReadAll(configFile, 1024*1024*10, &xmlStr) < 0)
+            goto error;
+
+        if (!(def = virDomainDefParseMinimal(NULL, xmlStr)))
+            goto error;
+
+        /*
+         * Remove the comment with a warning from the top.  Don't fail
+         * if we can't copy it or find it.
+         */
+        tmp = strstr(xmlStr, "-->");
+
+        if (tmp)
+            tmp += strlen("-->");
+        else
+            tmp = xmlStr;
+
+        if (virAsprintf(&def->xmlStr,
+                        "<!-- %s\n\n%s\n-->%s",
+                        _("WARNING: The following XML failed to load!"
+                          "\n\n"
+                          "In order for it to be loaded properly, "
+                          "it needs to be fixed.\n"
+                          "The error that was reported while loading "
+                          "is provided below for your convenience:"),
+                        xmlErr, tmp) < 0) {
+            def->xmlStr = xmlStr;
+            xmlStr = NULL;
+        }
+
+        def->parseError = xmlErr;
+        xmlErr = NULL;
+    }

     if ((autostartLink = virDomainConfigFile(autostartDir, name)) == NULL)
         goto error;
@@ -22869,6 +22958,11 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,

     dom->autostart = autostart;

+    if (def->parseError) {
+        virDomainObjSetState(dom, VIR_DOMAIN_SHUTOFF,
+                             VIR_DOMAIN_SHUTOFF_INVALID_XML);
+    }
+
     if (notify)
         (*notify)(dom, oldDef == NULL, opaque);

@@ -22878,6 +22972,8 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
     return dom;

  error:
+    VIR_FREE(xmlErr);
+    VIR_FREE(xmlStr);
     VIR_FREE(configFile);
     VIR_FREE(autostartLink);
     virDomainDefFree(def);
@@ -22947,6 +23043,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
                                int liveStatus,
                                virCapsPtr caps,
                                virDomainXMLOptionPtr xmlopt,
+                               bool keep_invalid,
                                virDomainLoadConfigNotify notify,
                                void *opaque)
 {
@@ -22994,6 +23091,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
                                              configDir,
                                              autostartDir,
                                              entry->d_name,
+                                             keep_invalid,
                                              notify,
                                              opaque);
         if (dom) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 038d65b059af..7000012ee559 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2209,6 +2209,10 @@ struct _virDomainDef {
     char *title;
     char *description;

+    /* Possible error and string that failed parsing */
+    char *xmlStr;
+    char *parseError;
+
     virDomainBlkiotune blkio;
     virDomainMemtune mem;

@@ -2923,6 +2927,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
                                    int liveStatus,
                                    virCapsPtr caps,
                                    virDomainXMLOptionPtr xmlopt,
+                                   bool keep_invalid,
                                    virDomainLoadConfigNotify notify,
                                    void *opaque);

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 35d7fae892a6..322be00eed42 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -696,6 +696,7 @@ libxlStateInitialize(bool privileged,
                                        1,
                                        cfg->caps,
                                        libxl_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto error;

@@ -708,6 +709,7 @@ libxlStateInitialize(bool privileged,
                                        0,
                                        cfg->caps,
                                        libxl_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto error;

@@ -748,6 +750,7 @@ libxlStateReload(void)
                                    1,
                                    cfg->caps,
                                    libxl_driver->xmlopt,
+                                   false,
                                    NULL, libxl_driver);

     virDomainObjListForEach(libxl_driver->domains, libxlAutostartDomain,
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 1a9550e438ae..a4292046e486 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1666,6 +1666,7 @@ static int lxcStateInitialize(bool privileged,
                                        NULL, 1,
                                        caps,
                                        lxc_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto cleanup;

@@ -1677,6 +1678,7 @@ static int lxcStateInitialize(bool privileged,
                                        cfg->autostartDir, 0,
                                        caps,
                                        lxc_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto cleanup;

@@ -1741,6 +1743,7 @@ lxcStateReload(void)
                                    cfg->autostartDir, 0,
                                    caps,
                                    lxc_driver->xmlopt,
+                                   false,
                                    lxcNotifyLoadDomain, lxc_driver);
     virObjectUnref(caps);
     virObjectUnref(cfg);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 65437cc749f8..0323a451e4a0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -905,6 +905,7 @@ qemuStateInitialize(bool privileged,
                                        NULL, 1,
                                        qemu_driver->caps,
                                        qemu_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto error;

@@ -927,6 +928,7 @@ qemuStateInitialize(bool privileged,
                                        cfg->autostartDir, 0,
                                        qemu_driver->caps,
                                        qemu_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto error;

@@ -1007,6 +1009,7 @@ qemuStateReload(void)
                                    cfg->configDir,
                                    cfg->autostartDir, 0,
                                    caps, qemu_driver->xmlopt,
+                                   false,
                                    qemuNotifyLoadDomain, qemu_driver);
  cleanup:
     virObjectUnref(cfg);
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 14598fc20c8d..6136fe36b07c 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -578,6 +578,7 @@ umlStateInitialize(bool privileged,
                                        uml_driver->autostartDir, 0,
                                        uml_driver->caps,
                                        uml_driver->xmlopt,
+                                       false,
                                        NULL, NULL) < 0)
         goto error;

@@ -646,6 +647,7 @@ umlStateReload(void)
                                    uml_driver->autostartDir, 0,
                                    uml_driver->caps,
                                    uml_driver->xmlopt,
+                                   false,
                                    umlNotifyLoadDomain, uml_driver);
     umlDriverUnlock(uml_driver);

-- 
2.6.3




More information about the libvir-list mailing list