[libvirt] [PATCH 0/8] Make loading domains with invalid XML possible
The RFC version [1] was ACKed, but I have found out that we could then cause inconsistency by calling virDomainCreateXML() or Restore() calls etc. This series is basically the same apart from the fact that it is rebased on current master and then one simple diff [2] is added to PATCH 5/8. [1] https://www.redhat.com/archives/libvir-list/2015-September/msg00698.html [2] diff to the previous version: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 16f26acf4839..c8291be2b7b5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2875,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), -- Martin Kletzander (8): conf, virsh: Add new domain shutoff reason qemu: Few whitespace cleanups conf: Extract name-parsing into its own function conf: Extract UUID parsing into its own function conf: Optionally keep domains with invalid XML, but don't allow starting them qemu: Don't lookup invalid domains unless specified otherwise qemu: Prepare basic APIs to handle invalid defs qemu: Load domains with invalid XML on start include/libvirt/libvirt-domain.h | 2 + src/bhyve/bhyve_driver.c | 2 + src/conf/domain_conf.c | 191 +++++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 5 + src/libxl/libxl_driver.c | 3 + src/lxc/lxc_driver.c | 3 + src/qemu/qemu_driver.c | 64 ++++++++++--- src/uml/uml_driver.c | 2 + tools/virsh-domain-monitor.c | 3 +- 9 files changed, 234 insertions(+), 41 deletions(-) -- 2.6.3
This new reason means the domain is shutoff because parsing of the XML failed while daemon was starting and from user's point of view, there's nothing else to do with it other than re-defining it. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/conf/domain_conf.c | 3 ++- tools/virsh-domain-monitor.c | 3 ++- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1ea6a5d0786..37aa6db85cda 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -142,6 +142,8 @@ typedef enum { VIR_DOMAIN_SHUTOFF_FAILED = 6, /* domain failed to start */ VIR_DOMAIN_SHUTOFF_FROM_SNAPSHOT = 7, /* restored from a snapshot which was * taken while domain was shutoff */ + VIR_DOMAIN_SHUTOFF_INVALID_XML = 8, /* Domain XML failed to load for some + * reason, it needs to be re-defined */ # ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_SHUTOFF_LAST # endif diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7ec6954ac5a0..145106b75c94 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -717,7 +717,8 @@ VIR_ENUM_IMPL(virDomainShutoffReason, VIR_DOMAIN_SHUTOFF_LAST, "migrated", "saved", "failed", - "from snapshot") + "from snapshot", + "invalid XML") VIR_ENUM_IMPL(virDomainCrashedReason, VIR_DOMAIN_CRASHED_LAST, "unknown", diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d4e500b21225..cdcee3cbb267 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -218,7 +218,8 @@ VIR_ENUM_IMPL(virshDomainShutoffReason, N_("migrated"), N_("saved"), N_("failed"), - N_("from snapshot")) + N_("from snapshot"), + N_("invalid XML")) VIR_ENUM_DECL(virshDomainCrashedReason) VIR_ENUM_IMPL(virshDomainCrashedReason, -- 2.6.3
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 65ccf99986af..65437cc749f8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7047,9 +7047,9 @@ qemuDomainObjRestore(virConnectPtr conn, } -static char -*qemuDomainGetXMLDesc(virDomainPtr dom, - unsigned int flags) +static char * +qemuDomainGetXMLDesc(virDomainPtr dom, + unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -7456,7 +7456,10 @@ qemuDomainCreate(virDomainPtr dom) return qemuDomainCreateWithFlags(dom, 0); } -static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) +static virDomainPtr +qemuDomainDefineXMLFlags(virConnectPtr conn, + const char *xml, + unsigned int flags) { virQEMUDriverPtr driver = conn->privateData; virDomainDefPtr def = NULL; -- 2.6.3
Create virDomainDefParseName that parses only the name from XML definition. This will be used in future patches. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 145106b75c94..0aea59f37a13 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14760,6 +14760,19 @@ virDomainVcpuParse(virDomainDefPtr def, return ret; } +static int +virDomainDefParseName(virDomainDefPtr def, + xmlXPathContextPtr ctxt) +{ + if (!(def->name = virXPathString("string(./name[1])", ctxt))) { + virReportError(VIR_ERR_NO_NAME, NULL); + return -1; + } + + return 0; +} + + static virDomainDefPtr virDomainDefParseXML(xmlDocPtr xml, xmlNodePtr root, @@ -14885,11 +14898,8 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_FREE(capsdata); } - /* Extract domain name */ - if (!(def->name = virXPathString("string(./name[1])", ctxt))) { - virReportError(VIR_ERR_NO_NAME, NULL); + if (virDomainDefParseName(def, ctxt) < 0) goto error; - } /* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid * exist, they must match; and if only the latter exists, it can -- 2.6.3
Create virDomainDefParseUUID that parses only the UUID from XML definition. This will be used in future patch(es). Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/domain_conf.c | 64 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0aea59f37a13..9bce34b73905 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14760,6 +14760,49 @@ virDomainVcpuParse(virDomainDefPtr def, return ret; } +/* + * Parse domain's UUID, optionally generating it if missing. + */ +static int +virDomainDefParseUUID(virDomainDefPtr def, + xmlXPathContextPtr ctxt, + bool required, + bool *generated) +{ + char *tmp = virXPathString("string(./uuid[1])", ctxt); + + if (generated) + *generated = false; + + if (tmp) { + if (virUUIDParse(tmp, def->uuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("malformed uuid element")); + VIR_FREE(tmp); + return -1; + } + VIR_FREE(tmp); + return 0; + } + + if (required) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing required UUID")); + return -1; + } + + if (virUUIDGenerate(def->uuid)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Failed to generate UUID")); + return -1; + } + + if (generated) + *generated = true; + + return 0; +} + static int virDomainDefParseName(virDomainDefPtr def, xmlXPathContextPtr ctxt) @@ -14901,25 +14944,8 @@ virDomainDefParseXML(xmlDocPtr xml, if (virDomainDefParseName(def, ctxt) < 0) goto error; - /* Extract domain uuid. If both uuid and sysinfo/system/entry/uuid - * exist, they must match; and if only the latter exists, it can - * also serve as the uuid. */ - tmp = virXPathString("string(./uuid[1])", ctxt); - if (!tmp) { - if (virUUIDGenerate(def->uuid)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Failed to generate UUID")); - goto error; - } - uuid_generated = true; - } else { - if (virUUIDParse(tmp, def->uuid) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("malformed uuid element")); - goto error; - } - VIR_FREE(tmp); - } + if (virDomainDefParseUUID(def, ctxt, false, &uuid_generated) < 0) + goto error; /* Extract short description of domain (title) */ def->title = virXPathString("string(./title[1])", ctxt); -- 2.6.3
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@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
Change qemuDomObjFromDomain() to qemuDomObjFromDomainInternal() with additional parameter that controls how to deal with invalid domains. New qemuDomObjFromDomain() then follows the safe path wo we don't have to change its callers from all APIs and qemuDomObjFromDomainInvalid() is added as a new function that can be called from APIs that are prepared to handle invalid definitions as well. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0323a451e4a0..ec369719799e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -210,15 +210,22 @@ struct qemuAutostartData { /** * qemuDomObjFromDomain: * @domain: Domain pointer that has to be looked up + * @invalid: Whether to also return invalid definitions * * This function looks up @domain and returns the appropriate virDomainObjPtr * that has to be released by calling virDomainObjEndAPI(). * + * If @invalid is true, this function can return domain definition with only + * name and uuid set, so beware as that can lead to NULL pointer dereference. + * This function should be called with @invalid == true only from APIs that are + * needed to fix the domain definition (e.g. those needed for looking it up or + * providing a new definition. + * * Returns the domain object with incremented reference counter which is locked * on success, NULL otherwise. */ static virDomainObjPtr -qemuDomObjFromDomain(virDomainPtr domain) +qemuDomObjFromDomainInternal(virDomainPtr domain, bool invalid) { virDomainObjPtr vm; virQEMUDriverPtr driver = domain->conn->privateData; @@ -232,10 +239,29 @@ qemuDomObjFromDomain(virDomainPtr domain) uuidstr, domain->name); return NULL; } + if (!invalid && 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); + } return vm; } +static inline virDomainObjPtr +qemuDomObjFromDomain(virDomainPtr domain) +{ + return qemuDomObjFromDomainInternal(domain, false); +} + +static inline virDomainObjPtr +qemuDomObjFromDomainInvalid(virDomainPtr domain) +{ + return qemuDomObjFromDomainInternal(domain, true); +} + /* Looks up the domain object from snapshot and unlocks the * driver. The returned domain object is locked and ref'd and the * caller must call virDomainObjEndAPI() on it. */ -- 2.6.3
In order for the user to be able to fix broken domains function qemuDomainGetXMLDesc() needs to be able to lookup invalid domain definitions and handle them properly. When redefined, function qemuDomainDefineXMLFlags() must clear the 'invalid XML' reason. As a nice addition, qemuDomainGetState() can lookup such domains without any other change and that allows virsh not only to get their status, but also to list them. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ec369719799e..0832c8505417 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2711,7 +2711,7 @@ qemuDomainGetState(virDomainPtr dom, virCheckFlags(0, -1); - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomObjFromDomainInvalid(dom))) goto cleanup; if (virDomainGetStateEnsureACL(dom->conn, vm->def) < 0) @@ -7086,19 +7086,23 @@ qemuDomainGetXMLDesc(virDomainPtr dom, /* Flags checked by virDomainDefFormat */ - if (!(vm = qemuDomObjFromDomain(dom))) + if (!(vm = qemuDomObjFromDomainInvalid(dom))) goto cleanup; if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; - if (qemuDomainUpdateCurrentMemorySize(driver, vm) < 0) - goto cleanup; + if (vm->def->parseError) { + ignore_value(VIR_STRDUP(ret, vm->def->xmlStr)); + } else { + if (qemuDomainUpdateCurrentMemorySize(driver, vm) < 0) + goto cleanup; - if ((flags & VIR_DOMAIN_XML_MIGRATABLE)) - flags |= QEMU_DOMAIN_FORMAT_LIVE_FLAGS; + if ((flags & VIR_DOMAIN_XML_MIGRATABLE)) + flags |= QEMU_DOMAIN_FORMAT_LIVE_FLAGS; - ret = qemuDomainFormatXML(driver, vm, flags); + ret = qemuDomainFormatXML(driver, vm, flags); + } cleanup: virDomainObjEndAPI(&vm); @@ -7566,6 +7570,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, goto cleanup; } + if (oldDef && oldDef->parseError) + virDomainObjSetState(vm, virDomainObjGetState(vm, NULL), 0); + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_DEFINED, !oldDef ? -- 2.6.3
If we load such domains, we don't need to handle invalid XML checking in qemuProcessStart, but we can move it to qemuDomainDefPostParse() or even into the parsing functions if all goes well. So the only thing we'll need to worry about after this is XML parsing code that would error out for running domains or non-qemu drivers. The latter will be fixed in following patches, hence making the active XML parsing the only problematic part. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/qemu/qemu_driver.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0832c8505417..1be603c7de9d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -313,6 +313,7 @@ qemuAutostartDomain(virDomainObjPtr vm, virObjectRef(vm); virResetLastError(); if (vm->autostart && + !vm->def->parseError && !virDomainObjIsActive(vm)) { if (qemuProcessBeginJob(data->driver, vm) < 0) { err = virGetLastError(); @@ -954,7 +955,7 @@ qemuStateInitialize(bool privileged, cfg->autostartDir, 0, qemu_driver->caps, qemu_driver->xmlopt, - false, + true, NULL, NULL) < 0) goto error; @@ -1035,7 +1036,7 @@ qemuStateReload(void) cfg->configDir, cfg->autostartDir, 0, caps, qemu_driver->xmlopt, - false, + true, qemuNotifyLoadDomain, qemu_driver); cleanup: virObjectUnref(cfg); -- 2.6.3
On Thu, Nov 26, 2015 at 02:21:45PM +0100, Martin Kletzander wrote:
The RFC version [1] was ACKed, but I have found out that we could then cause inconsistency by calling virDomainCreateXML() or Restore() calls etc. This series is basically the same apart from the fact that it is rebased on current master and then one simple diff [2] is added to PATCH 5/8.
[1] https://www.redhat.com/archives/libvir-list/2015-September/msg00698.html
[2] diff to the previous version:
NACK, I've found few little things that could be done better, it needs to be rebased on top of some recent patches plus I have an improvement to include. v2 coming soon.
participants (1)
-
Martin Kletzander