[libvirt] [PATCH 1/3] bhyve: implement support for commandline args
Roman Bogorodskiy
bogorodskiy at gmail.com
Tue Jan 29 16:17:18 UTC 2019
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 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/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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 455 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190129/038a466c/attachment-0001.sig>
More information about the libvir-list
mailing list