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@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@redhat.com> John [...]

On Fri, Jan 18, 2019 at 07:41:00PM +0400, 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@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/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index aa50eac424..7672639cb6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -81,6 +81,9 @@ <optional> <ref name='launchSecurity'/> </optional> + <optional> + <ref name='bhyvecmdline'/> + </optional> </interleave> </element> </define> @@ -6127,6 +6130,20 @@ </element> </define>

+ <!-- + Optional hypervisor extensions in their own namespace: + Bhyve + --> + <define name="bhyvecmdline"> + <element name="commandline" ns="http://libvirt.org/schemas/domain/bhyve/1.0"> + <zeroOrMore> + <element name="arg"> + <attribute name='value'/> + </element> + </zeroOrMore> + </element> + </define> + <!-- Type library --> diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 84fda08943..a1ae2026a0 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -28,6 +28,7 @@ #include "bhyve_capabilities.h" #include "bhyve_command.h" #include "bhyve_domain.h" +#include "bhyve_conf.h" #include "bhyve_driver.h" #include "datatypes.h" #include "viralloc.h" @@ -626,6 +627,14 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error;

+ if (def->namespaceData) { + bhyveDomainCmdlineDefPtr bhyvecmd; + + bhyvecmd = def->namespaceData; + for (i = 0; i < bhyvecmd->num_args; i++) + virCommandAddArg(cmd, bhyvecmd->args[i]); + } + virCommandAddArg(cmd, def->name);

return cmd; 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) +{ + 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_conf.h b/src/bhyve/bhyve_conf.h index 8da39fde7a..eb4a2e0fb8 100644 --- a/src/bhyve/bhyve_conf.h +++ b/src/bhyve/bhyve_conf.h @@ -29,4 +29,13 @@ virBhyveDriverConfigPtr virBhyveDriverGetConfig(bhyveConnPtr driver); int virBhyveLoadDriverConfig(virBhyveDriverConfigPtr cfg, const char *filename);

+typedef struct _bhyveDomainCmdlineDef bhyveDomainCmdlineDef; +typedef bhyveDomainCmdlineDef *bhyveDomainCmdlineDefPtr; +struct _bhyveDomainCmdlineDef { + size_t num_args; + char **args; +}; + +void bhyveDomainCmdlineDefFree(bhyveDomainCmdlineDefPtr def); + #endif /* LIBVIRT_BHYVE_CONF_H */ 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 @@ -20,16 +20,21 @@

#include <config.h>

+#include "bhyve_conf.h" #include "bhyve_device.h" #include "bhyve_domain.h" #include "bhyve_capabilities.h" #include "viralloc.h" #include "virlog.h"

+#include <libxml/xpathInternals.h> + #define VIR_FROM_THIS VIR_FROM_BHYVE

VIR_LOG_INIT("bhyve.bhyve_domain");

+#define BHYVE_NAMESPACE_HREF "http://libvirt.org/schemas/domain/bhyve/1.0" + static void * bhyveDomainObjPrivateAlloc(void *opaque ATTRIBUTE_UNUSED) { @@ -157,7 +162,8 @@ virBhyveDriverCreateXMLConf(bhyveConnPtr driver) virBhyveDriverDomainDefParserConfig.priv = driver; return virDomainXMLOptionNew(&virBhyveDriverDomainDefParserConfig, &virBhyveDriverPrivateDataCallbacks, - NULL, NULL, NULL); + &virBhyveDriverDomainXMLNamespace, + NULL, NULL); }

virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = { @@ -165,3 +171,102 @@ virDomainDefParserConfig virBhyveDriverDomainDefParserConfig = { .domainPostParseCallback = bhyveDomainDefPostParse, .assignAddressesCallback = bhyveDomainDefAssignAddresses, }; + +static void +bhyveDomainDefNamespaceFree(void *nsdata) +{ + bhyveDomainCmdlineDefPtr cmd = nsdata; + + bhyveDomainCmdlineDefFree(cmd); +} + +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; + + 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; +

This bool and the whole logic of this function looks overly complicated.

+ if (n && VIR_ALLOC_N(cmd->args, n) < 0) + goto error; + + 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; + } + cmd->num_args++; + } + + VIR_FREE(nodes); + + if (uses_bhyve_ns) + *data = cmd; + else + VIR_FREE(cmd);

Strange to see a different free function here, but it looks there's no wayt to get here with anything but bare 'cmd' allocated'

+ + return 0; + + error: + VIR_FREE(nodes); + bhyveDomainDefNamespaceFree(cmd); + return -1; +} + +static int +bhyveDomainDefNamespaceFormatXML(virBufferPtr buf ATTRIBUTE_UNUSED,

buf is used here

+ void *nsdata)

Indentation is off Jano

On Fri, Jan 18, 2019 at 07:41:00PM +0400, 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@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_command.c b/src/bhyve/bhyve_command.c index 84fda08943..a1ae2026a0 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -28,6 +28,7 @@ #include "bhyve_capabilities.h" #include "bhyve_command.h" #include "bhyve_domain.h" +#include "bhyve_conf.h" #include "bhyve_driver.h" #include "datatypes.h" #include "viralloc.h" @@ -626,6 +627,14 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, if (bhyveBuildConsoleArgStr(def, cmd) < 0) goto error;

+ if (def->namespaceData) { + bhyveDomainCmdlineDefPtr bhyvecmd; + + bhyvecmd = def->namespaceData; + for (i = 0; i < bhyvecmd->num_args; i++) + virCommandAddArg(cmd, bhyvecmd->args[i]); + }

There should be a VIR_WARN emitted in this block to mark the fact that the guest is booted using command line passthrough feature as that limits supportability of the guest. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On 1/18/19 10:41 AM, Roman Bogorodskiy wrote:

Document the <bhyve:commandline> element which allows to inject custom command line arguments for bhyve.

Signed-off-by: Roman Bogorodskiy <bogorodskiy@gmail.com> --- docs/drvbhyve.html.in | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)

diff --git a/docs/drvbhyve.html.in b/docs/drvbhyve.html.in index b4d7df2edb..d3c1585e33 100644 --- a/docs/drvbhyve.html.in +++ b/docs/drvbhyve.html.in @@ -460,6 +460,26 @@ Example:</p> </cpu> ... </domain> +</pre> + +<h3><a id="bhyvecommand">Pass-through of arbitrary bhyve commands</a></h3> + +<p><span class="since">Since 5.1.0</span>, it's possible to pass additional command-line +arguments to the bhyve process when starting the domain using the +<code><bhyve:commandline></code> element under <code>domain</code>.

To supply an argument, use the element <code><bhyve:arg</code> with the attribute <code>value</code> set to addition argument to be added.

+This element could be repeated multiple times. To use this XML addition, it is necessary

s/This element could be/The arg element may be/ Reviewed-by: John Ferlan <jferlan@redhat.com> John

+to issue an XML namespace request (the special <code>xmlns:<i>name</i></code> attribute) +that pulls in <code>http://libvirt.org/schemas/domain/bhyve/1.0</code>; +typically, the namespace is given the name of <code>bhyve</code>. +</p> +<p>Example:</p> +<pre> +<domain type="bhyve" xmlns:bhyve="http://libvirt.org/schemas/domain/bhyve/1.0">; + ... + <bhyve:commandline> + <bhyve:arg value='-somebhyvearg'/> + </bhyve:commandline> +</domain> </pre>

</body>