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

[Jim Fehlig]

On 8/25/20 9:15 AM, Michal Privoznik wrote:
> 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 at suse.com>
>> ---
>>   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.

Yes, good point. I'll remove it before pushing.

Thanks a lot for reviewing the series!

Regards,
Jim