10:17 a.m.
Ján Tomko wrote:
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(a)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.
Yeah, I rewrote it without it as John suggested.
BTW, I used qemuDomainDefNamespaceParse() as a reference, so probably
it may be simplified as well.
>+ 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
Indeed.
>+ void *nsdata)
Indentation is off
Indeed. As it's already pushed, I'll fix it in a follow-up series. I'll
have to send a follow-up series anyway to updates docs and add a warning
as Daniel suggested.
Jano
Roman Bogorodskiy