Re: [libvirt] [PATCH v2 REPOST 2/8] Qemu arbitrary command-line arguments.
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