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

[Chris Lalancette]

On 07/01/10 - 03:39:25PM, Eric Blake wrote:
> >  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.

Hm, interesting, I didn't know that rule about environment variable names.
That is a good check to make, I'll add it.

> 
> > +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?

Yeah, I can invert the logic and do that instead.  Since I'll be respinning
the patch for the above environment variable checking, I'll change it.

--
Chris Lalancette