Re: [PATCH V2 1/3] Xen: Add support for qemu command-line passthrough

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 8/5/20 12:13 AM, Jim Fehlig wrote:
> Xen supports passing arbitrary arguments to the QEMU device model via
> the 'extra' member of the public libxl_domain_build_info structure.
> This patch adds a 'xen' namespace extension, similar to the QEMU and
> bhyve drivers, to map arbitrary arguments to the 'extra' member. Only
> passthrough of arguments is supported. Passthrough of environment
> variables or capabilities adjustments is not supported.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>   docs/drvxen.html.in           | 29 ++++++++++++
>   docs/schemas/domaincommon.rng | 17 +++++++
>   src/libxl/libxl_conf.c        | 10 ++++-
>   src/libxl/libxl_conf.h        |  8 ++++
>   src/libxl/libxl_domain.c      | 84 +++++++++++++++++++++++++++++++++++
>   src/libxl/libxl_domain.h      |  1 +
>   6 files changed, 148 insertions(+), 1 deletion(-)


> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index d9fcde4364..f6bc8211b3 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c

> +static int
> +libxlDomainDefNamespaceParse(xmlXPathContextPtr ctxt,
> +                             void **data)
> +{
> +    libxlDomainXmlNsDefPtr nsdata = NULL;
> +    g_autofree xmlNodePtr *nodes = NULL;
> +    ssize_t nnodes;
> +    size_t i;
> +    int ret = -1;
> +
> +    if ((nnodes = virXPathNodeSet("./xen:commandline/xen:arg", ctxt, &nodes)) < 0)
> +        return -1;
> +
> +    if (nnodes == 0)
> +        return 0;
> +
> +    nsdata = g_new0(libxlDomainXmlNsDef, 1);
> +    nsdata->args = g_new0(char *, nnodes + 1);
> +
> +    for (i = 0; i < nnodes; i++) {
> +        if (!(nsdata->args[nsdata->num_args++] = virXMLPropString(nodes[i], "value"))) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("No device model command-line argument specified"));
> +            goto cleanup;
> +        }
> +    }
> +
> +    if (nsdata->num_args > 0)
> +        *data = g_steal_pointer(&nsdata);

This if () looks redundant to me. The for-loop above in combination with 
nnodes = 0 check ensures that num_args is greater than zero here.

> +
> +    ret = 0;
> +
> + cleanup:
> +    libxlDomainDefNamespaceFree(nsdata);
> +    return ret;
> +}
> +

Michal