11:35 a.m.
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(a)redhat.com>
---
src/bhyve/bhyve_driver.c | 2 ++
src/conf/domain_conf.c | 36 +++++++++++++++++++++++
src/conf/domain_conf.h | 7 +++++
src/conf/virdomainobjlist.c | 71 ++++++++++++++++++++++++++++++++++++++++++---
src/conf/virdomainobjlist.h | 1 +
src/libvirt_private.syms | 1 +
src/libxl/libxl_driver.c | 3 ++
src/lxc/lxc_driver.c | 3 ++
src/qemu/qemu_driver.c | 3 ++
src/uml/uml_driver.c | 2 ++
10 files changed, 125 insertions(+), 4 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index fa1cdea24ae1..7f992dbfa8a1 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 7a295da507c4..0d9507eea186 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2472,6 +2472,9 @@ void virDomainDefFree(virDomainDefPtr def)
xmlFreeNode(def->metadata);
+ VIR_FREE(def->xmlStr);
+ VIR_FREE(def->parseError);
+
VIR_FREE(def);
}
@@ -22529,6 +22532,39 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
return ret;
}
+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;
+}
int
virDomainDeleteConfig(const char *configDir,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 90d8e13638c2..3ff20205b4d6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2215,6 +2215,10 @@ struct _virDomainDef {
char *title;
char *description;
+ /* Possible error and string that failed parsing */
+ char *xmlStr;
+ char *parseError;
+
virDomainBlkiotune blkio;
virDomainMemtune mem;
@@ -2886,6 +2890,9 @@ int virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
const char *statusDir,
virDomainObjPtr obj) ATTRIBUTE_RETURN_CHECK;
+virDomainDefPtr virDomainDefParseMinimal(const char *filename,
+ const char *xmlStr);
+
typedef void (*virDomainLoadConfigNotify)(virDomainObjPtr dom,
int newDomain,
void *opaque);
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 7568f937420a..26366a05a150 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -254,6 +254,15 @@ virDomainObjListAddLocked(virDomainObjListPtr doms,
}
}
+ if (flags & VIR_DOMAIN_OBJ_LIST_ADD_LIVE &&
+ 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);
+ goto error;
+ }
+
virDomainObjAssignDef(vm,
def,
!!(flags & VIR_DOMAIN_OBJ_LIST_ADD_LIVE),
@@ -398,6 +407,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
const char *configDir,
const char *autostartDir,
const char *name,
+ bool keep_invalid,
virDomainLoadConfigNotify notify,
void *opaque)
{
@@ -406,13 +416,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;
@@ -425,6 +479,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);
@@ -434,6 +493,8 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
return dom;
error:
+ VIR_FREE(xmlErr);
+ VIR_FREE(xmlStr);
VIR_FREE(configFile);
VIR_FREE(autostartLink);
virDomainDefFree(def);
@@ -505,6 +566,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
int liveStatus,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool keep_invalid,
virDomainLoadConfigNotify notify,
void *opaque)
{
@@ -552,6 +614,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
configDir,
autostartDir,
entry->d_name,
+ keep_invalid,
notify,
opaque);
if (dom) {
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index f47959825b10..fd5d6e461d3a 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -67,6 +67,7 @@ int virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
int liveStatus,
virCapsPtr caps,
virDomainXMLOptionPtr xmlopt,
+ bool keep_invalid,
virDomainLoadConfigNotify notify,
void *opaque);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dd085c3487bf..702c360adede 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -225,6 +225,7 @@ virDomainDefNeedsPlacementAdvice;
virDomainDefNew;
virDomainDefNewFull;
virDomainDefParseFile;
+virDomainDefParseMinimal;
virDomainDefParseNode;
virDomainDefParseString;
virDomainDefPostParse;
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 792b787659f4..e8711f0f3173 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