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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list