[libvirt] [PATCH 05/10] Handle arbitrary qemu command-lines in qemuParseCommandLine.

[Daniel P. Berrange]

On Wed, Apr 21, 2010 at 12:01:19PM -0400, Chris Lalancette wrote:
> Now that we have the ability to specify arbitrary qemu
> command-line parameters in the XML, use it to handle unknown
> command-line parameters when doing a native-to-xml conversion.
> 
> Signed-off-by: Chris Lalancette <clalance at redhat.com>
> ---
>  src/conf/domain_conf.c   |   13 +++++++++----
>  src/conf/domain_conf.h   |    2 ++
>  src/libvirt_private.syms |    1 +
>  src/qemu/qemu_conf.c     |   27 +++++++++++++++++++++------
>  4 files changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a374206..3f13f32 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3719,6 +3719,14 @@ static char *virDomainDefDefaultEmulator(virDomainDefPtr def,
>      return retemu;
>  }
>  
> +void virDomainDefAssignNamespace(virCapsPtr caps, virDomainDefPtr def)
> +{
> +    def->ns.parse = caps->ns->parse;
> +    def->ns.free = caps->ns->free;
> +    def->ns.format = caps->ns->format;
> +    def->ns.href = caps->ns->href;
> +}
> +
>  static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>                                              xmlDocPtr xml,
>                                              xmlNodePtr root,
> @@ -4377,10 +4385,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>          /* we have to make a copy of all of the callback pointers here since
>           * we won't have the virCaps structure available during free
>           */
> -        def->ns.parse = caps->ns->parse;
> -        def->ns.free = caps->ns->free;
> -        def->ns.format = caps->ns->format;
> -        def->ns.href = caps->ns->href;
> +        virDomainDefAssignNamespace(caps, def);


This could mostly go away, if we just directly passed around the statically
allocated driver table.

> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 95843bf..c4b55de 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -5704,6 +5704,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>      const char **nics = NULL;
>      int video = VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
>      int nvirtiodisk = 0;
> +    qemuDomainCmdlineDefPtr cmd;
>  
>      if (!progargv[0]) {
>          qemuReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -5714,6 +5715,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>      if (VIR_ALLOC(def) < 0)
>          goto no_memory;
>  
> +    /* allocate the cmdlinedef up-front; if it's unused, we'll free it later */
> +    if (VIR_ALLOC(cmd) < 0)
> +        goto no_memory;
> +
>      virUUIDGenerate(def->uuid);
>  
>      def->id = -1;
> @@ -6138,12 +6143,15 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>          } else if (STREQ(arg, "-S")) {
>              /* ignore, always added by libvirt */
>          } else {
> -            VIR_WARN(_("unknown QEMU argument '%s' during conversion"), arg);
> -#if 0
> -            qemuReportError(VIR_ERR_INTERNAL_ERROR,
> -                            _("unknown argument '%s'"), arg);
> -            goto error;
> -#endif
> +            /* something we can't yet parse.  Add it to the qemu namespace
> +             * cmdline/environment advanced options and hope for the best
> +             */
> +            if (VIR_REALLOC_N(cmd->extra, cmd->num_extra+1) < 0)
> +                goto no_memory;
> +            cmd->extra[cmd->num_extra] = strdup(arg);
> +            if (cmd->extra[cmd->num_extra] == NULL)
> +                goto no_memory;
> +            cmd->num_extra++;
>          }
>      }

IIRC, there are other places where we know about the arg, but don't know
how to handle its values. We can deal with that later though.

>  
> @@ -6203,6 +6211,13 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
>      if (virDomainDefAddImplicitControllers(def) < 0)
>          goto error;
>  
> +    if (cmd->num_extra || cmd->num_env) {
> +        virDomainDefAssignNamespace(caps, def);
> +        def->namespaceData = cmd;
> +    }
> +    else
> +        VIR_FREE(cmd);
> +
>      return def;
>  
>  no_memory:

IIUC this is leaking 'cmd' in the no_memory:  path ?


Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|