[libvirt] [RFC PATCH 5/8] conf: Optionally keep domains with invalid XML
Michal Privoznik
mprivozn at redhat.com
Tue Oct 13 10:10:09 UTC 2015
On 22.09.2015 14:15, Martin Kletzander wrote:
> 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.
>
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
> src/bhyve/bhyve_driver.c | 2 +
> src/conf/domain_conf.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++--
> 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, 109 insertions(+), 4 deletions(-)
>
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 7f365b1f2446..abf6789c2f9c 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -1219,6 +1219,7 @@ bhyveStateInitialize(bool privileged,
> NULL, 1,
> bhyve_driver->caps,
> bhyve_driver->xmlopt,
> + false,
> NULL, NULL) < 0)
> goto cleanup;
>
> @@ -1227,6 +1228,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 230800542f8c..6850e79396bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -22676,6 +22676,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,
> @@ -22684,6 +22717,7 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
> const char *configDir,
> const char *autostartDir,
> const char *name,
> + bool keep_invalid,
> virDomainLoadConfigNotify notify,
> void *opaque)
> {
> @@ -22692,13 +22726,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;
> @@ -22711,6 +22789,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);
>
> @@ -22720,6 +22803,8 @@ virDomainObjListLoadConfig(virDomainObjListPtr doms,
> return dom;
>
> error:
> + VIR_FREE(xmlErr);
> + VIR_FREE(xmlStr);
> VIR_FREE(configFile);
> VIR_FREE(autostartLink);
> virDomainDefFree(def);
> @@ -22790,6 +22875,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
> int liveStatus,
> virCapsPtr caps,
> virDomainXMLOptionPtr xmlopt,
> + bool keep_invalid,
> virDomainLoadConfigNotify notify,
> void *opaque)
> {
> @@ -22837,6 +22923,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 8be390b7811a..fa6449461047 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2195,6 +2195,10 @@ struct _virDomainDef {
> char *title;
> char *description;
>
> + /* Possible error and string that failed parsing */
> + char *xmlStr;
> + const char *parseError;
Why is this ^^ const if we are allocating the string ourselves?
Moreover, I think these would be leaked, so I'd suggest to free them in
virDomainDefFree().
> +
> virDomainBlkiotune blkio;
> virDomainMemtune mem;
>
Michal
More information about the libvir-list
mailing list