[libvirt] [PATCH 1/3] bhyve: implement support for commandline args

[John Ferlan]

On 1/18/19 10:41 AM, Roman Bogorodskiy wrote:
> Implement support for passing custom command line arguments
> to bhyve using the 'bhyve:commandline' element:
> 
>   <bhyve:commandline>
>     <bhyve:arg value='-newarg'/>
>   </bhyve:commandline>
> 
>  * Define virDomainXMLNamespace for the bhyve driver, which
>    at this point supports only the 'commandline' element
>    described above,
>  * Update command generation code to inject these command line
>    arguments between driver-generated arguments and the vmname
>    positional argument.
> 
> Signed-off-by: Roman Bogorodskiy <bogorodskiy at gmail.com>
> ---
>  docs/schemas/domaincommon.rng                 |  17 +++
>  src/bhyve/bhyve_command.c                     |   9 ++
>  src/bhyve/bhyve_conf.c                        |  15 +++
>  src/bhyve/bhyve_conf.h                        |   9 ++
>  src/bhyve/bhyve_domain.c                      | 107 +++++++++++++++++-
>  src/bhyve/bhyve_domain.h                      |   1 +
>  .../bhyvexml2argv-commandline.args            |   9 ++
>  .../bhyvexml2argv-commandline.ldargs          |   3 +
>  .../bhyvexml2argv-commandline.xml             |  27 +++++
>  tests/bhyvexml2argvtest.c                     |   1 +
>  .../bhyvexml2xmlout-commandline.xml           |  37 ++++++
>  tests/bhyvexml2xmltest.c                      |   1 +
>  12 files changed, 235 insertions(+), 1 deletion(-)
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-commandline.ldargs
>  create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-commandline.xml
>  create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-commandline.xml
> 

[...]

> diff --git a/src/bhyve/bhyve_conf.c b/src/bhyve/bhyve_conf.c
> index 60baa2e848..75709801c7 100644
> --- a/src/bhyve/bhyve_conf.c
> +++ b/src/bhyve/bhyve_conf.c
> @@ -25,6 +25,7 @@
>  #include "virlog.h"
>  #include "virstring.h"
>  #include "bhyve_conf.h"
> +#include "bhyve_domain.h"
>  #include "configmake.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_BHYVE
> @@ -107,3 +108,17 @@ virBhyveDriverConfigDispose(void *obj)
>  
>      VIR_FREE(cfg->firmwareDir);
>  }
> +
> +void bhyveDomainCmdlineDefFree(bhyveDomainCmdlineDefPtr def)

NIT: for current conventions...

void
bhyveDomainCmdlineDefFree(bhyveDomainCmdlineDefPtr def)

> +{
> +    size_t i;
> +
> +    if (!def)
> +        return;
> +
> +    for (i = 0; i < def->num_args; i++)
> +        VIR_FREE(def->args[i]);
> +
> +    VIR_FREE(def->args);
> +    VIR_FREE(def);
> +}

[...]

> diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
> index e54af75f4d..554188ebeb 100644
> --- a/src/bhyve/bhyve_domain.c
> +++ b/src/bhyve/bhyve_domain.c

[...]

> +static int
> +bhyveDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED,
> +                             xmlNodePtr root ATTRIBUTE_UNUSED,
> +                             xmlXPathContextPtr ctxt,
> +                             void **data)
> +{
> +    bhyveDomainCmdlineDefPtr cmd = NULL;
> +    bool uses_bhyve_ns = false;
> +    xmlNodePtr *nodes = NULL;
> +    int n;
> +    size_t i;

Alternative:

   int ret = -1;

> +
> +    if (xmlXPathRegisterNs(ctxt, BAD_CAST "bhyve", BAD_CAST BHYVE_NAMESPACE_HREF) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Failed to register xml namespace '%s'"),
> +                       BHYVE_NAMESPACE_HREF);
> +        return -1;
> +    }
> +
> +    if (VIR_ALLOC(cmd) < 0)
> +        return -1;
> +
> +    n = virXPathNodeSet("./bhyve:commandline/bhyve:arg", ctxt, &nodes);
> +    if (n < 0)
> +        goto error;
> +    uses_bhyve_ns = n > 0;

Alternative:

    if (n == 0)
        ret = 0;
    if (n <= 0)
        goto cleanup;

> +
> +    if (n && VIR_ALLOC_N(cmd->args, n) < 0)
> +        goto error;

Alternative "goto cleanup;"

> +
> +    for (i = 0; i < n; i++) {
> +        cmd->args[cmd->num_args] = virXMLPropString(nodes[i], "value");
> +        if (cmd->args[cmd->num_args] == NULL) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           "%s", _("No bhyve command-line argument specified"));
> +            goto error;

Alternative "goto cleanup;"

> +        }
> +        cmd->num_args++;
> +    }
> +
> +    VIR_FREE(nodes);
> +
> +    if (uses_bhyve_ns)
> +        *data = cmd;
> +    else
> +        VIR_FREE(cmd);


Alternative:

    VIR_STEAL_PTR(*data, cmd);
    ret = 0;

 cleanup:
     VIR_FREE(nodes);
     bhyveDomainCmdlineDefFree(cmd);

     return ret;

> +
> +    return 0;
> +
> + error:
> +    VIR_FREE(nodes);
> +    bhyveDomainDefNamespaceFree(cmd);
> +    return -1;
> +}
> +

[...]

> diff --git a/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args
> new file mode 100644
> index 0000000000..cb21b99cd6
> --- /dev/null
> +++ b/tests/bhyvexml2argvdata/bhyvexml2argv-commandline.args
> @@ -0,0 +1,9 @@
> +/usr/sbin/bhyve \
> +-c 1 \
> +-m 214 \
> +-u \
> +-H \
> +-P \
> +-s 0:0,hostbridge \
> +-s 2:0,ahci,hd:/tmp/freebsd.img \
> +-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 ARGUMENT1 ARGUMENT2 bhyve

>From the not that matters but viewpoint:

-s 3:0,virtio-net,faketapdev,mac=52:54:00:b9:94:02 \
ARGUMENT1 ARGUMENT2 bhyve


Reviewed-by: John Ferlan <jferlan at redhat.com>

John

[...]