Re: [RFC PATCH 5/8] conf: Optionally keep domains with invalid XML

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]