[libvirt] [PATCH 0/5] network: xmlns dnsmasq option passthrough
There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options. This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented. Cole Robinson (5): conf: Add virNetworkXMLOption conf: Add network xmlopt argument conf: Add virNetworkXMLNamespace network: wire up dnsmasq option xmlns docs: formatnetwork: Document xmlns:dnsmasq docs/formatnetwork.html.in | 22 +++ docs/schemas/network.rng | 11 ++ src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 91 +++++++-- src/conf/network_conf.h | 51 ++++- src/conf/virnetworkobj.c | 50 +++-- src/conf/virnetworkobj.h | 13 +- src/esx/esx_network_driver.c | 4 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 182 ++++++++++++++++-- src/network/bridge_driver.h | 12 ++ src/network/bridge_driver_platform.h | 2 + src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 11 +- src/vbox/vbox_network.c | 4 +- tests/Makefile.am | 14 +- .../networkxml2confdata/dnsmasq-options.conf | 18 ++ tests/networkxml2confdata/dnsmasq-options.xml | 15 ++ tests/networkxml2conftest.c | 8 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmlin/dnsmasq-options.xml | 15 ++ tests/networkxml2xmlout/dnsmasq-options.xml | 17 ++ tests/networkxml2xmltest.c | 11 +- tests/networkxml2xmlupdatetest.c | 4 +- 24 files changed, 478 insertions(+), 84 deletions(-) create mode 100644 tests/networkxml2confdata/dnsmasq-options.conf create mode 100644 tests/networkxml2confdata/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlin/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlout/dnsmasq-options.xml -- 2.21.0
Just a stub for now that is unused. Add init+cleanup plumbing and demostrate it in bridge_driver.c Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/network_conf.c | 33 ++++++++++++++++++++++++++++ src/conf/network_conf.h | 11 ++++++++++ src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 10 +++++++++ src/network/bridge_driver_platform.h | 2 ++ 5 files changed, 57 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 09e379ae9a..ca4b9986ad 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -73,6 +73,39 @@ VIR_ENUM_IMPL(virNetworkTaint, "hook-script", ); +static virClassPtr virNetworkXMLOptionClass; + +static void +virNetworkXMLOptionDispose(void *obj ATTRIBUTE_UNUSED) +{ + return; +} + +static int +virNetworkXMLOnceInit(void) +{ + if (!VIR_CLASS_NEW(virNetworkXMLOption, virClassForObject())) + return -1; + + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNetworkXML); + +virNetworkXMLOptionPtr +virNetworkXMLOptionNew(void) +{ + virNetworkXMLOptionPtr xmlopt; + + if (virNetworkXMLInitialize() < 0) + return NULL; + + if (!(xmlopt = virObjectNew(virNetworkXMLOptionClass))) + return NULL; + + return xmlopt; +} + static void virPortGroupDefClear(virPortGroupDefPtr def) { diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 424c4b0913..edd9f51f44 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -41,6 +41,14 @@ #include "virmacmap.h" #include "virenum.h" + +struct _virNetworkXMLOption { + virObject parent; +}; +typedef struct _virNetworkXMLOption virNetworkXMLOption; +typedef virNetworkXMLOption *virNetworkXMLOptionPtr; + + typedef enum { VIR_NETWORK_FORWARD_NONE = 0, VIR_NETWORK_FORWARD_NAT, @@ -289,6 +297,9 @@ enum { VIR_NETWORK_OBJ_LIST_ADD_CHECK_LIVE = (1 << 1), }; +virNetworkXMLOptionPtr +virNetworkXMLOptionNew(void); + virNetworkDefPtr virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7dfa5af3b3..5aec33cf52 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -730,6 +730,7 @@ virNetworkSaveConfig; virNetworkSetBridgeMacAddr; virNetworkTaintTypeFromString; virNetworkTaintTypeToString; +virNetworkXMLOptionNew; virPortGroupFindByName; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 6292e3b90a..5be6f1ba45 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -136,6 +136,12 @@ networkDnsmasqCapsRefresh(virNetworkDriverStatePtr driver) return 0; } +static virNetworkXMLOptionPtr +networkDnsmasqCreateXMLConf(void) +{ + return virNetworkXMLOptionNew(); +} + static int networkStateCleanup(void); @@ -605,6 +611,9 @@ networkStateInitialize(bool privileged, network_driver->privileged = privileged; + if (!(network_driver->xmlopt = networkDnsmasqCreateXMLConf())) + goto error; + /* configuration/state paths are one of * ~/.config/libvirt/... (session/unprivileged) * /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged). @@ -766,6 +775,7 @@ networkStateCleanup(void) return -1; virObjectUnref(network_driver->networkEventState); + virObjectUnref(network_driver->xmlopt); /* free inactive networks */ virObjectUnref(network_driver->networks); diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h index 95993c5e31..169417a6c0 100644 --- a/src/network/bridge_driver_platform.h +++ b/src/network/bridge_driver_platform.h @@ -55,6 +55,8 @@ struct _virNetworkDriverState { /* Immutable pointer, self-locking APIs */ virObjectEventStatePtr networkEventState; + + virNetworkXMLOptionPtr xmlopt; }; typedef struct _virNetworkDriverState virNetworkDriverState; -- 2.21.0
Pass an xmlopt argument through all the needed network conf functions, like is done for domain XML handling. No functional change for now Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 40 +++++++++++++++---------- src/conf/network_conf.h | 21 ++++++++++---- src/conf/virnetworkobj.c | 50 ++++++++++++++++++++------------ src/conf/virnetworkobj.h | 13 ++++++--- src/esx/esx_network_driver.c | 4 +-- src/network/bridge_driver.c | 46 ++++++++++++++++++----------- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 11 +++---- src/vbox/vbox_network.c | 4 +-- tests/networkxml2conftest.c | 2 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmltest.c | 4 +-- tests/networkxml2xmlupdatetest.c | 4 +-- 14 files changed, 128 insertions(+), 77 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3323c9a5b1..740da645c4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30932,7 +30932,7 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface) if (!(xml = virNetworkGetXMLDesc(net, 0))) goto cleanup; - if (!(def = virNetworkDefParseString(xml))) + if (!(def = virNetworkDefParseString(xml, NULL))) goto cleanup; switch ((virNetworkForwardType) def->forward.type) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index ca4b9986ad..067adf7936 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -282,7 +282,9 @@ virNetworkDefFree(virNetworkDefPtr def) * Returns a new NetworkDef on success, or NULL on failure. */ virNetworkDefPtr -virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags) +virNetworkDefCopy(virNetworkDefPtr def, + virNetworkXMLOptionPtr xmlopt, + unsigned int flags) { char *xml = NULL; virNetworkDefPtr newDef = NULL; @@ -294,9 +296,9 @@ virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags) } /* deep copy with a format/parse cycle */ - if (!(xml = virNetworkDefFormat(def, flags))) + if (!(xml = virNetworkDefFormat(def, xmlopt, flags))) goto cleanup; - newDef = virNetworkDefParseString(xml); + newDef = virNetworkDefParseString(xml, xmlopt); cleanup: VIR_FREE(xml); return newDef; @@ -1619,7 +1621,8 @@ virNetworkForwardDefParseXML(const char *networkName, virNetworkDefPtr -virNetworkDefParseXML(xmlXPathContextPtr ctxt) +virNetworkDefParseXML(xmlXPathContextPtr ctxt, + virNetworkXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) { virNetworkDefPtr def; char *tmp = NULL; @@ -2059,14 +2062,15 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) static virNetworkDefPtr virNetworkDefParse(const char *xmlStr, - const char *filename) + const char *filename, + virNetworkXMLOptionPtr xmlopt) { xmlDocPtr xml; virNetworkDefPtr def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)")))) { - def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml)); + def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt); xmlFreeDoc(xml); } @@ -2076,22 +2080,25 @@ virNetworkDefParse(const char *xmlStr, virNetworkDefPtr -virNetworkDefParseString(const char *xmlStr) +virNetworkDefParseString(const char *xmlStr, + virNetworkXMLOptionPtr xmlopt) { - return virNetworkDefParse(xmlStr, NULL); + return virNetworkDefParse(xmlStr, NULL, xmlopt); } virNetworkDefPtr -virNetworkDefParseFile(const char *filename) +virNetworkDefParseFile(const char *filename, + virNetworkXMLOptionPtr xmlopt) { - return virNetworkDefParse(NULL, filename); + return virNetworkDefParse(NULL, filename, xmlopt); } virNetworkDefPtr virNetworkDefParseNode(xmlDocPtr xml, - xmlNodePtr root) + xmlNodePtr root, + virNetworkXMLOptionPtr xmlopt) { xmlXPathContextPtr ctxt = NULL; virNetworkDefPtr def = NULL; @@ -2111,7 +2118,7 @@ virNetworkDefParseNode(xmlDocPtr xml, } ctxt->node = root; - def = virNetworkDefParseXML(ctxt); + def = virNetworkDefParseXML(ctxt, xmlopt); cleanup: xmlXPathFreeContext(ctxt); @@ -2405,6 +2412,7 @@ virNetworkForwardNatDefFormat(virBufferPtr buf, int virNetworkDefFormatBuf(virBufferPtr buf, const virNetworkDef *def, + virNetworkXMLOptionPtr xmlopt ATTRIBUTE_UNUSED, unsigned int flags) { const unsigned char *uuid; @@ -2631,11 +2639,12 @@ virNetworkDefFormatBuf(virBufferPtr buf, char * virNetworkDefFormat(const virNetworkDef *def, + virNetworkXMLOptionPtr xmlopt, unsigned int flags) { virBuffer buf = VIR_BUFFER_INITIALIZER; - if (virNetworkDefFormatBuf(&buf, def, flags) < 0) + if (virNetworkDefFormatBuf(&buf, def, xmlopt, flags) < 0) goto error; if (virBufferCheckError(&buf) < 0) @@ -2709,12 +2718,13 @@ virNetworkSaveXML(const char *configDir, int virNetworkSaveConfig(const char *configDir, - virNetworkDefPtr def) + virNetworkDefPtr def, + virNetworkXMLOptionPtr xmlopt) { int ret = -1; char *xml; - if (!(xml = virNetworkDefFormat(def, VIR_NETWORK_XML_INACTIVE))) + if (!(xml = virNetworkDefFormat(def, xmlopt, VIR_NETWORK_XML_INACTIVE))) goto cleanup; if (virNetworkSaveXML(configDir, def, xml)) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index edd9f51f44..b7ce569d4a 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -301,28 +301,36 @@ virNetworkXMLOptionPtr virNetworkXMLOptionNew(void); virNetworkDefPtr -virNetworkDefCopy(virNetworkDefPtr def, unsigned int flags); +virNetworkDefCopy(virNetworkDefPtr def, + virNetworkXMLOptionPtr xmlopt, + unsigned int flags); virNetworkDefPtr -virNetworkDefParseXML(xmlXPathContextPtr ctxt); +virNetworkDefParseXML(xmlXPathContextPtr ctxt, + virNetworkXMLOptionPtr xmlopt); virNetworkDefPtr -virNetworkDefParseString(const char *xmlStr); +virNetworkDefParseString(const char *xmlStr, + virNetworkXMLOptionPtr xmlopt); virNetworkDefPtr -virNetworkDefParseFile(const char *filename); +virNetworkDefParseFile(const char *filename, + virNetworkXMLOptionPtr xmlopt); virNetworkDefPtr virNetworkDefParseNode(xmlDocPtr xml, - xmlNodePtr root); + xmlNodePtr root, + virNetworkXMLOptionPtr xmlopt); char * virNetworkDefFormat(const virNetworkDef *def, + virNetworkXMLOptionPtr xmlopt, unsigned int flags); int virNetworkDefFormatBuf(virBufferPtr buf, const virNetworkDef *def, + virNetworkXMLOptionPtr xmlopt, unsigned int flags); const char * @@ -357,7 +365,8 @@ virNetworkSaveXML(const char *configDir, int virNetworkSaveConfig(const char *configDir, - virNetworkDefPtr def); + virNetworkDefPtr def, + virNetworkXMLOptionPtr xmlopt); char * virNetworkConfigFile(const char *dir, diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 476f66affc..d63ead7fac 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -673,7 +673,8 @@ virNetworkObjAssignDef(virNetworkObjListPtr nets, */ int virNetworkObjSetDefTransient(virNetworkObjPtr obj, - bool live) + bool live, + virNetworkXMLOptionPtr xmlopt) { if (!virNetworkObjIsActive(obj) && !live) return 0; @@ -681,7 +682,9 @@ virNetworkObjSetDefTransient(virNetworkObjPtr obj, if (!obj->persistent || obj->newDef) return 0; - obj->newDef = virNetworkDefCopy(obj->def, VIR_NETWORK_XML_INACTIVE); + obj->newDef = virNetworkDefCopy(obj->def, + xmlopt, + VIR_NETWORK_XML_INACTIVE); return obj->newDef ? 0 : -1; } @@ -759,6 +762,7 @@ virNetworkObjReplacePersistentDef(virNetworkObjPtr obj, */ static int virNetworkObjConfigChangeSetup(virNetworkObjPtr obj, + virNetworkXMLOptionPtr xmlopt, unsigned int flags) { bool isActive; @@ -782,7 +786,7 @@ virNetworkObjConfigChangeSetup(virNetworkObjPtr obj, /* this should already have been done by the driver, but do it * anyway just in case. */ - if (isActive && (virNetworkObjSetDefTransient(obj, false) < 0)) + if (isActive && (virNetworkObjSetDefTransient(obj, false, xmlopt) < 0)) goto cleanup; } @@ -811,6 +815,7 @@ virNetworkObjRemoveInactive(virNetworkObjListPtr nets, static char * virNetworkObjFormat(virNetworkObjPtr obj, + virNetworkXMLOptionPtr xmlopt, unsigned int flags) { virBuffer buf = VIR_BUFFER_INITIALIZER; @@ -832,7 +837,7 @@ virNetworkObjFormat(virNetworkObjPtr obj, virNetworkTaintTypeToString(i)); } - if (virNetworkDefFormatBuf(&buf, obj->def, flags) < 0) + if (virNetworkDefFormatBuf(&buf, obj->def, xmlopt, flags) < 0) goto error; virBufferAdjustIndent(&buf, -2); @@ -851,13 +856,14 @@ virNetworkObjFormat(virNetworkObjPtr obj, int virNetworkObjSaveStatus(const char *statusDir, - virNetworkObjPtr obj) + virNetworkObjPtr obj, + virNetworkXMLOptionPtr xmlopt) { int ret = -1; int flags = 0; char *xml; - if (!(xml = virNetworkObjFormat(obj, flags))) + if (!(xml = virNetworkObjFormat(obj, xmlopt, flags))) goto cleanup; if (virNetworkSaveXML(statusDir, obj->def, xml)) @@ -873,7 +879,8 @@ virNetworkObjSaveStatus(const char *statusDir, static virNetworkObjPtr virNetworkLoadState(virNetworkObjListPtr nets, const char *stateDir, - const char *name) + const char *name, + virNetworkXMLOptionPtr xmlopt) { char *configFile = NULL; virNetworkDefPtr def = NULL; @@ -902,7 +909,7 @@ virNetworkLoadState(virNetworkObjListPtr nets, /* parse the definition first */ ctxt->node = node; - if (!(def = virNetworkDefParseXML(ctxt))) + if (!(def = virNetworkDefParseXML(ctxt, xmlopt))) goto error; if (STRNEQ(name, def->name)) { @@ -1000,7 +1007,8 @@ static virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets, const char *configDir, const char *autostartDir, - const char *name) + const char *name, + virNetworkXMLOptionPtr xmlopt) { char *configFile = NULL, *autostartLink = NULL; virNetworkDefPtr def = NULL; @@ -1015,7 +1023,7 @@ virNetworkLoadConfig(virNetworkObjListPtr nets, if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) goto error; - if (!(def = virNetworkDefParseFile(configFile))) + if (!(def = virNetworkDefParseFile(configFile, xmlopt))) goto error; if (STRNEQ(name, def->name)) { @@ -1033,7 +1041,7 @@ virNetworkLoadConfig(virNetworkObjListPtr nets, case VIR_NETWORK_FORWARD_OPEN: if (!def->mac_specified) { virNetworkSetBridgeMacAddr(def); - virNetworkSaveConfig(configDir, def); + virNetworkSaveConfig(configDir, def, xmlopt); } break; @@ -1073,7 +1081,8 @@ virNetworkLoadConfig(virNetworkObjListPtr nets, int virNetworkObjLoadAllState(virNetworkObjListPtr nets, - const char *stateDir) + const char *stateDir, + virNetworkXMLOptionPtr xmlopt) { DIR *dir; struct dirent *entry; @@ -1089,7 +1098,7 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets, if (!virStringStripSuffix(entry->d_name, ".xml")) continue; - obj = virNetworkLoadState(nets, stateDir, entry->d_name); + obj = virNetworkLoadState(nets, stateDir, entry->d_name, xmlopt); if (obj && virNetworkObjLoadAllPorts(obj, stateDir) < 0) { @@ -1108,7 +1117,8 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets, int virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets, const char *configDir, - const char *autostartDir) + const char *autostartDir, + virNetworkXMLOptionPtr xmlopt) { DIR *dir; struct dirent *entry; @@ -1129,7 +1139,8 @@ virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets, obj = virNetworkLoadConfig(nets, configDir, autostartDir, - entry->d_name); + entry->d_name, + xmlopt); virNetworkObjEndAPI(&obj); } @@ -1239,20 +1250,21 @@ virNetworkObjUpdate(virNetworkObjPtr obj, unsigned int section, /* virNetworkUpdateSection */ int parentIndex, const char *xml, + virNetworkXMLOptionPtr xmlopt, unsigned int flags) /* virNetworkUpdateFlags */ { int ret = -1; virNetworkDefPtr livedef = NULL, configdef = NULL; /* normalize config data, and check for common invalid requests. */ - if (virNetworkObjConfigChangeSetup(obj, flags) < 0) + if (virNetworkObjConfigChangeSetup(obj, xmlopt, flags) < 0) goto cleanup; if (flags & VIR_NETWORK_UPDATE_AFFECT_LIVE) { virNetworkDefPtr checkdef; /* work on a copy of the def */ - if (!(livedef = virNetworkDefCopy(obj->def, 0))) + if (!(livedef = virNetworkDefCopy(obj->def, xmlopt, 0))) goto cleanup; if (virNetworkDefUpdateSection(livedef, command, section, parentIndex, xml, flags) < 0) { @@ -1261,7 +1273,7 @@ virNetworkObjUpdate(virNetworkObjPtr obj, /* run a final format/parse cycle to make sure we didn't * add anything illegal to the def */ - if (!(checkdef = virNetworkDefCopy(livedef, 0))) + if (!(checkdef = virNetworkDefCopy(livedef, xmlopt, 0))) goto cleanup; virNetworkDefFree(checkdef); } @@ -1271,6 +1283,7 @@ virNetworkObjUpdate(virNetworkObjPtr obj, /* work on a copy of the def */ if (!(configdef = virNetworkDefCopy(virNetworkObjGetPersistentDef(obj), + xmlopt, VIR_NETWORK_XML_INACTIVE))) { goto cleanup; } @@ -1279,6 +1292,7 @@ virNetworkObjUpdate(virNetworkObjPtr obj, goto cleanup; } if (!(checkdef = virNetworkDefCopy(configdef, + xmlopt, VIR_NETWORK_XML_INACTIVE))) { goto cleanup; } diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h index e649b6b350..1c28f0888c 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -140,7 +140,8 @@ virNetworkObjUpdateAssignDef(virNetworkObjPtr network, int virNetworkObjSetDefTransient(virNetworkObjPtr network, - bool live); + bool live, + virNetworkXMLOptionPtr xmlopt); void virNetworkObjUnsetDefTransient(virNetworkObjPtr network); @@ -191,16 +192,19 @@ virNetworkObjPortListExport(virNetworkPtr net, int virNetworkObjSaveStatus(const char *statusDir, - virNetworkObjPtr net) ATTRIBUTE_RETURN_CHECK; + virNetworkObjPtr net, + virNetworkXMLOptionPtr xmlopt) ATTRIBUTE_RETURN_CHECK; int virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets, const char *configDir, - const char *autostartDir); + const char *autostartDir, + virNetworkXMLOptionPtr xmlopt); int virNetworkObjLoadAllState(virNetworkObjListPtr nets, - const char *stateDir); + const char *stateDir, + virNetworkXMLOptionPtr xmlopt); int virNetworkObjDeleteConfig(const char *configDir, @@ -218,6 +222,7 @@ virNetworkObjUpdate(virNetworkObjPtr obj, unsigned int section, /* virNetworkUpdateSection */ int parentIndex, const char *xml, + virNetworkXMLOptionPtr xmlopt, unsigned int flags); /* virNetworkUpdateFlags */ int diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 31bceb7bff..40bd5c2168 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -298,7 +298,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) return NULL; /* Parse network XML */ - def = virNetworkDefParseString(xml); + def = virNetworkDefParseString(xml, NULL); if (!def) return NULL; @@ -806,7 +806,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) } } - xml = virNetworkDefFormat(def, flags); + xml = virNetworkDefFormat(def, NULL, flags); cleanup: esxVI_HostVirtualSwitch_Free(&hostVirtualSwitch); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5be6f1ba45..3353754eb5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -231,7 +231,7 @@ networkRunHook(virNetworkObjPtr obj, virBufferAddLit(&buf, "<hookData>\n"); virBufferAdjustIndent(&buf, 2); - if (virNetworkDefFormatBuf(&buf, def, 0) < 0) + if (virNetworkDefFormatBuf(&buf, def, network_driver->xmlopt, 0) < 0) goto cleanup; if (port && virNetworkPortDefFormatBuf(&buf, port) < 0) goto cleanup; @@ -673,12 +673,14 @@ networkStateInitialize(bool privileged, goto error; if (virNetworkObjLoadAllState(network_driver->networks, - network_driver->stateDir) < 0) + network_driver->stateDir, + network_driver->xmlopt) < 0) goto error; if (virNetworkObjLoadAllConfigs(network_driver->networks, network_driver->networkConfigDir, - network_driver->networkAutostartDir) < 0) + network_driver->networkAutostartDir, + network_driver->xmlopt) < 0) goto error; /* Update the internal status of all allegedly active @@ -750,10 +752,12 @@ networkStateReload(void) return 0; virNetworkObjLoadAllState(network_driver->networks, - network_driver->stateDir); + network_driver->stateDir, + network_driver->xmlopt); virNetworkObjLoadAllConfigs(network_driver->networks, network_driver->networkConfigDir, - network_driver->networkAutostartDir); + network_driver->networkAutostartDir, + network_driver->xmlopt); networkReloadFirewallRules(network_driver, false); networkRefreshDaemons(network_driver); virNetworkObjListForEach(network_driver->networks, @@ -2789,7 +2793,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver, virNetworkObjDeleteAllPorts(obj, driver->stateDir); VIR_DEBUG("Setting current network def as transient"); - if (virNetworkObjSetDefTransient(obj, true) < 0) + if (virNetworkObjSetDefTransient(obj, true, network_driver->xmlopt) < 0) goto cleanup; /* Run an early hook to set-up missing devices. @@ -2847,7 +2851,8 @@ networkStartNetwork(virNetworkDriverStatePtr driver, * is setup. */ VIR_DEBUG("Writing network status to disk"); - if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) + if (virNetworkObjSaveStatus(driver->stateDir, + obj, network_driver->xmlopt) < 0) goto cleanup; virNetworkObjSetActive(obj, true); @@ -3563,7 +3568,7 @@ networkCreateXML(virConnectPtr conn, virNetworkPtr net = NULL; virObjectEventPtr event = NULL; - if (!(newDef = virNetworkDefParseString(xml))) + if (!(newDef = virNetworkDefParseString(xml, network_driver->xmlopt))) goto cleanup; if (virNetworkCreateXMLEnsureACL(conn, newDef) < 0) @@ -3615,7 +3620,7 @@ networkDefineXML(virConnectPtr conn, virNetworkPtr net = NULL; virObjectEventPtr event = NULL; - if (!(def = virNetworkDefParseString(xml))) + if (!(def = virNetworkDefParseString(xml, network_driver->xmlopt))) goto cleanup; if (virNetworkDefineXMLEnsureACL(conn, def) < 0) @@ -3630,7 +3635,8 @@ networkDefineXML(virConnectPtr conn, /* def was assigned to network object */ freeDef = false; - if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) { + if (virNetworkSaveConfig(driver->networkConfigDir, + def, network_driver->xmlopt) < 0) { if (!virNetworkObjIsActive(obj)) { virNetworkObjRemoveInactive(driver->networks, obj); goto cleanup; @@ -3811,7 +3817,9 @@ networkUpdate(virNetworkPtr net, } /* update the network config in memory/on disk */ - if (virNetworkObjUpdate(obj, command, section, parentIndex, xml, flags) < 0) { + if (virNetworkObjUpdate(obj, command, section, + parentIndex, xml, + network_driver->xmlopt, flags) < 0) { if (needFirewallRefresh) ignore_value(networkAddFirewallRules(def)); goto cleanup; @@ -3826,7 +3834,8 @@ networkUpdate(virNetworkPtr net, if (flags & VIR_NETWORK_UPDATE_AFFECT_CONFIG) { /* save updated persistent config to disk */ if (virNetworkSaveConfig(driver->networkConfigDir, - virNetworkObjGetPersistentDef(obj)) < 0) { + virNetworkObjGetPersistentDef(obj), + network_driver->xmlopt) < 0) { goto cleanup; } } @@ -3893,7 +3902,8 @@ networkUpdate(virNetworkPtr net, } /* save current network state to disk */ - if ((ret = virNetworkObjSaveStatus(driver->stateDir, obj)) < 0) + if ((ret = virNetworkObjSaveStatus(driver->stateDir, + obj, network_driver->xmlopt)) < 0) goto cleanup; } @@ -4014,7 +4024,7 @@ networkGetXMLDesc(virNetworkPtr net, else curDef = def; - ret = virNetworkDefFormat(curDef, flags); + ret = virNetworkDefFormat(curDef, network_driver->xmlopt, flags); cleanup: virNetworkObjEndAPI(&obj); @@ -5153,7 +5163,7 @@ networkPlugBandwidthImpl(virNetworkObjPtr obj, tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); /* update status file */ - if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) { + if (virNetworkObjSaveStatus(driver->stateDir, obj, network_driver->xmlopt) < 0) { ignore_value(virBitmapClearBit(classIdMap, next_id)); tmp_floor_sum -= ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); @@ -5243,7 +5253,8 @@ networkUnplugBandwidth(virNetworkObjPtr obj, /* return class ID */ ignore_value(virBitmapClearBit(classIdMap, *class_id)); /* update status file */ - if (virNetworkObjSaveStatus(driver->stateDir, obj) < 0) { + if (virNetworkObjSaveStatus(driver->stateDir, + obj, network_driver->xmlopt) < 0) { tmp_floor_sum += ifaceBand->in->floor; virNetworkObjSetFloorSum(obj, tmp_floor_sum); ignore_value(virBitmapSetBit(classIdMap, *class_id)); @@ -5337,7 +5348,8 @@ networkUpdatePortBandwidth(virNetworkObjPtr obj, if (virNetDevBandwidthUpdateRate(def->bridge, 2, def->bandwidth, new_rate) < 0 || - virNetworkObjSaveStatus(driver->stateDir, obj) < 0) { + virNetworkObjSaveStatus(driver->stateDir, + obj, network_driver->xmlopt) < 0) { /* Ouch, rollback */ tmp_floor_sum -= newBandwidth->in->floor; tmp_floor_sum += oldBandwidth->in->floor; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aa09ef175a..746d701640 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4713,7 +4713,7 @@ qemuProcessGetNetworkAddress(const char *netname, if (!xml) goto cleanup; - netdef = virNetworkDefParseString(xml); + netdef = virNetworkDefParseString(xml, NULL); if (!netdef) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index c10344f6cd..dff384392e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -971,7 +971,7 @@ testParseNetworks(testDriverPtr privconn, if (!node) return -1; - def = virNetworkDefParseNode(ctxt->doc, node); + def = virNetworkDefParseNode(ctxt->doc, node, NULL); if (!def) return -1; @@ -4148,7 +4148,7 @@ testNetworkCreateXML(virConnectPtr conn, const char *xml) virNetworkPtr net = NULL; virObjectEventPtr event = NULL; - if ((newDef = virNetworkDefParseString(xml)) == NULL) + if ((newDef = virNetworkDefParseString(xml, NULL)) == NULL) goto cleanup; if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef, @@ -4184,7 +4184,7 @@ testNetworkDefineXML(virConnectPtr conn, virNetworkPtr net = NULL; virObjectEventPtr event = NULL; - if ((newDef = virNetworkDefParseString(xml)) == NULL) + if ((newDef = virNetworkDefParseString(xml, NULL)) == NULL) goto cleanup; if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef, 0))) @@ -4270,7 +4270,8 @@ testNetworkUpdate(virNetworkPtr net, } /* update the network config in memory/on disk */ - if (virNetworkObjUpdate(obj, command, section, parentIndex, xml, flags) < 0) + if (virNetworkObjUpdate(obj, command, section, + parentIndex, xml, NULL, flags) < 0) goto cleanup; ret = 0; @@ -4354,7 +4355,7 @@ testNetworkGetXMLDesc(virNetworkPtr net, if (!(obj = testNetworkObjFindByName(privconn, net->name))) goto cleanup; - ret = virNetworkDefFormat(virNetworkObjGetDef(obj), flags); + ret = virNetworkDefFormat(virNetworkObjGetDef(obj), NULL, flags); cleanup: virNetworkObjEndAPI(&obj); diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c index 7326ae7d07..814f27155f 100644 --- a/src/vbox/vbox_network.c +++ b/src/vbox/vbox_network.c @@ -375,7 +375,7 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start) PRUnichar *networkNameUtf16 = NULL; char *networkNameUtf8 = NULL; IHostNetworkInterface *networkInterface = NULL; - virNetworkDefPtr def = virNetworkDefParseString(xml); + virNetworkDefPtr def = virNetworkDefParseString(xml, NULL); virNetworkIPDefPtr ipdef = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; vboxIID vboxnetiid; @@ -911,7 +911,7 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, unsigned int flags) } DEBUGIID("Network UUID", &vboxnet0IID); - ret = virNetworkDefFormat(def, 0); + ret = virNetworkDefFormat(def, NULL, 0); cleanup: vboxIIDUnalloc(&vboxnet0IID); diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index bf9675838d..c445551099 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -26,7 +26,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr char *pidfile = NULL; dnsmasqContext *dctx = NULL; - if (!(def = virNetworkDefParseFile(inxml))) + if (!(def = virNetworkDefParseFile(inxml, NULL))) goto fail; if (!(obj = virNetworkObjNew())) diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index c25282ebb1..78844085a0 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -70,7 +70,7 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandSetDryRun(&buf, testCommandDryRun, NULL); - if (!(def = virNetworkDefParseFile(xml))) + if (!(def = virNetworkDefParseFile(xml, NULL))) goto cleanup; if (networkAddFirewallRules(def) < 0) diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index b19a365ff4..cd76ce5375 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -30,14 +30,14 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, testCompareNetXML2XMLResult result = TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS; virNetworkDefPtr dev = NULL; - if (!(dev = virNetworkDefParseFile(inxml))) { + if (!(dev = virNetworkDefParseFile(inxml, NULL))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE; goto cleanup; } if (expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE) goto cleanup; - if (!(actual = virNetworkDefFormat(dev, flags))) { + if (!(actual = virNetworkDefFormat(dev, NULL, flags))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT; goto cleanup; } diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index 46cb4c2f0b..69c19e800d 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -27,14 +27,14 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, if (virTestLoadFile(updatexml, &updateXmlData) < 0) goto error; - if (!(def = virNetworkDefParseFile(netxml))) + if (!(def = virNetworkDefParseFile(netxml, NULL))) goto fail; if (virNetworkDefUpdateSection(def, command, section, parentIndex, updateXmlData, 0) < 0) goto fail; - if (!(actual = virNetworkDefFormat(def, flags))) + if (!(actual = virNetworkDefFormat(def, NULL, flags))) goto fail; if (!expectFailure) { -- 2.21.0
Just the plumbing, no real implementation yet Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/network_conf.c | 22 ++++++++++++++++++++-- src/conf/network_conf.h | 21 ++++++++++++++++++++- src/network/bridge_driver.c | 2 +- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 067adf7936..c5a243684a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -93,7 +93,7 @@ virNetworkXMLOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetworkXML); virNetworkXMLOptionPtr -virNetworkXMLOptionNew(void) +virNetworkXMLOptionNew(virNetworkXMLNamespacePtr xmlns) { virNetworkXMLOptionPtr xmlopt; @@ -103,6 +103,9 @@ virNetworkXMLOptionNew(void) if (!(xmlopt = virObjectNew(virNetworkXMLOptionClass))) return NULL; + if (xmlns) + xmlopt->ns = *xmlns; + return xmlopt; } @@ -268,6 +271,8 @@ virNetworkDefFree(virNetworkDefPtr def) xmlFreeNode(def->metadata); + if (def->namespaceData && def->ns.free) + (def->ns.free)(def->namespaceData); VIR_FREE(def); } @@ -1622,7 +1627,7 @@ virNetworkForwardDefParseXML(const char *networkName, virNetworkDefPtr virNetworkDefParseXML(xmlXPathContextPtr ctxt, - virNetworkXMLOptionPtr xmlopt ATTRIBUTE_UNUSED) + virNetworkXMLOptionPtr xmlopt) { virNetworkDefPtr def; char *tmp = NULL; @@ -2043,6 +2048,12 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, virXMLNodeSanitizeNamespaces(def->metadata); } + if (xmlopt) + def->ns = xmlopt->ns; + if (def->ns.parse && + (def->ns.parse)(ctxt, &def->namespaceData) < 0) + goto error; + ctxt->node = save; return def; @@ -2422,6 +2433,8 @@ virNetworkDefFormatBuf(virBufferPtr buf, bool hasbridge = false; virBufferAddLit(buf, "<network"); + if (def->namespaceData && def->ns.href) + virBufferAsprintf(buf, " %s", (def->ns.href)()); if (!(flags & VIR_NETWORK_XML_INACTIVE) && (def->connections > 0)) virBufferAsprintf(buf, " connections='%d'", def->connections); if (def->ipv6nogw) @@ -2627,6 +2640,11 @@ virNetworkDefFormatBuf(virBufferPtr buf, if (virPortGroupDefFormat(buf, &def->portGroups[i]) < 0) goto error; + if (def->namespaceData && def->ns.format) { + if ((def->ns.format)(buf, def->namespaceData) < 0) + return -1; + } + virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</network>\n"); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index b7ce569d4a..b167b57e85 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -41,9 +41,24 @@ #include "virmacmap.h" #include "virenum.h" +typedef int (*virNetworkDefNamespaceParse)(xmlXPathContextPtr, void **); +typedef void (*virNetworkDefNamespaceFree)(void *); +typedef int (*virNetworkDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef const char *(*virNetworkDefNamespaceHref)(void); + +typedef struct _virNetworkXMLNamespace virNetworkXMLNamespace; +typedef virNetworkXMLNamespace *virNetworkXMLNamespacePtr; +struct _virNetworkXMLNamespace { + virNetworkDefNamespaceParse parse; + virNetworkDefNamespaceFree free; + virNetworkDefNamespaceXMLFormat format; + virNetworkDefNamespaceHref href; +}; struct _virNetworkXMLOption { virObject parent; + + virNetworkXMLNamespace ns; }; typedef struct _virNetworkXMLOption virNetworkXMLOption; typedef virNetworkXMLOption *virNetworkXMLOptionPtr; @@ -277,6 +292,10 @@ struct _virNetworkDef { /* Application-specific custom metadata */ xmlNodePtr metadata; + + /* Network specific XML namespace data */ + void *namespaceData; + virNetworkXMLNamespace ns; }; typedef enum { @@ -298,7 +317,7 @@ enum { }; virNetworkXMLOptionPtr -virNetworkXMLOptionNew(void); +virNetworkXMLOptionNew(virNetworkXMLNamespacePtr xmlns); virNetworkDefPtr virNetworkDefCopy(virNetworkDefPtr def, diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3353754eb5..1a4d6e7f7b 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -139,7 +139,7 @@ networkDnsmasqCapsRefresh(virNetworkDriverStatePtr driver) static virNetworkXMLOptionPtr networkDnsmasqCreateXMLConf(void) { - return virNetworkXMLOptionNew(); + return virNetworkXMLOptionNew(NULL); } -- 2.21.0
On Sun, Jul 14, 2019 at 08:03:59PM -0400, Cole Robinson wrote:
Just the plumbing, no real implementation yet
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- src/conf/network_conf.c | 22 ++++++++++++++++++++-- src/conf/network_conf.h | 21 ++++++++++++++++++++- src/network/bridge_driver.c | 2 +- 3 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index b7ce569d4a..b167b57e85 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -41,9 +41,24 @@ #include "virmacmap.h" #include "virenum.h"
+typedef int (*virNetworkDefNamespaceParse)(xmlXPathContextPtr, void **); +typedef void (*virNetworkDefNamespaceFree)(void *); +typedef int (*virNetworkDefNamespaceXMLFormat)(virBufferPtr, void *); +typedef const char *(*virNetworkDefNamespaceHref)(void); + +typedef struct _virNetworkXMLNamespace virNetworkXMLNamespace; +typedef virNetworkXMLNamespace *virNetworkXMLNamespacePtr; +struct _virNetworkXMLNamespace { + virNetworkDefNamespaceParse parse; + virNetworkDefNamespaceFree free; + virNetworkDefNamespaceXMLFormat format; + virNetworkDefNamespaceHref href; +};
These declarations are identical to virDomainXMLNamespace Jano
This maps to XML like: <network xmlns:dnsmasq='http://libvirt.org/schemas/network/dnsmasq/1.0'> ... <dnsmasq:options> <dnsmasq:option value="foo=bar"/> <dnsmasq:option value="cname=*.foo.example.com,master.example.com"/> </dnsmasq:options> </network> To dnsmasq config options ... foo=bar cname=*.foo.example.com,master.example.com Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/schemas/network.rng | 11 ++ src/network/bridge_driver.c | 130 +++++++++++++++++- src/network/bridge_driver.h | 12 ++ tests/Makefile.am | 14 +- .../networkxml2confdata/dnsmasq-options.conf | 18 +++ tests/networkxml2confdata/dnsmasq-options.xml | 15 ++ tests/networkxml2conftest.c | 8 +- tests/networkxml2xmlin/dnsmasq-options.xml | 15 ++ tests/networkxml2xmlout/dnsmasq-options.xml | 17 +++ tests/networkxml2xmltest.c | 11 +- 10 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 tests/networkxml2confdata/dnsmasq-options.conf create mode 100644 tests/networkxml2confdata/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlin/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlout/dnsmasq-options.xml diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 2a6e3358fd..56937d6a4e 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -405,6 +405,17 @@ <zeroOrMore> <ref name="route"/> </zeroOrMore> + + <!-- <dnsmasq:options> --> + <optional> + <element name="options" ns="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <zeroOrMore> + <element name="option"> + <attribute name='value'/> + </element> + </zeroOrMore> + </element> + </optional> </interleave> </element> </define> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 1a4d6e7f7b..41fa89a4af 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -69,6 +69,8 @@ #include "virjson.h" #include "virnetworkportdef.h" +#include <libxml/xpathInternals.h> + #define VIR_FROM_THIS VIR_FROM_NETWORK #define MAX_BRIDGE_ID 256 @@ -83,6 +85,8 @@ VIR_LOG_INIT("network.bridge_driver"); +#define DNSMASQ_NAMESPACE_HREF "http://libvirt.org/schemas/network/dnsmasq/1.0" + static virNetworkDriverStatePtr network_driver; @@ -136,10 +140,126 @@ networkDnsmasqCapsRefresh(virNetworkDriverStatePtr driver) return 0; } -static virNetworkXMLOptionPtr + +static void +networkDnsmasqDefNamespaceFree(void *nsdata) +{ + networkDnsmasqXmlNsDefPtr def = nsdata; + if (!def) + return; + + virStringListFreeCount(def->options, def->noptions); + + VIR_FREE(def); +} + + +static int +networkDnsmasqDefNamespaceParseOptions(networkDnsmasqXmlNsDefPtr nsdef, + xmlXPathContextPtr ctxt) +{ + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; + ssize_t nnodes; + size_t i; + + if ((nnodes = virXPathNodeSet("./dnsmasq:options/dnsmasq:option", + ctxt, &nodes)) < 0) + return -1; + + if (nnodes == 0) + return 0; + + if (VIR_ALLOC_N(nsdef->options, nnodes) < 0) + return -1; + + for (i = 0; i < nnodes; i++) { + if (!(nsdef->options[nsdef->noptions++] = virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No dnsmasq options value specified")); + return -1; + } + } + + return 0; +} + + +static int +networkDnsmasqDefNamespaceParse(xmlXPathContextPtr ctxt, + void **data) +{ + networkDnsmasqXmlNsDefPtr nsdata = NULL; + int ret = -1; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "dnsmasq", + BAD_CAST DNSMASQ_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + DNSMASQ_NAMESPACE_HREF); + return -1; + } + + if (VIR_ALLOC(nsdata) < 0) + return -1; + + if (networkDnsmasqDefNamespaceParseOptions(nsdata, ctxt)) + goto cleanup; + + if (nsdata->noptions > 0) + VIR_STEAL_PTR(*data, nsdata); + + ret = 0; + + cleanup: + networkDnsmasqDefNamespaceFree(nsdata); + return ret; +} + + +static int +networkDnsmasqDefNamespaceFormatXML(virBufferPtr buf, + void *nsdata) +{ + networkDnsmasqXmlNsDefPtr def = nsdata; + size_t i; + + if (!def->noptions) + return 0; + + virBufferAddLit(buf, "<dnsmasq:options>\n"); + virBufferAdjustIndent(buf, 2); + + for (i = 0; i < def->noptions; i++) { + virBufferEscapeString(buf, "<dnsmasq:option value='%s'/>\n", + def->options[i]); + } + + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</dnsmasq:options>\n"); + + return 0; +} + + +static const char * +networkDnsmasqDefNamespaceHref(void) +{ + return "xmlns:dnsmasq='" DNSMASQ_NAMESPACE_HREF "'"; +} + + +virNetworkXMLNamespace networkDnsmasqXMLNamespace = { + .parse = networkDnsmasqDefNamespaceParse, + .free = networkDnsmasqDefNamespaceFree, + .format = networkDnsmasqDefNamespaceFormatXML, + .href = networkDnsmasqDefNamespaceHref, +}; + + +virNetworkXMLOptionPtr networkDnsmasqCreateXMLConf(void) { - return virNetworkXMLOptionNew(NULL); + return virNetworkXMLOptionNew(&networkDnsmasqXMLNamespace); } @@ -1480,6 +1600,12 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, } } + if (def->namespaceData) { + networkDnsmasqXmlNsDefPtr dnsmasqxmlns = def->namespaceData; + for (i = 0; i < dnsmasqxmlns->noptions; i++) + virBufferAsprintf(&configbuf, "%s\n", dnsmasqxmlns->options[i]); + } + if (!(*configstr = virBufferContentAndReset(&configbuf))) goto cleanup; diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 7357c1754c..b095388a0b 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -27,6 +27,18 @@ #include "virdnsmasq.h" #include "virnetworkobj.h" +extern virNetworkXMLNamespace networkDnsmasqXMLNamespace; + +typedef struct _networkDnsmasqXmlNsDef networkDnsmasqXmlNsDef; +typedef networkDnsmasqXmlNsDef *networkDnsmasqXmlNsDefPtr; +struct _networkDnsmasqXmlNsDef { + size_t noptions; + char **options; +}; + +virNetworkXMLOptionPtr +networkDnsmasqCreateXMLConf(void); + int networkRegister(void); diff --git a/tests/Makefile.am b/tests/Makefile.am index 107f2de859..65192bac8e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -331,13 +331,13 @@ test_programs += virjsontest endif WITH_YAJL test_programs += \ - networkxml2xmltest \ networkxml2xmlupdatetest \ virnetworkportxml2xmltest \ $(NULL) if WITH_NETWORK test_programs += \ + networkxml2xmltest \ networkxml2conftest \ networkxml2firewalltest \ $(NULL) @@ -806,11 +806,6 @@ EXTRA_DIST += \ bhyveargv2xmlmock.c endif ! WITH_BHYVE -networkxml2xmltest_SOURCES = \ - networkxml2xmltest.c \ - testutils.c testutils.h -networkxml2xmltest_LDADD = $(LDADDS) - networkxml2xmlupdatetest_SOURCES = \ networkxml2xmlupdatetest.c \ testutils.c testutils.h @@ -822,6 +817,11 @@ virnetworkportxml2xmltest_SOURCES = \ virnetworkportxml2xmltest_LDADD = $(LDADDS) if WITH_NETWORK +networkxml2xmltest_SOURCES = \ + networkxml2xmltest.c \ + testutils.c testutils.h +networkxml2xmltest_LDADD = ../src/libvirt_driver_network_impl.la $(LDADDS) + networkxml2conftest_SOURCES = \ networkxml2conftest.c \ testutils.c testutils.h @@ -833,7 +833,7 @@ networkxml2firewalltest_SOURCES = \ networkxml2firewalltest_LDADD = ../src/libvirt_driver_network_impl.la $(LDADDS) else ! WITH_NETWORK -EXTRA_DIST += networkxml2conftest.c +EXTRA_DIST += networkxml2xmltest.c networkxml2conftest.c endif ! WITH_NETWORK if WITH_STORAGE_SHEEPDOG diff --git a/tests/networkxml2confdata/dnsmasq-options.conf b/tests/networkxml2confdata/dnsmasq-options.conf new file mode 100644 index 0000000000..867f355c79 --- /dev/null +++ b/tests/networkxml2confdata/dnsmasq-options.conf @@ -0,0 +1,18 @@ +##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE +##OVERWRITTEN AND LOST. Changes to this configuration should be made using: +## virsh net-edit default +## or other application using the libvirt API. +## +## dnsmasq conf file created by libvirt +strict-order +except-interface=lo +bind-dynamic +interface=virbr0 +dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0 +dhcp-no-override +dhcp-authoritative +dhcp-lease-max=253 +dhcp-hostsfile=/var/lib/libvirt/dnsmasq/default.hostsfile +addn-hosts=/var/lib/libvirt/dnsmasq/default.addnhosts +foo=bar +cname=*.cloudapps.example.com,master.example.com diff --git a/tests/networkxml2confdata/dnsmasq-options.xml b/tests/networkxml2confdata/dnsmasq-options.xml new file mode 100644 index 0000000000..35a87b8e3e --- /dev/null +++ b/tests/networkxml2confdata/dnsmasq-options.xml @@ -0,0 +1,15 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + </dhcp> + </ip> + <dnsmasq:options> + <dnsmasq:option value="foo=bar"/> + <dnsmasq:option value="cname=*.cloudapps.example.com,master.example.com"/> + </dnsmasq:options> +</network> diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index c445551099..dcb99aad6e 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -25,8 +25,12 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr virCommandPtr cmd = NULL; char *pidfile = NULL; dnsmasqContext *dctx = NULL; + virNetworkXMLOptionPtr xmlopt = NULL; - if (!(def = virNetworkDefParseFile(inxml, NULL))) + if (!(xmlopt = networkDnsmasqCreateXMLConf())) + goto fail; + + if (!(def = virNetworkDefParseFile(inxml, xmlopt))) goto fail; if (!(obj = virNetworkObjNew())) @@ -63,6 +67,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, dnsmasqCapsPtr VIR_FREE(actual); VIR_FREE(pidfile); virCommandFree(cmd); + virObjectUnref(xmlopt); virNetworkObjEndAPI(&obj); dnsmasqContextFree(dctx); return ret; @@ -141,6 +146,7 @@ mymain(void) DO_TEST("dhcp6-nat-network", dhcpv6); DO_TEST("dhcp6host-routed-network", dhcpv6); DO_TEST("ptr-domains-auto", dhcpv6); + DO_TEST("dnsmasq-options", dhcpv6); virObjectUnref(dhcpv6); virObjectUnref(full); diff --git a/tests/networkxml2xmlin/dnsmasq-options.xml b/tests/networkxml2xmlin/dnsmasq-options.xml new file mode 100644 index 0000000000..35a87b8e3e --- /dev/null +++ b/tests/networkxml2xmlin/dnsmasq-options.xml @@ -0,0 +1,15 @@ +<network xmlns:dnsmasq="http://libvirt.org/schemas/network/dnsmasq/1.0"> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <bridge name="virbr0"/> + <forward mode="nat" dev="eth1"/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254"/> + </dhcp> + </ip> + <dnsmasq:options> + <dnsmasq:option value="foo=bar"/> + <dnsmasq:option value="cname=*.cloudapps.example.com,master.example.com"/> + </dnsmasq:options> +</network> diff --git a/tests/networkxml2xmlout/dnsmasq-options.xml b/tests/networkxml2xmlout/dnsmasq-options.xml new file mode 100644 index 0000000000..856a018f25 --- /dev/null +++ b/tests/networkxml2xmlout/dnsmasq-options.xml @@ -0,0 +1,17 @@ +<network xmlns:dnsmasq='http://libvirt.org/schemas/network/dnsmasq/1.0'> + <name>default</name> + <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> + <forward dev='eth1' mode='nat'> + <interface dev='eth1'/> + </forward> + <bridge name='virbr0' stp='on' delay='0'/> + <ip address='192.168.122.1' netmask='255.255.255.0'> + <dhcp> + <range start='192.168.122.2' end='192.168.122.254'/> + </dhcp> + </ip> + <dnsmasq:options> + <dnsmasq:option value='foo=bar'/> + <dnsmasq:option value='cname=*.cloudapps.example.com,master.example.com'/> + </dnsmasq:options> +</network> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index cd76ce5375..3d90023445 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -10,6 +10,7 @@ #include "network_conf.h" #include "testutilsqemu.h" #include "virstring.h" +#include "network/bridge_driver.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -29,15 +30,19 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, int ret; testCompareNetXML2XMLResult result = TEST_COMPARE_NET_XML2XML_RESULT_SUCCESS; virNetworkDefPtr dev = NULL; + virNetworkXMLOptionPtr xmlopt = NULL; - if (!(dev = virNetworkDefParseFile(inxml, NULL))) { + if (!(xmlopt = networkDnsmasqCreateXMLConf())) + goto cleanup; + + if (!(dev = virNetworkDefParseFile(inxml, xmlopt))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE; goto cleanup; } if (expectResult == TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE) goto cleanup; - if (!(actual = virNetworkDefFormat(dev, NULL, flags))) { + if (!(actual = virNetworkDefFormat(dev, xmlopt, flags))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_FORMAT; goto cleanup; } @@ -67,6 +72,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, VIR_FREE(actual); virNetworkDefFree(dev); + virObjectUnref(xmlopt); return ret; } @@ -158,6 +164,7 @@ mymain(void) DO_TEST_PARSE_ERROR("passthrough-duplicate"); DO_TEST("metadata"); DO_TEST("set-mtu"); + DO_TEST("dnsmasq-options"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.21.0
On Sun, Jul 14, 2019 at 08:04:00PM -0400, Cole Robinson wrote:
This maps to XML like:
<network xmlns:dnsmasq='http://libvirt.org/schemas/network/dnsmasq/1.0'> ... <dnsmasq:options> <dnsmasq:option value="foo=bar"/> <dnsmasq:option value="cname=*.foo.example.com,master.example.com"/> </dnsmasq:options> </network>
To dnsmasq config options
... foo=bar cname=*.foo.example.com,master.example.com
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/schemas/network.rng | 11 ++ src/network/bridge_driver.c | 130 +++++++++++++++++- src/network/bridge_driver.h | 12 ++ tests/Makefile.am | 14 +- .../networkxml2confdata/dnsmasq-options.conf | 18 +++ tests/networkxml2confdata/dnsmasq-options.xml | 15 ++ tests/networkxml2conftest.c | 8 +- tests/networkxml2xmlin/dnsmasq-options.xml | 15 ++ tests/networkxml2xmlout/dnsmasq-options.xml | 17 +++ tests/networkxml2xmltest.c | 11 +- 10 files changed, 239 insertions(+), 12 deletions(-) create mode 100644 tests/networkxml2confdata/dnsmasq-options.conf create mode 100644 tests/networkxml2confdata/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlin/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlout/dnsmasq-options.xml
+static int +networkDnsmasqDefNamespaceParse(xmlXPathContextPtr ctxt, + void **data) +{ + networkDnsmasqXmlNsDefPtr nsdata = NULL; + int ret = -1; + + if (xmlXPathRegisterNs(ctxt, BAD_CAST "dnsmasq", + BAD_CAST DNSMASQ_NAMESPACE_HREF) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to register xml namespace '%s'"), + DNSMASQ_NAMESPACE_HREF); + return -1; + }
This breaks the build on Debian in Jenkins and Ubuntu 18 on travis: https://ci.centos.org/view/libvirt/job/libvirt-build/systems=libvirt-debian-... https://travis-ci.org/libvirt/libvirt/jobs/560399203 Jano
+ + if (VIR_ALLOC(nsdata) < 0) + return -1; + + if (networkDnsmasqDefNamespaceParseOptions(nsdata, ctxt)) + goto cleanup; + + if (nsdata->noptions > 0) + VIR_STEAL_PTR(*data, nsdata); + + ret = 0; + + cleanup: + networkDnsmasqDefNamespaceFree(nsdata); + return ret; +}
Signed-off-by: Cole Robinson <crobinso@redhat.com> --- docs/formatnetwork.html.in | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index 509cca9e8b..2448fb09e7 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -1096,6 +1096,28 @@ </dd> </dl> + <h3><a id="elementsNamespaces">Network namespaces</a></h3> + + <p> + A special XML namespace is available for passing options directly to the + underlying dnsmasq configuration file. Usage of XML namespaces comes with no + support guarantees, so use at your own risk. + </p> + + <p> + This example XML will pass the option strings <code>foo=bar</code> and + <code>cname=*.foo.example.com,master.example.com</code> directly to the + underlying dnsmasq instance. + <pre> +<network xmlns:dnsmasq='http://libvirt.org/schemas/network/dnsmasq/1.0'>; + ... + <dnsmasq:options> + <dnsmasq:option value="foo=bar"/> + <dnsmasq:option value="cname=*.foo.example.com,master.example.com"/> + </dnsmasq:options> +</network></pre> + </p> + <h2><a id="examples">Example configuration</a></h2> <h3><a id="examplesNAT">NAT based network</a></h3> -- 2.21.0
On 7/14/19 8:03 PM, Cole Robinson wrote:
There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options.
This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented.
This all looks like a reasonable facsimile of the qemu:commandline stuff, and it worked in my tests (including not creating any problems with all the existing networks I have on my test system). My one reservation would be that it will tend to discourage adding *supported* methods of configuring things that people will instead use this backdoor to implement. But on the other hand, it makes it possible to do new things without needing to wait for an officially supported method (which could take a very long time, or simply never happen, as we've seen). (Actually I for some reason thought we already *had* support for the specific example you used - CNAME records. I guess I had lumped it in with SRV and TXT records in my memory. It really should be straightforward to do, and should still be done, but that shouldn't stop this patch series from going in). This is for the entire series: Reviewed-by: Laine Stump <laine@laine.org>
Cole Robinson (5): conf: Add virNetworkXMLOption conf: Add network xmlopt argument conf: Add virNetworkXMLNamespace network: wire up dnsmasq option xmlns docs: formatnetwork: Document xmlns:dnsmasq
docs/formatnetwork.html.in | 22 +++ docs/schemas/network.rng | 11 ++ src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 91 +++++++-- src/conf/network_conf.h | 51 ++++- src/conf/virnetworkobj.c | 50 +++-- src/conf/virnetworkobj.h | 13 +- src/esx/esx_network_driver.c | 4 +- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 182 ++++++++++++++++-- src/network/bridge_driver.h | 12 ++ src/network/bridge_driver_platform.h | 2 + src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 11 +- src/vbox/vbox_network.c | 4 +- tests/Makefile.am | 14 +- .../networkxml2confdata/dnsmasq-options.conf | 18 ++ tests/networkxml2confdata/dnsmasq-options.xml | 15 ++ tests/networkxml2conftest.c | 8 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmlin/dnsmasq-options.xml | 15 ++ tests/networkxml2xmlout/dnsmasq-options.xml | 17 ++ tests/networkxml2xmltest.c | 11 +- tests/networkxml2xmlupdatetest.c | 4 +- 24 files changed, 478 insertions(+), 84 deletions(-) create mode 100644 tests/networkxml2confdata/dnsmasq-options.conf create mode 100644 tests/networkxml2confdata/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlin/dnsmasq-options.xml create mode 100644 tests/networkxml2xmlout/dnsmasq-options.xml
On 7/17/19 12:49 PM, Laine Stump wrote:
On 7/14/19 8:03 PM, Cole Robinson wrote:
There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options.
This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented.
This all looks like a reasonable facsimile of the qemu:commandline stuff, and it worked in my tests (including not creating any problems with all the existing networks I have on my test system).
My one reservation would be that it will tend to discourage adding *supported* methods of configuring things that people will instead use this backdoor to implement. But on the other hand, it makes it possible to do new things without needing to wait for an officially supported method (which could take a very long time, or simply never happen, as we've seen).
(Actually I for some reason thought we already *had* support for the specific example you used - CNAME records. I guess I had lumped it in with SRV and TXT records in my memory. It really should be straightforward to do, and should still be done, but that shouldn't stop this patch series from going in).
This is for the entire series:
Reviewed-by: Laine Stump <laine@laine.org>
Thanks, I've pushed this now. Regarding the bigger point, I think it's worth considering whether we should aim to expose every requested dnsmasq option as official XML or not. We effectively have one network driver; there's an impl for vbox and esx but they seem very basic. It doesn't look like we are going to have another backend impl any time soon. Unless the requested option has some specific reason to be represented in libvirt XML, like if another part of libvirt may need/want to programmatically act on that data for some reason, maybe it's okay to say that dnsmasq passthrough is the solution. Some examples might be * dhcp-option: https://bugzilla.redhat.com/show_bug.cgi?id=666556 * auth-zone: https://bugzilla.redhat.com/show_bug.cgi?id=1690943 * This bug has a lot of mentioned options: https://bugzilla.redhat.com/show_bug.cgi?id=824573 - Cole
On Wed, Jul 17, 2019 at 05:38:51PM -0400, Cole Robinson wrote:
On 7/17/19 12:49 PM, Laine Stump wrote:
On 7/14/19 8:03 PM, Cole Robinson wrote:
There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options.
This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented.
This all looks like a reasonable facsimile of the qemu:commandline stuff, and it worked in my tests (including not creating any problems with all the existing networks I have on my test system).
My one reservation would be that it will tend to discourage adding *supported* methods of configuring things that people will instead use this backdoor to implement. But on the other hand, it makes it possible to do new things without needing to wait for an officially supported method (which could take a very long time, or simply never happen, as we've seen).
(Actually I for some reason thought we already *had* support for the specific example you used - CNAME records. I guess I had lumped it in with SRV and TXT records in my memory. It really should be straightforward to do, and should still be done, but that shouldn't stop this patch series from going in).
This is for the entire series:
Reviewed-by: Laine Stump <laine@laine.org>
Thanks, I've pushed this now.
Regarding the bigger point, I think it's worth considering whether we should aim to expose every requested dnsmasq option as official XML or not. We effectively have one network driver; there's an impl for vbox and esx but they seem very basic. It doesn't look like we are going to have another backend impl any time soon.
I wouldn't rule it out. I can't remember where it was, but a few months ago I had a discussion with some folks precisely about replacing dnsmasq with new daemon(s). Primarily the purpose would getting a more secure solution modern solution written in a memory safe language.
Unless the requested option has some specific reason to be represented in libvirt XML, like if another part of libvirt may need/want to programmatically act on that data for some reason, maybe it's okay to say that dnsmasq passthrough is the solution. Some examples might be
* dhcp-option: https://bugzilla.redhat.com/show_bug.cgi?id=666556 * auth-zone: https://bugzilla.redhat.com/show_bug.cgi?id=1690943 * This bug has a lot of mentioned options: https://bugzilla.redhat.com/show_bug.cgi?id=824573
Most of the stuff across these bugs is not really dnsmasq specific. It would be applicable to any DNS/DHCP service, so its just a matter of expressing it sensibly in the XML, so you're not tied to dnsmasq specific syntax. 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 7/18/19 4:59 AM, Daniel P. Berrangé wrote:
On Wed, Jul 17, 2019 at 05:38:51PM -0400, Cole Robinson wrote:
On 7/17/19 12:49 PM, Laine Stump wrote:
On 7/14/19 8:03 PM, Cole Robinson wrote:
There's several unresolved RFEs for the <network> bridge driver that are essentially requests to add XML wrappers for underlying dnsmasq options.
This series adds a dnsmasq xmlns namespace for <network> XML that allows passing option strings directly to the generated dnsmasq config file. It will allow motivated users to work around libvirt until those types of RFEs are properly implemented.
This all looks like a reasonable facsimile of the qemu:commandline stuff, and it worked in my tests (including not creating any problems with all the existing networks I have on my test system).
My one reservation would be that it will tend to discourage adding *supported* methods of configuring things that people will instead use this backdoor to implement. But on the other hand, it makes it possible to do new things without needing to wait for an officially supported method (which could take a very long time, or simply never happen, as we've seen).
(Actually I for some reason thought we already *had* support for the specific example you used - CNAME records. I guess I had lumped it in with SRV and TXT records in my memory. It really should be straightforward to do, and should still be done, but that shouldn't stop this patch series from going in).
This is for the entire series:
Reviewed-by: Laine Stump <laine@laine.org> Thanks, I've pushed this now.
Regarding the bigger point, I think it's worth considering whether we should aim to expose every requested dnsmasq option as official XML or not.
Definitely not; there's a lot of things that maybe only one person would ever do, or it's just too difficult and pointless to try and abstract the config in a way that retains full functionality while being implementation agnostic (that's what stopped the dhcp options stuff in its tracks - the original patches had a problem with escaped characters or something, but once you started down the rabbit hole there was just no end). But if something is commonly used then it's better to have a straightforward way to do it so that everybody does it the same way, and we don't end up with all sorts of obscure questions because someone follows some loosely connected bad advice they found on a random page on the internet and their setup becomes completely broken.
We effectively have one network driver; there's an impl for vbox and esx but they seem very basic. It doesn't look like we are going to have another backend impl any time soon. I wouldn't rule it out. I can't remember where it was, but a few months ago I had a discussion with some folks precisely about replacing dnsmasq with new daemon(s). Primarily the purpose would getting a more secure solution modern solution written in a memory safe language.
Unless the requested option has some specific reason to be represented in libvirt XML, like if another part of libvirt may need/want to programmatically act on that data for some reason, maybe it's okay to say that dnsmasq passthrough is the solution. Some examples might be
* dhcp-option: https://bugzilla.redhat.com/show_bug.cgi?id=666556 * auth-zone: https://bugzilla.redhat.com/show_bug.cgi?id=1690943 * This bug has a lot of mentioned options: https://bugzilla.redhat.com/show_bug.cgi?id=824573 Most of the stuff across these bugs is not really dnsmasq specific. It would be applicable to any DNS/DHCP service, so its just a matter of expressing it sensibly in the XML, so you're not tied to dnsmasq specific syntax.
Yeah, in the past we may have been trying too hard to make the XML backend-agnostic. And even for domain XML, it's never been the case that the exact same XML would work unmodified for both Xen and qemu, for example. It definitely is good to work *towards* that though; makes it less of a task in the case that a change in backend does happen (for example, what Dan mentioned above).
participants (4)
-
Cole Robinson -
Daniel P. Berrangé -
Ján Tomko -
Laine Stump