[libvirt] [PATCH v3 3/3] virsh: add support for domain XML validation

[Jiri Denemark]

On Tue, Jan 13, 2015 at 17:00:16 +0000, Daniel Berrange wrote:
> The 'virsh edit' command gets XML validation enabled by default,
> with a --skip-validate option to disable it. The 'virsh define'
> and 'virsh create' commands get a --validate option to enable
> it, to avoid regressions for existing scripts.
> 
> The quality of error reporting from libxml2 varies depending
> on the type of XML error made. Sometimes it is quite clear
> and useful, other times it is obscure & inaccurate. At least
> the user will see an error now, rather than having their
> XML modification silently disappear.
> ---
>  tools/virsh-domain.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 78 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 6733cfa..fe8f4ba 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -133,6 +133,56 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd,
>      return vshLookupDomainInternal(ctl, cmd->def->name, n, flags);
>  }
>  
> +static virDomainPtr
> +vshDomainDefine(virConnectPtr conn, const char *xml, unsigned int flags)
> +{
> +    virDomainPtr dom;
> +    if (flags) {
> +        dom = virDomainDefineXMLFlags(conn, xml, flags);
> +        /* If validate is the only flag, just drop it and
> +         * try again.
> +         */

This behavior is fine for virsh edit, where we enable validation by
default but if it is enabled explicitly by a user for virsh define, I
think we should propagate the error back instead of silently ignoring
the --validate option.

> +        if (!dom) {
> +            virErrorPtr err = virGetLastError();
> +            if (err &&
> +                (err->code == VIR_ERR_NO_SUPPORT) &&
> +                (flags == VIR_DOMAIN_DEFINE_VALIDATE))
> +                dom = virDomainDefineXML(conn, xml);
> +        }
> +    } else {
> +        dom = virDomainDefineXML(conn, xml);
> +    }
> +    return dom;
> +}
> +
> +
> +static virDomainPtr
> +vshDomainCreateXML(virConnectPtr conn, const char *xml,
> +                   size_t nfds, int *fds, unsigned int flags)
> +{
> +    virDomainPtr dom;
> +    if (nfds)
> +        dom = virDomainCreateXMLWithFiles(conn, xml,
> +                                          nfds, fds, flags);
> +    else
> +        dom = virDomainCreateXML(conn, xml, flags);
> +    /* If validate is set, just drop it and try again */
> +    if (!dom) {
> +        virErrorPtr err = virGetLastError();
> +        if (err &&
> +            (err->code == VIR_ERR_INVALID_ARG) &&
> +            (flags & VIR_DOMAIN_START_VALIDATE)) {
> +            flags &= ~VIR_DOMAIN_START_VALIDATE;
> +            if (nfds)
> +                dom = virDomainCreateXMLWithFiles(conn, xml,
> +                                                  nfds, fds, flags);
> +            else
> +                dom = virDomainCreateXML(conn, xml, flags);
> +        }
> +    }

We shouldn't need anything clever here since vshDomainCreateXML is
called with VIR_DOMAIN_START_VALIDATE iff it was explicitly requested.

> +    return dom;
> +}
> +
>  VIR_ENUM_DECL(vshDomainVcpuState)
>  VIR_ENUM_IMPL(vshDomainVcpuState,
>                VIR_VCPU_LAST,
...

Jirka