[libvirt] [PATCH v2 REPOST 2/8] Qemu arbitrary command-line arguments.

[Eric Blake]

On 07/01/2010 02:59 PM, Chris Lalancette wrote:
> Implement the qemu hooks for XML namespace data.  This
> allows us to specify a qemu XML namespace, and then
> specify:
> 
> <qemu:commandline>
>  <qemu:arg value='arg'/>
>  <qemu:env name='name' value='value'/>
> </qemu:commandline>
> 
> In the domain XML.
> 
> Changes since v1:
>  - Change the <qemu:arg>arg</qemu:arg> XML to <qemu:arg value='arg'/> XML
>  - Fix up some memory leaks in qemuDomainDefNamespaceParse
>  - Rename num_extra and extra to num_args and args, respectively
>  - Fixed up some error messages
>  - Make sure to escape user-provided data in qemuDomainDefNamesapceFormatXML
> 
> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> ---
>  src/qemu/qemu_conf.c   |   14 +++++
>  src/qemu/qemu_conf.h   |   11 ++++
>  src/qemu/qemu_driver.c |  150 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 175 insertions(+), 0 deletions(-)
> 
>  
> +    if (def->namespaceData) {
> +        qemuDomainCmdlineDefPtr cmd;
> +
> +        cmd = def->namespaceData;
> +        for (i = 0; i < cmd->num_args; i++)
> +            ADD_ARG_LIT(cmd->args[i]);

It would be nice if we had the new task creation API stablized and
implemented, but for now, this is reasonable.

> +    for (i = 0; i < n; i++) {
> +        cmd->env_name[cmd->num_env] = virXMLPropString(nodes[i], "name");
> +        if (cmd->env_name[cmd->num_env] == NULL) {
> +            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> +                            "%s", _("No qemu environment name specified"));
> +            goto error;

Do we need to validate that the resulting name is valid (starts with a
letter, and contains only alphanumeric and _)?  arg and env_value can
obviously be arbitrary strings, but not env_name.

> +static int qemuDomainDefNamespaceFormatXML(virBufferPtr buf,
> +                                           void *nsdata)
> +{
> +    qemuDomainCmdlineDefPtr cmd = nsdata;
> +    unsigned int i;
> +
> +    if (cmd->num_args || cmd->num_env)
> +        virBufferAddLit(buf, "  <qemu:commandline>\n");

Rather than check cmd->num_args and cmd->num_env here and again below,
why not just do an early return 0 here?

> +    for (i = 0; i < cmd->num_args; i++)
> +        virBufferEscapeString(buf, "    <qemu:arg value='%s'/>\n",
> +                              cmd->args[i]);
> +    for (i = 0; i < cmd->num_env; i++) {
> +        virBufferVSprintf(buf, "    <qemu:env name='%s'", cmd->env_name[i]);
> +        if (cmd->env_value[i])
> +            virBufferEscapeString(buf, " value='%s'", cmd->env_value[i]);
> +        virBufferAddLit(buf, "/>\n");
> +    }
> +    if (cmd->num_args || cmd->num_env)
> +        virBufferAddLit(buf, "  </qemu:commandline>\n");
> +
> +    return 0;
> +}
> +
> +static const char *qemuDomainDefNamespaceHref(void)
> +{
> +    return "xmlns:qemu='" QEMU_NAMESPACE_HREF "'";
> +}

May need a tweak to add an ATTRIBUTE_IGNORED parameter depending on your
reaction to my comment on the 1/8 patch.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100701/5eb4513b/attachment-0001.sig>