[libvirt] [PATCH 0/7] refactoring network xml parsing + net-update additions
Patches 1,2, and 4-7 refactor the network XML parsing functions to have a more uniform calling sequence for different subelements of <network>. This makes it simpler to write new virNetworkUpdate*() backend functions for those bits (because they're then more similar to existing functions.) Patch 3 actually adds some new backends for updating subelements that weren't previously implemented - <dns> host/srv/txt records. The intent is to followup on patched 4-7 with backends for updating <ip>, <forward>, and <bridge>; while I haven't gotten to those yet, I'd rather get these changes in sooner than later, both for more testing, and to make future backports simpler.
This shortens the name of the structs for srv and txt, and their instances in virNetworkDNSDef, to be more compact and uniform with the naming of the dns host array. --- src/conf/network_conf.c | 102 ++++++++++++++++++++++---------------------- src/conf/network_conf.h | 32 +++++++------- src/network/bridge_driver.c | 34 +++++++-------- 3 files changed, 85 insertions(+), 83 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index c36bdd3..7caf39f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -136,13 +136,13 @@ static void virNetworkIpDefClear(virNetworkIpDefPtr def) static void virNetworkDNSDefFree(virNetworkDNSDefPtr def) { if (def) { - if (def->txtrecords) { - while (def->ntxtrecords--) { - VIR_FREE(def->txtrecords[def->ntxtrecords].name); - VIR_FREE(def->txtrecords[def->ntxtrecords].value); + if (def->txts) { + while (def->ntxts--) { + VIR_FREE(def->txts[def->ntxts].name); + VIR_FREE(def->txts[def->ntxts].value); } } - VIR_FREE(def->txtrecords); + VIR_FREE(def->txts); if (def->nhosts) { while (def->nhosts--) { while (def->hosts[def->nhosts].nnames--) @@ -151,15 +151,15 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def) } } VIR_FREE(def->hosts); - if (def->nsrvrecords) { - while (def->nsrvrecords--) { - VIR_FREE(def->srvrecords[def->nsrvrecords].domain); - VIR_FREE(def->srvrecords[def->nsrvrecords].service); - VIR_FREE(def->srvrecords[def->nsrvrecords].protocol); - VIR_FREE(def->srvrecords[def->nsrvrecords].target); + if (def->nsrvs) { + while (def->nsrvs--) { + VIR_FREE(def->srvs[def->nsrvs].domain); + VIR_FREE(def->srvs[def->nsrvs].service); + VIR_FREE(def->srvs[def->nsrvs].protocol); + VIR_FREE(def->srvs[def->nsrvs].target); } } - VIR_FREE(def->srvrecords); + VIR_FREE(def->srvs); VIR_FREE(def); } } @@ -874,18 +874,18 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, goto error; } - if (VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + 1) < 0) { + if (VIR_REALLOC_N(def->srvs, def->nsrvs + 1) < 0) { virReportOOMError(); goto error; } - def->srvrecords[def->nsrvrecords].service = service; - def->srvrecords[def->nsrvrecords].protocol = protocol; - def->srvrecords[def->nsrvrecords].domain = NULL; - def->srvrecords[def->nsrvrecords].target = NULL; - def->srvrecords[def->nsrvrecords].port = 0; - def->srvrecords[def->nsrvrecords].priority = 0; - def->srvrecords[def->nsrvrecords].weight = 0; + def->srvs[def->nsrvs].service = service; + def->srvs[def->nsrvs].protocol = protocol; + def->srvs[def->nsrvs].domain = NULL; + def->srvs[def->nsrvs].target = NULL; + def->srvs[def->nsrvs].port = 0; + def->srvs[def->nsrvs].priority = 0; + def->srvs[def->nsrvs].weight = 0; /* Following attributes are optional but we had to make sure they're NULL above */ if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { @@ -893,23 +893,23 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, ctxt->node = cur; if (virXPathInt("string(./@port)", ctxt, &port)) - def->srvrecords[def->nsrvrecords].port = port; + def->srvs[def->nsrvs].port = port; if (virXPathInt("string(./@priority)", ctxt, &priority)) - def->srvrecords[def->nsrvrecords].priority = priority; + def->srvs[def->nsrvs].priority = priority; if (virXPathInt("string(./@weight)", ctxt, &weight)) - def->srvrecords[def->nsrvrecords].weight = weight; + def->srvs[def->nsrvs].weight = weight; ctxt->node = save_ctxt; - def->srvrecords[def->nsrvrecords].domain = domain; - def->srvrecords[def->nsrvrecords].target = target; - def->srvrecords[def->nsrvrecords].port = port; - def->srvrecords[def->nsrvrecords].priority = priority; - def->srvrecords[def->nsrvrecords].weight = weight; + def->srvs[def->nsrvs].domain = domain; + def->srvs[def->nsrvs].target = target; + def->srvs[def->nsrvs].port = port; + def->srvs[def->nsrvs].priority = priority; + def->srvs[def->nsrvs].weight = weight; } - def->nsrvrecords++; + def->nsrvs++; goto cleanup; @@ -962,14 +962,14 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, goto error; } - if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) { + if (VIR_REALLOC_N(def->txts, def->ntxts + 1) < 0) { virReportOOMError(); goto error; } - def->txtrecords[def->ntxtrecords].name = name; - def->txtrecords[def->ntxtrecords].value = value; - def->ntxtrecords++; + def->txts[def->ntxts].name = name; + def->txts[def->ntxts].value = value; + def->ntxts++; name = NULL; value = NULL; } else if (cur->type == XML_ELEMENT_NODE && @@ -1681,28 +1681,28 @@ virNetworkDNSDefFormat(virBufferPtr buf, virBufferAddLit(buf, "<dns>\n"); virBufferAdjustIndent(buf, 2); - for (i = 0 ; i < def->ntxtrecords ; i++) { + for (i = 0 ; i < def->ntxts ; i++) { virBufferAsprintf(buf, "<txt name='%s' value='%s' />\n", - def->txtrecords[i].name, - def->txtrecords[i].value); + def->txts[i].name, + def->txts[i].value); } - for (i = 0 ; i < def->nsrvrecords ; i++) { - if (def->srvrecords[i].service && def->srvrecords[i].protocol) { + for (i = 0 ; i < def->nsrvs ; i++) { + if (def->srvs[i].service && def->srvs[i].protocol) { virBufferAsprintf(buf, "<srv service='%s' protocol='%s'", - def->srvrecords[i].service, - def->srvrecords[i].protocol); - - if (def->srvrecords[i].domain) - virBufferAsprintf(buf, " domain='%s'", def->srvrecords[i].domain); - if (def->srvrecords[i].target) - virBufferAsprintf(buf, " target='%s'", def->srvrecords[i].target); - if (def->srvrecords[i].port) - virBufferAsprintf(buf, " port='%d'", def->srvrecords[i].port); - if (def->srvrecords[i].priority) - virBufferAsprintf(buf, " priority='%d'", def->srvrecords[i].priority); - if (def->srvrecords[i].weight) - virBufferAsprintf(buf, " weight='%d'", def->srvrecords[i].weight); + def->srvs[i].service, + def->srvs[i].protocol); + + if (def->srvs[i].domain) + virBufferAsprintf(buf, " domain='%s'", def->srvs[i].domain); + if (def->srvs[i].target) + virBufferAsprintf(buf, " target='%s'", def->srvs[i].target); + if (def->srvs[i].port) + virBufferAsprintf(buf, " port='%d'", def->srvs[i].port); + if (def->srvs[i].priority) + virBufferAsprintf(buf, " priority='%d'", def->srvs[i].priority); + if (def->srvs[i].weight) + virBufferAsprintf(buf, " weight='%d'", def->srvs[i].weight); virBufferAsprintf(buf, "/>\n"); } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index d4dc214..e78ddae 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -76,16 +76,16 @@ struct _virNetworkDHCPHostDef { virSocketAddr ip; }; -typedef struct _virNetworkDNSTxtRecordsDef virNetworkDNSTxtRecordsDef; -typedef virNetworkDNSTxtRecordsDef *virNetworkDNSTxtRecordsDefPtr; -struct _virNetworkDNSTxtRecordsDef { +typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef; +typedef virNetworkDNSTxtDef *virNetworkDNSTxtDefPtr; +struct _virNetworkDNSTxtDef { char *name; char *value; }; -typedef struct _virNetworkDNSSrvRecordsDef virNetworkDNSSrvRecordsDef; -typedef virNetworkDNSSrvRecordsDef *virNetworkDNSSrvRecordsDefPtr; -struct _virNetworkDNSSrvRecordsDef { +typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef; +typedef virNetworkDNSSrvDef *virNetworkDNSSrvDefPtr; +struct _virNetworkDNSSrvDef { char *domain; char *service; char *protocol; @@ -95,21 +95,23 @@ struct _virNetworkDNSSrvRecordsDef { int weight; }; -struct _virNetworkDNSHostsDef { +typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef; +typedef virNetworkDNSHostDef *virNetworkDNSHostDefPtr; +struct _virNetworkDNSHostDef { virSocketAddr ip; int nnames; char **names; }; -typedef struct _virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr; - +typedef struct _virNetworkDNSDef virNetworkDNSDef; +typedef virNetworkDNSDef *virNetworkDNSDefPtr; struct _virNetworkDNSDef { - unsigned int ntxtrecords; - virNetworkDNSTxtRecordsDefPtr txtrecords; - unsigned int nhosts; - virNetworkDNSHostsDefPtr hosts; - unsigned int nsrvrecords; - virNetworkDNSSrvRecordsDefPtr srvrecords; + size_t ntxts; + virNetworkDNSTxtDefPtr txts; + size_t nhosts; + virNetworkDNSHostDefPtr hosts; + size_t nsrvs; + virNetworkDNSSrvDefPtr srvs; }; typedef struct _virNetworkDNSDef *virNetworkDNSDefPtr; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 00cffee..453bb63 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -580,7 +580,7 @@ networkBuildDnsmasqHostsfile(dnsmasqContext *dctx, if (dnsdef) { for (i = 0; i < dnsdef->nhosts; i++) { - virNetworkDNSHostsDefPtr host = &(dnsdef->hosts[i]); + virNetworkDNSHostDefPtr host = &(dnsdef->hosts[i]); if (VIR_SOCKET_ADDR_VALID(&host->ip)) { for (j = 0; j < host->nnames; j++) if (dnsmasqAddHost(dctx, &host->ip, host->names[j]) < 0) @@ -719,38 +719,38 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, virNetworkDNSDefPtr dns = network->def->dns; int i; - for (i = 0; i < dns->ntxtrecords; i++) { + for (i = 0; i < dns->ntxts; i++) { virCommandAddArgFormat(cmd, "--txt-record=%s,%s", - dns->txtrecords[i].name, - dns->txtrecords[i].value); + dns->txts[i].name, + dns->txts[i].value); } - for (i = 0; i < dns->nsrvrecords; i++) { - if (dns->srvrecords[i].service && dns->srvrecords[i].protocol) { - if (dns->srvrecords[i].port) { - if (virAsprintf(&recordPort, "%d", dns->srvrecords[i].port) < 0) { + for (i = 0; i < dns->nsrvs; i++) { + if (dns->srvs[i].service && dns->srvs[i].protocol) { + if (dns->srvs[i].port) { + if (virAsprintf(&recordPort, "%d", dns->srvs[i].port) < 0) { virReportOOMError(); goto cleanup; } } - if (dns->srvrecords[i].priority) { - if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i].priority) < 0) { + if (dns->srvs[i].priority) { + if (virAsprintf(&recordPriority, "%d", dns->srvs[i].priority) < 0) { virReportOOMError(); goto cleanup; } } - if (dns->srvrecords[i].weight) { - if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i].weight) < 0) { + if (dns->srvs[i].weight) { + if (virAsprintf(&recordWeight, "%d", dns->srvs[i].weight) < 0) { virReportOOMError(); goto cleanup; } } if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", - dns->srvrecords[i].service, - dns->srvrecords[i].protocol, - dns->srvrecords[i].domain ? dns->srvrecords[i].domain : "", - dns->srvrecords[i].target ? dns->srvrecords[i].target : "", + dns->srvs[i].service, + dns->srvs[i].protocol, + dns->srvs[i].domain ? dns->srvs[i].domain : "", + dns->srvs[i].target ? dns->srvs[i].target : "", recordPort ? recordPort : "", recordPriority ? recordPriority : "", recordWeight ? recordWeight : "") < 0) { @@ -2764,7 +2764,7 @@ networkValidate(struct network_driver *driver, return -1; } if (def->dns && - (def->dns->ntxtrecords || def->dns->nhosts || def->dns->nsrvrecords)) { + (def->dns->ntxts || def->dns->nhosts || def->dns->nsrvs)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported <dns> element in network %s " "with forward mode='%s'"), -- 1.7.11.7
Since there is only a single virNetworkDNSDef for any virNetworkDef, and it's trivial to determine whether or not it contains any real data, it's much simpler (and fits more uniformly with the parse function calling sequence of the parsers for many other objects that are subordinates of virNetworkDef) if virNetworkDef *contains* an virNetworkDNSDef rather than pointing to one. Since it is now just a part of another object rather than its own object, it no longer makes sense to have a *Free() function, so that is changed to a *Clear() function. More importantly though, ParseXML and Clear functions are needed for the individual items contained in a virNetworkDNSDef (srv, txt, and host records), but none of them have a *Clear(), and only two of the three had *ParseXML() functions (both of which used a non-uniform arglist). Those problems are cleared up by this patch - it splits the higher-level Clear function into separate functions for each of the three, creates a parse for txt records, and cleans up the srv and host parsers, so we now have all the utility functions necessary to implement virNetworkDefUpdateDNS(Host|Srv|Txt). --- src/conf/network_conf.c | 412 ++++++++++++++++++++++++++------------------ src/conf/network_conf.h | 8 +- src/network/bridge_driver.c | 89 +++++----- 3 files changed, 286 insertions(+), 223 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 7caf39f..3b28b5e 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -133,34 +133,47 @@ static void virNetworkIpDefClear(virNetworkIpDefPtr def) VIR_FREE(def->bootfile); } -static void virNetworkDNSDefFree(virNetworkDNSDefPtr def) +static void +virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def) { - if (def) { - if (def->txts) { - while (def->ntxts--) { - VIR_FREE(def->txts[def->ntxts].name); - VIR_FREE(def->txts[def->ntxts].value); - } - } + VIR_FREE(def->name); + VIR_FREE(def->value); +} + +static void +virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def) +{ + while (def->nnames--) + VIR_FREE(def->names[def->nnames]); + VIR_FREE(def->names); +} + +static void +virNetworkDNSSrvDefClear(virNetworkDNSSrvDefPtr def) +{ + VIR_FREE(def->domain); + VIR_FREE(def->service); + VIR_FREE(def->protocol); + VIR_FREE(def->target); +} + +static void +virNetworkDNSDefClear(virNetworkDNSDefPtr def) +{ + if (def->txts) { + while (def->ntxts--) + virNetworkDNSTxtDefClear(&def->txts[def->ntxts]); VIR_FREE(def->txts); - if (def->nhosts) { - while (def->nhosts--) { - while (def->hosts[def->nhosts].nnames--) - VIR_FREE(def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames]); - VIR_FREE(def->hosts[def->nhosts].names); - } - } + } + if (def->hosts) { + while (def->nhosts--) + virNetworkDNSHostDefClear(&def->hosts[def->nhosts]); VIR_FREE(def->hosts); - if (def->nsrvs) { - while (def->nsrvs--) { - VIR_FREE(def->srvs[def->nsrvs].domain); - VIR_FREE(def->srvs[def->nsrvs].service); - VIR_FREE(def->srvs[def->nsrvs].protocol); - VIR_FREE(def->srvs[def->nsrvs].target); - } - } + } + if (def->srvs) { + while (def->nsrvs--) + virNetworkDNSSrvDefClear(&def->srvs[def->nsrvs]); VIR_FREE(def->srvs); - VIR_FREE(def); } } @@ -195,7 +208,7 @@ void virNetworkDefFree(virNetworkDefPtr def) } VIR_FREE(def->portGroups); - virNetworkDNSDefFree(def->dns); + virNetworkDNSDefClear(&def->dns); VIR_FREE(def->virtPortProfile); @@ -778,224 +791,279 @@ virNetworkDHCPDefParse(const char *networkName, } static int -virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def, - xmlNodePtr node) +virNetworkDNSHostDefParseXML(const char *networkName, + xmlNodePtr node, + virNetworkDNSHostDefPtr def, + bool partialOkay) { xmlNodePtr cur; char *ip; - virSocketAddr inaddr; - int ret = -1; - if (!(ip = virXMLPropString(node, "ip")) || - (virSocketAddrParse(&inaddr, ip, AF_UNSPEC) < 0)) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("Missing IP address in DNS host definition")); + if (!(ip = virXMLPropString(node, "ip")) && !partialOkay) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing IP address in network '%s' DNS HOST record"), + networkName); VIR_FREE(ip); goto error; } - VIR_FREE(ip); - - if (VIR_REALLOC_N(def->hosts, def->nhosts + 1) < 0) { - virReportOOMError(); - goto error; - } - - def->hosts[def->nhosts].ip = inaddr; - def->hosts[def->nhosts].nnames = 0; - if (VIR_ALLOC(def->hosts[def->nhosts].names) < 0) { - virReportOOMError(); + if (ip && (virSocketAddrParse(&def->ip, ip, AF_UNSPEC) < 0)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Invalid IP address in network '%s' DNS HOST record"), + networkName); + VIR_FREE(ip); goto error; } + VIR_FREE(ip); cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "hostname")) { if (cur->children != NULL) { - if (VIR_REALLOC_N(def->hosts[def->nhosts].names, def->hosts[def->nhosts].nnames + 1) < 0) { + if (VIR_REALLOC_N(def->names, def->nnames + 1) < 0) { virReportOOMError(); goto error; } - - def->hosts[def->nhosts].names[def->hosts[def->nhosts].nnames] = strdup((char *)cur->children->content); - def->hosts[def->nhosts].nnames++; + def->names[def->nnames++] = (char *)xmlNodeGetContent(cur); + if (!def->names[def->nnames - 1]) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing hostname in network '%s' DNS HOST record"), + networkName); + } } } - cur = cur->next; } + if (def->nnames == 0&& !partialOkay) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing hostname in network '%s' DNS HOST record"), + networkName); + goto error; + } - def->nhosts++; + if (!VIR_SOCKET_ADDR_VALID(&def->ip) && def->nnames == 0) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing ip and hostname in network '%s' DNS HOST record"), + networkName); + goto error; + } - ret = 0; + return 0; error: - return ret; + virNetworkDNSHostDefClear(def); + return -1; } static int -virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, - xmlNodePtr cur, - xmlXPathContextPtr ctxt) +virNetworkDNSSrvDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkDNSSrvDefPtr def, + bool partialOkay) { - char *domain = NULL; - char *service = NULL; - char *protocol = NULL; - char *target = NULL; - int port; - int priority; - int weight; - int ret = 0; - - if (!(service = virXMLPropString(cur, "service"))) { + if (!(def->service = virXMLPropString(node, "service")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, - "%s", _("Missing required service attribute in dns srv record")); + _("Missing required service attribute in DNS SRV record " + " of network %s"), networkName); goto error; } - - if (strlen(service) > DNS_RECORD_LENGTH_SRV) { + if (strlen(def->service) > DNS_RECORD_LENGTH_SRV) { virReportError(VIR_ERR_XML_DETAIL, - _("Service name is too long, limit is %d bytes"), - DNS_RECORD_LENGTH_SRV); + _("Service name '%s' in network %s is too long, limit is %d bytes"), + def->service, networkName, DNS_RECORD_LENGTH_SRV); goto error; } - if (!(protocol = virXMLPropString(cur, "protocol"))) { + if (!(def->protocol = virXMLPropString(node, "protocol")) && !partialOkay) { virReportError(VIR_ERR_XML_DETAIL, - _("Missing required protocol attribute in dns srv record '%s'"), service); + _("Missing required protocol attribute " + "in dns srv record '%s' of network %s"), + def->service, networkName); goto error; } /* Check whether protocol value is the supported one */ - if (STRNEQ(protocol, "tcp") && (STRNEQ(protocol, "udp"))) { + if (STRNEQ(def->protocol, "tcp") && (STRNEQ(def->protocol, "udp"))) { virReportError(VIR_ERR_XML_DETAIL, - _("Invalid protocol attribute value '%s'"), protocol); + _("Invalid protocol attribute value '%s' " + " in DNS SRV record of network %s"), + def->protocol, networkName); goto error; } - if (VIR_REALLOC_N(def->srvs, def->nsrvs + 1) < 0) { - virReportOOMError(); - goto error; - } - - def->srvs[def->nsrvs].service = service; - def->srvs[def->nsrvs].protocol = protocol; - def->srvs[def->nsrvs].domain = NULL; - def->srvs[def->nsrvs].target = NULL; - def->srvs[def->nsrvs].port = 0; - def->srvs[def->nsrvs].priority = 0; - def->srvs[def->nsrvs].weight = 0; - - /* Following attributes are optional but we had to make sure they're NULL above */ - if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { + /* Following attributes are optional */ + if ((def->target = virXMLPropString(node, "target")) && + (def->domain = virXMLPropString(node, "domain"))) { xmlNodePtr save_ctxt = ctxt->node; - ctxt->node = cur; - if (virXPathInt("string(./@port)", ctxt, &port)) - def->srvs[def->nsrvs].port = port; + ctxt->node = node; + if (virXPathUInt("string(./@port)", ctxt, &def->port) < 0 || + def->port > 65535) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid port attribute " + "in network %s"), networkName); + goto error; + } - if (virXPathInt("string(./@priority)", ctxt, &priority)) - def->srvs[def->nsrvs].priority = priority; + if (virXPathUInt("string(./@priority)", ctxt, &def->priority) < 0 || + def->priority > 65535) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid priority attribute " + "in network %s"), networkName); + goto error; + } - if (virXPathInt("string(./@weight)", ctxt, &weight)) - def->srvs[def->nsrvs].weight = weight; - ctxt->node = save_ctxt; + if (virXPathUInt("string(./@weight)", ctxt, &def->weight) < 0 || + def->weight > 65535) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing or invalid weight attribute " + "in network %s"), networkName); + goto error; + } - def->srvs[def->nsrvs].domain = domain; - def->srvs[def->nsrvs].target = target; - def->srvs[def->nsrvs].port = port; - def->srvs[def->nsrvs].priority = priority; - def->srvs[def->nsrvs].weight = weight; + ctxt->node = save_ctxt; } - def->nsrvs++; - - goto cleanup; + if (!(def->service || def->protocol)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing required service attribute or protocol " + "in DNS SRV record of network %s"), networkName); + goto error; + } + return 0; error: - VIR_FREE(domain); - VIR_FREE(service); - VIR_FREE(protocol); - VIR_FREE(target); + virNetworkDNSSrvDefClear(def); + return -1; +} - ret = -1; +static int +virNetworkDNSTxtDefParseXML(const char *networkName, + xmlNodePtr node, + virNetworkDNSTxtDefPtr def, + bool partialOkay) +{ + if (!(def->name = virXMLPropString(node, "name"))) { + virReportError(VIR_ERR_XML_DETAIL, + _("missing required name attribute in DNS TXT record " + "of network %s"), networkName); + goto error; + } + if (strchr(def->name, ' ') != NULL) { + virReportError(VIR_ERR_XML_DETAIL, + _("prohibited space character in DNS TXT record " + "name '%s' of network %s"), def->name, networkName); + goto error; + } + if (!(def->value = virXMLPropString(node, "value")) && !partialOkay) { + virReportError(VIR_ERR_XML_DETAIL, + _("missing required value attribute in DNS TXT record " + "named '%s' of network %s"), def->name, networkName); + goto error; + } -cleanup: - return ret; + if (!(def->name || def->value)) { + virReportError(VIR_ERR_XML_DETAIL, + _("Missing required name or value " + "in DNS TXT record of network %s"), networkName); + goto error; + } + return 0; + +error: + virNetworkDNSTxtDefClear(def); + return -1; } static int -virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, +virNetworkDNSDefParseXML(const char *networkName, xmlNodePtr node, - xmlXPathContextPtr ctxt) + xmlXPathContextPtr ctxt, + virNetworkDNSDefPtr def) { - xmlNodePtr cur; - int ret = -1; - char *name = NULL; - char *value = NULL; - virNetworkDNSDefPtr def = NULL; + xmlNodePtr *hostNodes = NULL; + xmlNodePtr *srvNodes = NULL; + xmlNodePtr *txtNodes = NULL; + int nhosts, nsrvs, ntxts; + int ii, ret = -1; + xmlNodePtr save = ctxt->node; - if (VIR_ALLOC(def) < 0) { - virReportOOMError(); - goto error; + ctxt->node = node; + + nhosts = virXPathNodeSet("./host", ctxt, &hostNodes); + if (nhosts < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <host> element found in <dns> of network %s"), + networkName); + goto cleanup; } + if (nhosts > 0) { + if (VIR_ALLOC_N(def->hosts, nhosts) < 0) { + virReportOOMError(); + goto cleanup; + } - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "txt")) { - if (!(name = virXMLPropString(cur, "name"))) { - virReportError(VIR_ERR_XML_DETAIL, - "%s", _("Missing required name attribute in dns txt record")); - goto error; - } - if (!(value = virXMLPropString(cur, "value"))) { - virReportError(VIR_ERR_XML_DETAIL, - _("Missing required value attribute in dns txt record '%s'"), name); - goto error; + for (ii = 0; ii < nhosts; ii++) { + if (virNetworkDNSHostDefParseXML(networkName, hostNodes[ii], + &def->hosts[def->nhosts], false) < 0) { + goto cleanup; } + def->nhosts++; + } + } - if (strchr(name, ' ') != NULL) { - virReportError(VIR_ERR_XML_DETAIL, - _("spaces are not allowed in DNS TXT record names (name is '%s')"), name); - goto error; - } + nsrvs = virXPathNodeSet("./srv", ctxt, &srvNodes); + if (nsrvs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <srv> element found in <dns> of network %s"), + networkName); + goto cleanup; + } + if (nsrvs > 0) { + if (VIR_ALLOC_N(def->srvs, nsrvs) < 0) { + virReportOOMError(); + goto cleanup; + } - if (VIR_REALLOC_N(def->txts, def->ntxts + 1) < 0) { - virReportOOMError(); - goto error; + for (ii = 0; ii < nsrvs; ii++) { + if (virNetworkDNSSrvDefParseXML(networkName, srvNodes[ii], ctxt, + &def->srvs[def->nsrvs], false) < 0) { + goto cleanup; } + def->nsrvs++; + } + } - def->txts[def->ntxts].name = name; - def->txts[def->ntxts].value = value; - def->ntxts++; - name = NULL; - value = NULL; - } else if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "srv")) { - ret = virNetworkDNSSrvDefParseXML(def, cur, ctxt); - if (ret < 0) - goto error; - } else if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "host")) { - ret = virNetworkDNSHostsDefParseXML(def, cur); - if (ret < 0) - goto error; + ntxts = virXPathNodeSet("./txt", ctxt, &txtNodes); + if (ntxts < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <txt> element found in <dns> of network %s"), + networkName); + goto cleanup; + } + if (ntxts > 0) { + if (VIR_ALLOC_N(def->txts, ntxts) < 0) { + virReportOOMError(); + goto cleanup; } - cur = cur->next; + for (ii = 0; ii < ntxts; ii++) { + if (virNetworkDNSTxtDefParseXML(networkName, txtNodes[ii], + &def->txts[def->ntxts], false) < 0) { + goto cleanup; + } + def->ntxts++; + } } ret = 0; -error: - if (ret < 0) { - VIR_FREE(name); - VIR_FREE(value); - virNetworkDNSDefFree(def); - } else { - *dnsdef = def; - } +cleanup: + VIR_FREE(hostNodes); + VIR_FREE(srvNodes); + VIR_FREE(txtNodes); + ctxt->node = save; return ret; } @@ -1311,9 +1379,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } dnsNode = virXPathNode("./dns", ctxt); - if (dnsNode != NULL) { - if (virNetworkDNSDefParseXML(&def->dns, dnsNode, ctxt) < 0) - goto error; + if (dnsNode != NULL && + virNetworkDNSDefParseXML(def->name, dnsNode, ctxt, &def->dns) < 0) { + goto error; } virtPortNode = virXPathNode("./virtualport", ctxt); @@ -1675,7 +1743,7 @@ virNetworkDNSDefFormat(virBufferPtr buf, int result = 0; int i; - if (def == NULL) + if (!(def->nhosts || def->nsrvs || def->ntxts)) goto out; virBufferAddLit(buf, "<dns>\n"); @@ -1950,7 +2018,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) if (def->domain) virBufferAsprintf(&buf, "<domain name='%s'/>\n", def->domain); - if (virNetworkDNSDefFormat(&buf, def->dns) < 0) + if (virNetworkDNSDefFormat(&buf, &def->dns) < 0) goto error; if (virNetDevVlanFormat(&def->vlan, &buf) < 0) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index e78ddae..c0d4fa2 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -90,9 +90,9 @@ struct _virNetworkDNSSrvDef { char *service; char *protocol; char *target; - int port; - int priority; - int weight; + unsigned int port; + unsigned int priority; + unsigned int weight; }; typedef struct _virNetworkDNSHostDef virNetworkDNSHostDef; @@ -206,7 +206,7 @@ struct _virNetworkDef { size_t nips; virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */ - virNetworkDNSDefPtr dns; /* ptr to dns related configuration */ + virNetworkDNSDef dns; /* dns related configuration */ virNetDevVPortProfilePtr virtPortProfile; size_t nPortGroups; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 453bb63..a5b9182 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -608,6 +608,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, char *recordPort = NULL; char *recordWeight = NULL; char *recordPriority = NULL; + virNetworkDNSDefPtr dns = &network->def->dns; virNetworkIpDefPtr tmpipdef; /* @@ -715,55 +716,50 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, "--no-resolv", NULL); } - if (network->def->dns != NULL) { - virNetworkDNSDefPtr dns = network->def->dns; - int i; - - for (i = 0; i < dns->ntxts; i++) { - virCommandAddArgFormat(cmd, "--txt-record=%s,%s", - dns->txts[i].name, - dns->txts[i].value); - } + for (ii = 0; ii < dns->ntxts; ii++) { + virCommandAddArgFormat(cmd, "--txt-record=%s,%s", + dns->txts[ii].name, + dns->txts[ii].value); + } - for (i = 0; i < dns->nsrvs; i++) { - if (dns->srvs[i].service && dns->srvs[i].protocol) { - if (dns->srvs[i].port) { - if (virAsprintf(&recordPort, "%d", dns->srvs[i].port) < 0) { - virReportOOMError(); - goto cleanup; - } - } - if (dns->srvs[i].priority) { - if (virAsprintf(&recordPriority, "%d", dns->srvs[i].priority) < 0) { - virReportOOMError(); - goto cleanup; - } + for (ii = 0; ii < dns->nsrvs; ii++) { + if (dns->srvs[ii].service && dns->srvs[ii].protocol) { + if (dns->srvs[ii].port) { + if (virAsprintf(&recordPort, "%d", dns->srvs[ii].port) < 0) { + virReportOOMError(); + goto cleanup; } - if (dns->srvs[i].weight) { - if (virAsprintf(&recordWeight, "%d", dns->srvs[i].weight) < 0) { - virReportOOMError(); - goto cleanup; - } + } + if (dns->srvs[ii].priority) { + if (virAsprintf(&recordPriority, "%d", dns->srvs[ii].priority) < 0) { + virReportOOMError(); + goto cleanup; } - - if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", - dns->srvs[i].service, - dns->srvs[i].protocol, - dns->srvs[i].domain ? dns->srvs[i].domain : "", - dns->srvs[i].target ? dns->srvs[i].target : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) { + } + if (dns->srvs[ii].weight) { + if (virAsprintf(&recordWeight, "%d", dns->srvs[ii].weight) < 0) { virReportOOMError(); goto cleanup; } + } - virCommandAddArgPair(cmd, "--srv-host", record); - VIR_FREE(record); - VIR_FREE(recordPort); - VIR_FREE(recordWeight); - VIR_FREE(recordPriority); + if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", + dns->srvs[ii].service, + dns->srvs[ii].protocol, + dns->srvs[ii].domain ? dns->srvs[ii].domain : "", + dns->srvs[ii].target ? dns->srvs[ii].target : "", + recordPort ? recordPort : "", + recordPriority ? recordPriority : "", + recordWeight ? recordWeight : "") < 0) { + virReportOOMError(); + goto cleanup; } + + virCommandAddArgPair(cmd, "--srv-host", record); + VIR_FREE(record); + VIR_FREE(recordPort); + VIR_FREE(recordWeight); + VIR_FREE(recordPriority); } } @@ -813,9 +809,9 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, /* add domain to any non-qualified hostnames in /etc/hosts or addn-hosts */ if (network->def->domain) - virCommandAddArg(cmd, "--expand-hosts"); + virCommandAddArg(cmd, "--expand-hosts"); - if (networkBuildDnsmasqHostsfile(dctx, ipdef, network->def->dns) < 0) + if (networkBuildDnsmasqHostsfile(dctx, ipdef, &network->def->dns) < 0) goto cleanup; /* Even if there are currently no static hosts, if we're @@ -965,7 +961,7 @@ networkStartDhcpDaemon(struct network_driver *driver, if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET) && (ipdef->nranges || ipdef->nhosts)) { if (networkBuildDnsmasqHostsfile(dctx, ipdef, - network->def->dns) < 0) + &network->def->dns) < 0) goto cleanup; break; @@ -1040,7 +1036,7 @@ networkRefreshDhcpDaemon(struct network_driver *driver, if (!(dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR))) goto cleanup; - if (networkBuildDnsmasqHostsfile(dctx, ipdef, network->def->dns) < 0) + if (networkBuildDnsmasqHostsfile(dctx, ipdef, &network->def->dns) < 0) goto cleanup; if ((ret = dnsmasqSave(dctx)) < 0) @@ -2763,8 +2759,7 @@ networkValidate(struct network_driver *driver, virNetworkForwardTypeToString(def->forwardType)); return -1; } - if (def->dns && - (def->dns->ntxts || def->dns->nhosts || def->dns->nsrvs)) { + if (def->dns.ntxts || def->dns.nhosts || def->dns.nsrvs) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported <dns> element in network %s " "with forward mode='%s'"), -- 1.7.11.7
These three functions are very similar - none allow a MODIFY operation; you can only add or delete. The biggest difference between them (other than the data itself) is in the criteria for determining a match, and whether or not multiple matches are possible: 1) for HOST records, it's considered a match if the IP address or any of the hostnames of an existing record matches. 2) for SRV records, it's a match if all of domain+service+protocol+target *which have been specified* are matched. 3) for TXT records, there is only a single field to match - name (value can be the same for multiple records, and isn't considered a search term), so by definition there can be no ambiguous matches. In all three cases, if any matches are found, ADD will fail; if multiple matches are found, it means the search term was ambiguous, and a DELETE will fail. The upper level code in bridge_driver.c is already implemented for these functions - appropriate conf files will be re-written, and dnsmasq will be SIGHUPed or restarted as appropriate. --- src/conf/network_conf.c | 240 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 232 insertions(+), 8 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3b28b5e..a71c2d9 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2915,32 +2915,256 @@ virNetworkDefUpdateDNSHost(virNetworkDefPtr def, /* virNetworkUpdateFlags */ unsigned int fflags ATTRIBUTE_UNUSED) { - virNetworkDefUpdateNoSupport(def, "dns host"); - return -1; + int ii, jj, kk, foundIdx, ret = -1; + virNetworkDNSDefPtr dns = &def->dns; + virNetworkDNSHostDef host; + bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); + bool foundCt = 0; + + memset(&host, 0, sizeof(host)); + + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("DNS HOST records cannot be modified, " + "only added or deleted")); + goto cleanup; + } + + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "host") < 0) + goto cleanup; + + if (virNetworkDNSHostDefParseXML(def->name, ctxt->node, &host, !isAdd) < 0) + goto cleanup; + + for (ii = 0; ii < dns->nhosts; ii++) { + bool foundThisTime = false; + + if (virSocketAddrEqual(&host.ip, &dns->hosts[ii].ip)) + foundThisTime = true; + + for (jj = 0; jj < host.nnames && !foundThisTime; jj++) { + for (kk = 0; kk < dns->hosts[ii].nnames && !foundThisTime; kk++) { + if (STREQ(host.names[jj], dns->hosts[ii].names[kk])) + foundThisTime = true; + } + } + if (foundThisTime) { + foundCt++; + foundIdx = ii; + } + } + + if (isAdd) { + + if (foundCt > 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("there is already at least one DNS HOST " + "record with a matching field in network %s"), + def->name); + goto cleanup; + } + + /* add to beginning/end of list */ + if (VIR_INSERT_ELEMENT(dns->hosts, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : dns->nhosts, dns->nhosts, &host) < 0) { + virReportOOMError(); + goto cleanup; + } + + } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { + + if (foundCt == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS HOST " + "record in network %s"), def->name); + goto cleanup; + } + if (foundCt > 1) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("multiple matching DNS HOST records were " + "found in network %s"), def->name); + goto cleanup; + } + + /* remove it */ + virNetworkDNSHostDefClear(&dns->hosts[foundIdx]); + VIR_DELETE_ELEMENT(dns->hosts, foundIdx, dns->nhosts); + + } else { + virNetworkDefUpdateUnknownCommand(command); + goto cleanup; + } + + ret = 0; +cleanup: + virNetworkDNSHostDefClear(&host); + return ret; } static int -virNetworkDefUpdateDNSTxt(virNetworkDefPtr def, +virNetworkDefUpdateDNSSrv(virNetworkDefPtr def, unsigned int command ATTRIBUTE_UNUSED, int parentIndex ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, /* virNetworkUpdateFlags */ unsigned int fflags ATTRIBUTE_UNUSED) { - virNetworkDefUpdateNoSupport(def, "dns txt"); - return -1; + int ii, foundIdx, ret = -1; + virNetworkDNSDefPtr dns = &def->dns; + virNetworkDNSSrvDef srv; + bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); + bool foundCt = 0; + + memset(&srv, 0, sizeof(srv)); + + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("DNS SRV records cannot be modified, " + "only added or deleted")); + goto cleanup; + } + + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "srv") < 0) + goto cleanup; + + if (virNetworkDNSSrvDefParseXML(def->name, ctxt->node, ctxt, &srv, !isAdd) < 0) + goto cleanup; + + for (ii = 0; ii < dns->nsrvs; ii++) { + if ((!srv.domain || STREQ_NULLABLE(srv.domain, dns->srvs[ii].domain)) && + (!srv.service || STREQ_NULLABLE(srv.service, dns->srvs[ii].service)) && + (!srv.protocol || STREQ_NULLABLE(srv.protocol, dns->srvs[ii].protocol)) && + (!srv.target || STREQ_NULLABLE(srv.target, dns->srvs[ii].target))) { + foundCt++; + foundIdx = ii; + } + } + + if (isAdd) { + + if (foundCt > 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("there is already at least one DNS SRV " + "record matching all specified fields in network %s"), + def->name); + goto cleanup; + } + + /* add to beginning/end of list */ + if (VIR_INSERT_ELEMENT(dns->srvs, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : dns->nsrvs, dns->nsrvs, &srv) < 0) { + virReportOOMError(); + goto cleanup; + } + + } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { + + if (foundCt == 0) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS SRV " + "record in network %s"), def->name); + goto cleanup; + } + if (foundCt > 1) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("multiple DNS SRV records matching all specified " + "fields were found in network %s"), def->name); + goto cleanup; + } + + /* remove it */ + virNetworkDNSSrvDefClear(&dns->srvs[foundIdx]); + VIR_DELETE_ELEMENT(dns->srvs, foundIdx, dns->nsrvs); + + } else { + virNetworkDefUpdateUnknownCommand(command); + goto cleanup; + } + + ret = 0; +cleanup: + virNetworkDNSSrvDefClear(&srv); + return ret; } static int -virNetworkDefUpdateDNSSrv(virNetworkDefPtr def, +virNetworkDefUpdateDNSTxt(virNetworkDefPtr def, unsigned int command ATTRIBUTE_UNUSED, int parentIndex ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED, /* virNetworkUpdateFlags */ unsigned int fflags ATTRIBUTE_UNUSED) { - virNetworkDefUpdateNoSupport(def, "dns txt"); - return -1; + int foundIdx, ret = -1; + virNetworkDNSDefPtr dns = &def->dns; + virNetworkDNSTxtDef txt; + bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST || + command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST); + + memset(&txt, 0, sizeof(txt)); + + if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("DNS TXT records cannot be modified, " + "only added or deleted")); + goto cleanup; + } + + if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0) + goto cleanup; + + if (virNetworkDNSTxtDefParseXML(def->name, ctxt->node, &txt, !isAdd) < 0) + goto cleanup; + + for (foundIdx = 0; foundIdx < dns->ntxts; foundIdx++) { + if (STREQ(txt.name, dns->txts[foundIdx].name)) + break; + } + + if (isAdd) { + + if (foundIdx < dns->ntxts) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("there is already a DNS TXT record " + "with name '%s' in network %s"), + txt.name, def->name); + goto cleanup; + } + + /* add to beginning/end of list */ + if (VIR_INSERT_ELEMENT(dns->txts, + command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST + ? 0 : dns->ntxts, dns->ntxts, &txt) < 0) { + virReportOOMError(); + goto cleanup; + } + + } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { + + if (foundIdx == dns->ntxts) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("couldn't locate a matching DNS TXT " + "record in network %s"), def->name); + goto cleanup; + } + + /* remove it */ + virNetworkDNSTxtDefClear(&dns->txts[foundIdx]); + VIR_DELETE_ELEMENT(dns->txts, foundIdx, dns->ntxts); + + } else { + virNetworkDefUpdateUnknownCommand(command); + goto cleanup; + } + + ret = 0; +cleanup: + virNetworkDNSTxtDefClear(&txt); + return ret; } static int -- 1.7.11.7
This makes some function names and arg lists for consistent with other parse functions in network_conf.c. While modifying virNetworkIPParseXML(), also change its "error" label to "cleanup", since the code at that label is executed on success as well as failure. --- src/conf/network_conf.c | 86 ++++++++++++++++++++++++------------------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index a71c2d9..35a4318 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -592,9 +592,9 @@ int virNetworkIpDefNetmask(const virNetworkIpDefPtr def, static int -virNetworkDHCPRangeDefParse(const char *networkName, - xmlNodePtr node, - virNetworkDHCPRangeDefPtr range) +virNetworkDHCPRangeDefParseXML(const char *networkName, + xmlNodePtr node, + virNetworkDHCPRangeDefPtr range) { @@ -636,10 +636,10 @@ cleanup: } static int -virNetworkDHCPHostDefParse(const char *networkName, - xmlNodePtr node, - virNetworkDHCPHostDefPtr host, - bool partialOkay) +virNetworkDHCPHostDefParseXML(const char *networkName, + xmlNodePtr node, + virNetworkDHCPHostDefPtr host, + bool partialOkay) { char *mac = NULL, *name = NULL, *ip = NULL; virMacAddr addr; @@ -723,9 +723,9 @@ cleanup: } static int -virNetworkDHCPDefParse(const char *networkName, - virNetworkIpDefPtr def, - xmlNodePtr node) +virNetworkDHCPDefParseXML(const char *networkName, + xmlNodePtr node, + virNetworkIpDefPtr def) { xmlNodePtr cur; @@ -739,8 +739,8 @@ virNetworkDHCPDefParse(const char *networkName, virReportOOMError(); return -1; } - if (virNetworkDHCPRangeDefParse(networkName, cur, - &def->ranges[def->nranges]) < 0) { + if (virNetworkDHCPRangeDefParseXML(networkName, cur, + &def->ranges[def->nranges]) < 0) { return -1; } def->nranges++; @@ -752,9 +752,9 @@ virNetworkDHCPDefParse(const char *networkName, virReportOOMError(); return -1; } - if (virNetworkDHCPHostDefParse(networkName, cur, - &def->hosts[def->nhosts], - false) < 0) { + if (virNetworkDHCPHostDefParseXML(networkName, cur, + &def->hosts[def->nhosts], + false) < 0) { return -1; } def->nhosts++; @@ -1068,10 +1068,10 @@ cleanup: } static int -virNetworkIPParseXML(const char *networkName, - virNetworkIpDefPtr def, - xmlNodePtr node, - xmlXPathContextPtr ctxt) +virNetworkIPDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkIpDefPtr def) { /* * virNetworkIpDef object is already allocated as part of an array. @@ -1101,7 +1101,7 @@ virNetworkIPParseXML(const char *networkName, virReportError(VIR_ERR_XML_ERROR, _("Bad address '%s' in definition of network '%s'"), address, networkName); - goto error; + goto cleanup; } } @@ -1113,27 +1113,27 @@ virNetworkIPParseXML(const char *networkName, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("no family specified for non-IPv4 address '%s' in network '%s'"), address, networkName); - goto error; + goto cleanup; } } else if (STREQ(def->family, "ipv4")) { if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("family 'ipv4' specified for non-IPv4 address '%s' in network '%s'"), address, networkName); - goto error; + goto cleanup; } } else if (STREQ(def->family, "ipv6")) { if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("family 'ipv6' specified for non-IPv6 address '%s' in network '%s'"), address, networkName); - goto error; + goto cleanup; } } else { virReportError(VIR_ERR_XML_ERROR, _("Unrecognized family '%s' in definition of network '%s'"), def->family, networkName); - goto error; + goto cleanup; } /* parse/validate netmask */ @@ -1143,14 +1143,14 @@ virNetworkIPParseXML(const char *networkName, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("netmask specified without address in network '%s'"), networkName); - goto error; + goto cleanup; } if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("netmask not supported for address '%s' in network '%s' (IPv4 only)"), address, networkName); - goto error; + goto cleanup; } if (def->prefix > 0) { @@ -1158,17 +1158,17 @@ virNetworkIPParseXML(const char *networkName, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("network '%s' cannot have both prefix='%u' and a netmask"), networkName, def->prefix); - goto error; + goto cleanup; } if (virSocketAddrParse(&def->netmask, netmask, AF_UNSPEC) < 0) - goto error; + goto cleanup; if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("network '%s' has invalid netmask '%s' for address '%s' (both must be IPv4)"), networkName, netmask, address); - goto error; + goto cleanup; } } @@ -1178,9 +1178,9 @@ virNetworkIPParseXML(const char *networkName, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "dhcp")) { - result = virNetworkDHCPDefParse(networkName, def, cur); + result = virNetworkDHCPDefParseXML(networkName, cur, def); if (result) - goto error; + goto cleanup; } else if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "tftp")) { @@ -1200,7 +1200,7 @@ virNetworkIPParseXML(const char *networkName, result = 0; -error: +cleanup: if (result < 0) { virNetworkIpDefClear(def); } @@ -1237,7 +1237,7 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, if (!def->name) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Missing required name attribute in portgroup")); - goto error; + goto cleanup; } isDefault = virXPathString("string(./@default)", ctxt); @@ -1246,21 +1246,21 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def, virtPortNode = virXPathNode("./virtualport", ctxt); if (virtPortNode && (!(def->virtPortProfile = virNetDevVPortProfileParse(virtPortNode, 0)))) { - goto error; + goto cleanup; } bandwidth_node = virXPathNode("./bandwidth", ctxt); if (bandwidth_node && !(def->bandwidth = virNetDevBandwidthParse(bandwidth_node))) { - goto error; + goto cleanup; } vlanNode = virXPathNode("./vlan", ctxt); if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0) - goto error; + goto cleanup; result = 0; -error: +cleanup: if (result < 0) { virPortGroupDefClear(def); } @@ -1428,8 +1428,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } /* parse each addr */ for (ii = 0; ii < nIps; ii++) { - int ret = virNetworkIPParseXML(def->name, &def->ips[ii], - ipNodes[ii], ctxt); + int ret = virNetworkIPDefParseXML(def->name, ipNodes[ii], + ctxt, &def->ips[ii]); if (ret < 0) goto error; def->nips++; @@ -2498,7 +2498,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, /* parse the xml into a virNetworkDHCPHostDef */ if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) { - if (virNetworkDHCPHostDefParse(def->name, ctxt->node, &host, false) < 0) + if (virNetworkDHCPHostDefParseXML(def->name, ctxt->node, &host, false) < 0) goto cleanup; /* search for the entry with this (mac|name), @@ -2531,7 +2531,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, } else if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) || (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) { - if (virNetworkDHCPHostDefParse(def->name, ctxt->node, &host, true) < 0) + if (virNetworkDHCPHostDefParseXML(def->name, ctxt->node, &host, true) < 0) goto cleanup; /* log error if an entry with same name/address/ip already exists */ @@ -2565,7 +2565,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, } } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { - if (virNetworkDHCPHostDefParse(def->name, ctxt->node, &host, false) < 0) + if (virNetworkDHCPHostDefParseXML(def->name, ctxt->node, &host, false) < 0) goto cleanup; /* find matching entry - all specified attributes must match */ @@ -2631,7 +2631,7 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, goto cleanup; } - if (virNetworkDHCPRangeDefParse(def->name, ctxt->node, &range) < 0) + if (virNetworkDHCPRangeDefParseXML(def->name, ctxt->node, &range) < 0) goto cleanup; /* check if an entry with same name/address/ip already exists */ -- 1.7.11.7
The other clear functions in network_conf.c that clear out arrays of sub-objects do so by using the n[itemname]s value as a counter going down to 0. Make this one consistent. There's no functional value, just makes the style more consistent with the rest of the file. --- src/conf/network_conf.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 35a4318..1bdf33a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -118,15 +118,14 @@ virNetworkDHCPHostDefClear(virNetworkDHCPHostDefPtr def) VIR_FREE(def->name); } -static void virNetworkIpDefClear(virNetworkIpDefPtr def) +static void +virNetworkIpDefClear(virNetworkIpDefPtr def) { - int ii; - VIR_FREE(def->family); VIR_FREE(def->ranges); - for (ii = 0 ; ii < def->nhosts && def->hosts ; ii++) - virNetworkDHCPHostDefClear(&def->hosts[ii]); + while (def->nhosts--) + virNetworkDHCPHostDefClear(&def->hosts[def->nhosts]); VIR_FREE(def->hosts); VIR_FREE(def->tftproot); -- 1.7.11.7
The attributes of a <network> element's <forward> element were previously stored directly in the virNetworkDef object, but virNetworkUpdateForward() needs to operate on a <forward> in isolation, so this patchs pulls out all those attributes into a separate virNetworkForwardDef struct (and shortens their names appropriately). This new object is contained in the virNetworkDef, not pointed to by it, so there is no extra memory management. This patch makes no functional changes, it only changes, e.g., "nForwardIfs" to "forward.nifs". --- src/conf/network_conf.c | 148 ++++++++++++++++----------------- src/conf/network_conf.h | 34 ++++---- src/esx/esx_network_driver.c | 30 +++---- src/network/bridge_driver.c | 192 +++++++++++++++++++++---------------------- src/vbox/vbox_tmpl.c | 4 +- 5 files changed, 207 insertions(+), 201 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 1bdf33a..64b0557 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -187,15 +187,15 @@ void virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def->bridge); VIR_FREE(def->domain); - for (ii = 0 ; ii < def->nForwardPfs && def->forwardPfs ; ii++) { - virNetworkForwardPfDefClear(&def->forwardPfs[ii]); + for (ii = 0 ; ii < def->forward.npfs && def->forward.pfs ; ii++) { + virNetworkForwardPfDefClear(&def->forward.pfs[ii]); } - VIR_FREE(def->forwardPfs); + VIR_FREE(def->forward.pfs); - for (ii = 0 ; ii < def->nForwardIfs && def->forwardIfs ; ii++) { - virNetworkForwardIfDefClear(&def->forwardIfs[ii]); + for (ii = 0 ; ii < def->forward.nifs && def->forward.ifs ; ii++) { + virNetworkForwardIfDefClear(&def->forward.ifs[ii]); } - VIR_FREE(def->forwardIfs); + VIR_FREE(def->forward.ifs); for (ii = 0 ; ii < def->nips && def->ips ; ii++) { virNetworkIpDefClear(&def->ips[ii]); @@ -1438,13 +1438,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) forwardNode = virXPathNode("./forward", ctxt); if (!forwardNode) { - def->forwardType = VIR_NETWORK_FORWARD_NONE; + def->forward.type = VIR_NETWORK_FORWARD_NONE; def->stp = (stp && STREQ(stp, "off")) ? 0 : 1; } else { ctxt->node = forwardNode; tmp = virXPathString("string(./@mode)", ctxt); if (tmp) { - if ((def->forwardType = virNetworkForwardTypeFromString(tmp)) < 0) { + if ((def->forward.type = virNetworkForwardTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown forwarding type '%s'"), tmp); VIR_FREE(tmp); @@ -1452,14 +1452,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } VIR_FREE(tmp); } else { - def->forwardType = VIR_NETWORK_FORWARD_NAT; + def->forward.type = VIR_NETWORK_FORWARD_NAT; } forwardDev = virXPathString("string(./@dev)", ctxt); forwardManaged = virXPathString("string(./@managed)", ctxt); if (forwardManaged != NULL) { if (STRCASEEQ(forwardManaged, "yes")) - def->managed = 1; + def->forward.managed = true; } /* all of these modes can use a pool of physical interfaces */ @@ -1486,7 +1486,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } if (nForwardPfs == 1) { - if (VIR_ALLOC_N(def->forwardPfs, nForwardPfs) < 0) { + if (VIR_ALLOC_N(def->forward.pfs, nForwardPfs) < 0) { virReportOOMError(); goto error; } @@ -1505,9 +1505,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } - def->forwardPfs->dev = forwardDev; + def->forward.pfs->dev = forwardDev; forwardDev = NULL; - def->nForwardPfs++; + def->forward.npfs++; } else if (nForwardPfs > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Use of more than one physical interface is not allowed")); @@ -1516,7 +1516,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (nForwardAddrs > 0) { int ii; - if (VIR_ALLOC_N(def->forwardIfs, nForwardAddrs) < 0) { + if (VIR_ALLOC_N(def->forward.ifs, nForwardAddrs) < 0) { virReportOOMError(); goto error; } @@ -1531,7 +1531,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) type = virXMLPropString(forwardAddrNodes[ii], "type"); if (type) { - if ((def->forwardIfs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) { + if ((def->forward.ifs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) { virReportError(VIR_ERR_XML_ERROR, _("unknown address type '%s'"), type); goto error; @@ -1542,9 +1542,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } - switch (def->forwardIfs[ii].type) { + switch (def->forward.ifs[ii].type) { case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: - if (virDevicePCIAddressParseXML(forwardAddrNodes[ii], &(def->forwardIfs[ii].device.pci)) < 0) + if (virDevicePCIAddressParseXML(forwardAddrNodes[ii], &(def->forward.ifs[ii].device.pci)) < 0) goto error; break; @@ -1556,23 +1556,23 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } VIR_FREE(type); - def->nForwardIfs++; + def->forward.nifs++; } } else if (nForwardIfs > 0 || forwardDev) { int ii; /* allocate array to hold all the portgroups */ - if (VIR_ALLOC_N(def->forwardIfs, MAX(nForwardIfs, 1)) < 0) { + if (VIR_ALLOC_N(def->forward.ifs, MAX(nForwardIfs, 1)) < 0) { virReportOOMError(); goto error; } if (forwardDev) { - def->forwardIfs[0].device.dev = forwardDev; - def->forwardIfs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + def->forward.ifs[0].device.dev = forwardDev; + def->forward.ifs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; forwardDev = NULL; - def->nForwardIfs++; + def->forward.nifs++; } /* parse each forwardIf */ @@ -1585,13 +1585,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) goto error; } - if ((ii == 0) && (def->nForwardIfs == 1)) { + if ((ii == 0) && (def->forward.nifs == 1)) { /* both forwardDev and an interface element are present. * If they don't match, it's an error. */ - if (STRNEQ(forwardDev, def->forwardIfs[0].device.dev)) { + if (STRNEQ(forwardDev, def->forward.ifs[0].device.dev)) { virReportError(VIR_ERR_XML_ERROR, _("forward dev '%s' must match first interface element dev '%s' in network '%s'"), - def->forwardIfs[0].device.dev, + def->forward.ifs[0].device.dev, forwardDev, def->name); goto error; } @@ -1599,10 +1599,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) continue; } - def->forwardIfs[ii].device.dev = forwardDev; + def->forward.ifs[ii].device.dev = forwardDev; forwardDev = NULL; - def->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; - def->nForwardIfs++; + def->forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + def->forward.nifs++; } } VIR_FREE(type); @@ -1611,7 +1611,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(forwardPfNodes); VIR_FREE(forwardIfNodes); VIR_FREE(forwardAddrNodes); - switch (def->forwardType) { + switch (def->forward.type) { case VIR_NETWORK_FORWARD_ROUTE: case VIR_NETWORK_FORWARD_NAT: /* It's pointless to specify L3 forwarding without specifying @@ -1620,11 +1620,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (def->nips == 0) { virReportError(VIR_ERR_XML_ERROR, _("%s forwarding requested, but no IP address provided for network '%s'"), - virNetworkForwardTypeToString(def->forwardType), + virNetworkForwardTypeToString(def->forward.type), def->name); goto error; } - if (def->nForwardIfs > 1) { + if (def->forward.nifs > 1) { virReportError(VIR_ERR_XML_ERROR, _("multiple forwarding interfaces specified for network '%s', only one is supported"), def->name); @@ -1639,7 +1639,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (def->bridge) { virReportError(VIR_ERR_XML_ERROR, _("bridge name not allowed in %s mode (network '%s')"), - virNetworkForwardTypeToString(def->forwardType), + virNetworkForwardTypeToString(def->forward.type), def->name); goto error; } @@ -1648,16 +1648,16 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) if (def->delay || stp) { virReportError(VIR_ERR_XML_ERROR, _("bridge delay/stp options only allowed in route, nat, and isolated mode, not in %s (network '%s')"), - virNetworkForwardTypeToString(def->forwardType), + virNetworkForwardTypeToString(def->forward.type), def->name); goto error; } - if (def->bridge && (def->nForwardIfs || nForwardPfs)) { + if (def->bridge && (def->forward.nifs || def->forward.npfs)) { virReportError(VIR_ERR_XML_ERROR, _("A network with forward mode='%s' can specify " "a bridge name or a forward dev, but not " "both (network '%s')"), - virNetworkForwardTypeToString(def->forwardType), + virNetworkForwardTypeToString(def->forward.type), def->name); goto error; } @@ -1934,53 +1934,53 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) virUUIDFormat(uuid, uuidstr); virBufferAsprintf(&buf, "<uuid>%s</uuid>\n", uuidstr); - if (def->forwardType != VIR_NETWORK_FORWARD_NONE) { + if (def->forward.type != VIR_NETWORK_FORWARD_NONE) { const char *dev = NULL; - if (!def->nForwardPfs) + if (!def->forward.npfs) dev = virNetworkDefForwardIf(def, 0); - const char *mode = virNetworkForwardTypeToString(def->forwardType); + const char *mode = virNetworkForwardTypeToString(def->forward.type); if (!mode) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown forward type %d in network '%s'"), - def->forwardType, def->name); + def->forward.type, def->name); goto error; } virBufferAddLit(&buf, "<forward"); virBufferEscapeString(&buf, " dev='%s'", dev); virBufferAsprintf(&buf, " mode='%s'", mode); - if (def->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { - if (def->managed == 1) + if (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) { + if (def->forward.managed) virBufferAddLit(&buf, " managed='yes'"); else virBufferAddLit(&buf, " managed='no'"); } virBufferAsprintf(&buf, "%s>\n", - (def->nForwardIfs || def->nForwardPfs) ? "" : "/"); + (def->forward.nifs || def->forward.npfs) ? "" : "/"); virBufferAdjustIndent(&buf, 2); - /* For now, hard-coded to at most 1 forwardPfs */ - if (def->nForwardPfs) + /* For now, hard-coded to at most 1 forward.pfs */ + if (def->forward.npfs) virBufferEscapeString(&buf, "<pf dev='%s'/>\n", - def->forwardPfs[0].dev); + def->forward.pfs[0].dev); - if (def->nForwardIfs && - (!def->nForwardPfs || !(flags & VIR_NETWORK_XML_INACTIVE))) { - for (ii = 0; ii < def->nForwardIfs; ii++) { - if (def->forwardType != VIR_NETWORK_FORWARD_HOSTDEV) { + if (def->forward.nifs && + (!def->forward.npfs || !(flags & VIR_NETWORK_XML_INACTIVE))) { + for (ii = 0; ii < def->forward.nifs; ii++) { + if (def->forward.type != VIR_NETWORK_FORWARD_HOSTDEV) { virBufferEscapeString(&buf, "<interface dev='%s'", - def->forwardIfs[ii].device.dev); + def->forward.ifs[ii].device.dev); if (!(flags & VIR_NETWORK_XML_INACTIVE) && - (def->forwardIfs[ii].connections > 0)) { + (def->forward.ifs[ii].connections > 0)) { virBufferAsprintf(&buf, " connections='%d'", - def->forwardIfs[ii].connections); + def->forward.ifs[ii].connections); } virBufferAddLit(&buf, "/>\n"); } else { - if (def->forwardIfs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI) { + if (def->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI) { if (virDevicePCIAddressFormat(&buf, - def->forwardIfs[ii].device.pci, + def->forward.ifs[ii].device.pci, true) < 0) goto error; } @@ -1988,13 +1988,13 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) } } virBufferAdjustIndent(&buf, -2); - if (def->nForwardPfs || def->nForwardIfs) + if (def->forward.npfs || def->forward.nifs) virBufferAddLit(&buf, "</forward>\n"); } - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || - def->forwardType == VIR_NETWORK_FORWARD_NAT || - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + if (def->forward.type == VIR_NETWORK_FORWARD_NONE || + def->forward.type == VIR_NETWORK_FORWARD_NAT || + def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { virBufferAddLit(&buf, "<bridge"); if (def->bridge) @@ -2002,7 +2002,7 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags) virBufferAsprintf(&buf, " stp='%s' delay='%ld' />\n", def->stp ? "on" : "off", def->delay); - } else if (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && + } else if (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && def->bridge) { virBufferEscapeString(&buf, "<bridge name='%s' />\n", def->bridge); } @@ -2162,9 +2162,9 @@ virNetworkObjPtr virNetworkLoadConfig(virNetworkObjListPtr nets, goto error; } - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || - def->forwardType == VIR_NETWORK_FORWARD_NAT || - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + if (def->forward.type == VIR_NETWORK_FORWARD_NONE || + def->forward.type == VIR_NETWORK_FORWARD_NAT || + def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { /* Generate a bridge if none is specified, but don't check for collisions * if a bridge is hardcoded, so the network is at least defined. @@ -2734,17 +2734,17 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, } /* check if an <interface> with same dev name already exists */ - for (ii = 0; ii < def->nForwardIfs; ii++) { - if (def->forwardIfs[ii].type + for (ii = 0; ii < def->forward.nifs; ii++) { + if (def->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV && - STREQ(iface.device.dev, def->forwardIfs[ii].device.dev)) + STREQ(iface.device.dev, def->forward.ifs[ii].device.dev)) break; } if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) || (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) { - if (ii < def->nForwardIfs) { + if (ii < def->forward.nifs) { virReportError(VIR_ERR_OPERATION_INVALID, _("there is an existing interface entry " "in network '%s' that matches " @@ -2754,17 +2754,17 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, } /* add to beginning/end of list */ - if (VIR_INSERT_ELEMENT(def->forwardIfs, + if (VIR_INSERT_ELEMENT(def->forward.ifs, command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST - ? 0 : def->nForwardIfs, - def->nForwardIfs, &iface) < 0) { + ? 0 : def->forward.nifs, + def->forward.nifs, &iface) < 0) { virReportOOMError(); goto cleanup; } } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) { - if (ii == def->nForwardIfs) { + if (ii == def->forward.nifs) { virReportError(VIR_ERR_OPERATION_INVALID, _("couldn't find an interface entry " "in network '%s' matching <interface dev='%s'>"), @@ -2773,19 +2773,19 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def, } /* fail if the interface is being used */ - if (def->forwardIfs[ii].connections > 0) { + if (def->forward.ifs[ii].connections > 0) { virReportError(VIR_ERR_OPERATION_INVALID, _("unable to delete interface '%s' " "in network '%s'. It is currently being used " " by %d domains."), iface.device.dev, def->name, - def->forwardIfs[ii].connections); + def->forward.ifs[ii].connections); goto cleanup; } /* remove it */ - virNetworkForwardIfDefClear(&def->forwardIfs[ii]); - VIR_DELETE_ELEMENT(def->forwardIfs, ii, def->nForwardIfs); + virNetworkForwardIfDefClear(&def->forward.ifs[ii]); + VIR_DELETE_ELEMENT(def->forward.ifs, ii, def->forward.nifs); } else { virNetworkDefUpdateUnknownCommand(command); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index c0d4fa2..e1d666b 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -161,6 +161,22 @@ struct _virNetworkForwardPfDef { int connections; /* how many guest interfaces are connected to this device? */ }; +typedef struct _virNetworkForwardDef virNetworkForwardDef; +typedef virNetworkForwardDef *virNetworkForwardDefPtr; +struct _virNetworkForwardDef { + int type; /* One of virNetworkForwardType constants */ + bool managed; /* managed attribute for hostdev mode */ + + /* If there are multiple forward devices (i.e. a pool of + * interfaces), they will be listed here. + */ + size_t npfs; + virNetworkForwardPfDefPtr pfs; + + size_t nifs; + virNetworkForwardIfDefPtr ifs; +}; + typedef struct _virPortGroupDef virPortGroupDef; typedef virPortGroupDef *virPortGroupDefPtr; struct _virPortGroupDef { @@ -191,17 +207,7 @@ struct _virNetworkDef { */ bool ipv6nogw; - int forwardType; /* One of virNetworkForwardType constants */ - int managed; /* managed attribute for hostdev mode */ - - /* If there are multiple forward devices (i.e. a pool of - * interfaces), they will be listed here. - */ - size_t nForwardPfs; - virNetworkForwardPfDefPtr forwardPfs; - - size_t nForwardIfs; - virNetworkForwardIfDefPtr forwardIfs; + virNetworkForwardDef forward; size_t nips; virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */ @@ -280,9 +286,9 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags); static inline const char * virNetworkDefForwardIf(const virNetworkDefPtr def, size_t n) { - return ((def->forwardIfs && (def->nForwardIfs > n) && - def->forwardIfs[n].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) - ? def->forwardIfs[n].device.dev : NULL); + return ((def->forward.ifs && (def->forward.nifs > n) && + def->forward.ifs[n].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) + ? def->forward.ifs[n].device.dev : NULL); } virPortGroupDefPtr virPortGroupFindByName(virNetworkDefPtr net, diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index c8f101a..6a87abd 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -372,11 +372,11 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) } /* FIXME: Add support for NAT */ - if (def->forwardType != VIR_NETWORK_FORWARD_NONE && - def->forwardType != VIR_NETWORK_FORWARD_BRIDGE) { + if (def->forward.type != VIR_NETWORK_FORWARD_NONE && + def->forward.type != VIR_NETWORK_FORWARD_BRIDGE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported forward mode '%s'"), - virNetworkForwardTypeToString(def->forwardType)); + virNetworkForwardTypeToString(def->forward.type)); goto cleanup; } @@ -405,7 +405,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - if (def->forwardType != VIR_NETWORK_FORWARD_NONE && def->nForwardIfs > 0) { + if (def->forward.type != VIR_NETWORK_FORWARD_NONE && def->forward.nifs > 0) { if (esxVI_HostVirtualSwitchBondBridge_Alloc (&hostVirtualSwitchBondBridge) < 0) { goto cleanup; @@ -419,10 +419,10 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - for (i = 0; i < def->nForwardIfs; ++i) { + for (i = 0; i < def->forward.nifs; ++i) { bool found = false; - if (def->forwardIfs[i].type != + if (def->forward.ifs[i].type != VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unsupported device type in network %s " @@ -433,7 +433,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) for (physicalNic = physicalNicList; physicalNic != NULL; physicalNic = physicalNic->_next) { - if (STREQ(def->forwardIfs[i].device.dev, physicalNic->device)) { + if (STREQ(def->forward.ifs[i].device.dev, physicalNic->device)) { if (esxVI_String_AppendValueToList (&hostVirtualSwitchBondBridge->nicDevice, physicalNic->key) < 0) { @@ -448,7 +448,7 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) if (! found) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not find PhysicalNic with name '%s'"), - def->forwardIfs[i].device.dev); + def->forward.ifs[i].device.dev); goto cleanup; } } @@ -721,7 +721,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) goto cleanup; } - def->forwardType = VIR_NETWORK_FORWARD_NONE; + def->forward.type = VIR_NETWORK_FORWARD_NONE; /* Count PhysicalNics on HostVirtualSwitch */ count = 0; @@ -732,9 +732,9 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) } if (count > 0) { - def->forwardType = VIR_NETWORK_FORWARD_BRIDGE; + def->forward.type = VIR_NETWORK_FORWARD_BRIDGE; - if (VIR_ALLOC_N(def->forwardIfs, count) < 0) { + if (VIR_ALLOC_N(def->forward.ifs, count) < 0) { virReportOOMError(); goto cleanup; } @@ -751,17 +751,17 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) for (physicalNic = physicalNicList; physicalNic != NULL; physicalNic = physicalNic->_next) { if (STREQ(physicalNicKey->value, physicalNic->key)) { - def->forwardIfs[def->nForwardIfs].type + def->forward.ifs[def->forward.nifs].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; - def->forwardIfs[def->nForwardIfs].device.dev + def->forward.ifs[def->forward.nifs].device.dev = strdup(physicalNic->device); - if (def->forwardIfs[def->nForwardIfs].device.dev == NULL) { + if (def->forward.ifs[def->forward.nifs].device.dev == NULL) { virReportOOMError(); goto cleanup; } - ++def->nForwardIfs; + ++def->forward.nifs; found = true; break; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a5b9182..c94a9a1 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -711,7 +711,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, * host's /etc/resolv.conf (since this could be used as a channel * to build a connection to the outside). */ - if (network->def->forwardType == VIR_NETWORK_FORWARD_NONE) { + if (network->def->forward.type == VIR_NETWORK_FORWARD_NONE) { virCommandAddArgList(cmd, "--dhcp-option=3", "--no-resolv", NULL); } @@ -1325,9 +1325,9 @@ networkRefreshDaemons(struct network_driver *driver) virNetworkObjLock(network); if (virNetworkObjIsActive(network) && - ((network->def->forwardType == VIR_NETWORK_FORWARD_NONE) || - (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) || - (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE))) { + ((network->def->forward.type == VIR_NETWORK_FORWARD_NONE) || + (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) || + (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { /* Only the three L3 network types that are configured by * libvirt will have a dnsmasq or radvd daemon associated * with them. Here we send a SIGHUP to an existing @@ -1862,12 +1862,12 @@ networkAddIpSpecificIptablesRules(struct network_driver *driver, * forward mode is NAT. This is because IPv6 has no NAT. */ - if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { + if (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) { if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) return networkAddMasqueradingIptablesRules(driver, network, ipdef); else if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) return networkAddRoutingIptablesRules(driver, network, ipdef); - } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + } else if (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { return networkAddRoutingIptablesRules(driver, network, ipdef); } return 0; @@ -1878,12 +1878,12 @@ networkRemoveIpSpecificIptablesRules(struct network_driver *driver, virNetworkObjPtr network, virNetworkIpDefPtr ipdef) { - if (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) { + if (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) { if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) networkRemoveMasqueradingIptablesRules(driver, network, ipdef); else if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) networkRemoveRoutingIptablesRules(driver, network, ipdef); - } else if (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + } else if (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { networkRemoveRoutingIptablesRules(driver, network, ipdef); } } @@ -1951,9 +1951,9 @@ networkReloadIptablesRules(struct network_driver *driver) virNetworkObjLock(network); if (virNetworkObjIsActive(network) && - ((network->def->forwardType == VIR_NETWORK_FORWARD_NONE) || - (network->def->forwardType == VIR_NETWORK_FORWARD_NAT) || - (network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE))) { + ((network->def->forward.type == VIR_NETWORK_FORWARD_NONE) || + (network->def->forward.type == VIR_NETWORK_FORWARD_NAT) || + (network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) { /* Only the three L3 network types that are configured by libvirt * need to have iptables rules reloaded. */ @@ -2253,8 +2253,8 @@ networkStartNetworkVirtual(struct network_driver *driver, if (virNetDevSetOnline(network->def->bridge, 1) < 0) goto err2; - /* If forwardType != NONE, turn on global IP forwarding */ - if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && + /* If forward.type != NONE, turn on global IP forwarding */ + if (network->def->forward.type != VIR_NETWORK_FORWARD_NONE && networkEnableIpForwarding(v4present, v6present) < 0) { virReportSystemError(errno, "%s", _("failed to enable IP forwarding")); @@ -2424,7 +2424,7 @@ networkStartNetwork(struct network_driver *driver, if (virNetworkObjSetDefTransient(network, true) < 0) return -1; - switch (network->def->forwardType) { + switch (network->def->forward.type) { case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: @@ -2486,7 +2486,7 @@ static int networkShutdownNetwork(struct network_driver *driver, unlink(stateFile); VIR_FREE(stateFile); - switch (network->def->forwardType) { + switch (network->def->forward.type) { case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: @@ -2739,9 +2739,9 @@ networkValidate(struct network_driver *driver, /* Only the three L3 network types that are configured by libvirt * need to have a bridge device name / mac address provided */ - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || - def->forwardType == VIR_NETWORK_FORWARD_NAT || - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { + if (def->forward.type == VIR_NETWORK_FORWARD_NONE || + def->forward.type == VIR_NETWORK_FORWARD_NAT || + def->forward.type == VIR_NETWORK_FORWARD_ROUTE) { if (virNetworkSetBridgeName(&driver->networks, def, 1)) return -1; @@ -2756,7 +2756,7 @@ networkValidate(struct network_driver *driver, _("Unsupported <ip> element in network %s " "with forward mode='%s'"), def->name, - virNetworkForwardTypeToString(def->forwardType)); + virNetworkForwardTypeToString(def->forward.type)); return -1; } if (def->dns.ntxts || def->dns.nhosts || def->dns.nsrvs) { @@ -2764,7 +2764,7 @@ networkValidate(struct network_driver *driver, _("Unsupported <dns> element in network %s " "with forward mode='%s'"), def->name, - virNetworkForwardTypeToString(def->forwardType)); + virNetworkForwardTypeToString(def->forward.type)); return -1; } if (def->domain) { @@ -2772,7 +2772,7 @@ networkValidate(struct network_driver *driver, _("Unsupported <domain> element in network %s " "with forward mode='%s'"), def->name, - virNetworkForwardTypeToString(def->forwardType)); + virNetworkForwardTypeToString(def->forward.type)); return -1; } } @@ -2797,7 +2797,7 @@ networkValidate(struct network_driver *driver, * a pool, and those using an Open vSwitch bridge. */ - vlanAllowed = (def->forwardType == VIR_NETWORK_FORWARD_BRIDGE && + vlanAllowed = (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && def->virtPortProfile && def->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH); @@ -2810,7 +2810,7 @@ networkValidate(struct network_driver *driver, * supports a vlan tag. */ if (def->portGroups[ii].virtPortProfile) { - if (def->forwardType != VIR_NETWORK_FORWARD_BRIDGE || + if (def->forward.type != VIR_NETWORK_FORWARD_BRIDGE || def->portGroups[ii].virtPortProfile->virtPortType != VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { badVlanUse = true; @@ -3105,9 +3105,9 @@ networkUpdate(virNetworkPtr net, if ((section == VIR_NETWORK_SECTION_IP || section == VIR_NETWORK_SECTION_FORWARD || section == VIR_NETWORK_SECTION_FORWARD_INTERFACE) && - (network->def->forwardType == VIR_NETWORK_FORWARD_NONE || - network->def->forwardType == VIR_NETWORK_FORWARD_NAT || - network->def->forwardType == VIR_NETWORK_FORWARD_ROUTE)) { + (network->def->forward.type == VIR_NETWORK_FORWARD_NONE || + network->def->forward.type == VIR_NETWORK_FORWARD_NAT || + network->def->forward.type == VIR_NETWORK_FORWARD_ROUTE)) { /* these could affect the iptables rules */ networkRemoveIptablesRules(driver, network); if (networkAddIptablesRules(driver, network) < 0) @@ -3405,37 +3405,37 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { struct pci_config_address **virt_fns; int ret = -1, ii = 0; - if ((virNetDevGetVirtualFunctions(netdef->forwardPfs->dev, + if ((virNetDevGetVirtualFunctions(netdef->forward.pfs->dev, &vfname, &virt_fns, &num_virt_fns)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), - netdef->forwardPfs->dev); + netdef->forward.pfs->dev); goto finish; } if (num_virt_fns == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("No Vf's present on SRIOV PF %s"), - netdef->forwardPfs->dev); + netdef->forward.pfs->dev); goto finish; } - if ((VIR_ALLOC_N(netdef->forwardIfs, num_virt_fns)) < 0) { + if ((VIR_ALLOC_N(netdef->forward.ifs, num_virt_fns)) < 0) { virReportOOMError(); goto finish; } - netdef->nForwardIfs = num_virt_fns; + netdef->forward.nifs = num_virt_fns; - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || - (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || - (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || - (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) { - netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) || + (netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) || + (netdef->forward.type == VIR_NETWORK_FORWARD_VEPA) || + (netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) { + netdef->forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; if (vfname[ii]) { - netdef->forwardIfs[ii].device.dev = strdup(vfname[ii]); - if (!netdef->forwardIfs[ii].device.dev) { + netdef->forward.ifs[ii].device.dev = strdup(vfname[ii]); + if (!netdef->forward.ifs[ii].device.dev) { virReportOOMError(); goto finish; } @@ -3446,13 +3446,13 @@ networkCreateInterfacePool(virNetworkDefPtr netdef) { goto finish; } } - else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + else if (netdef->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) { /* VF's are always PCI devices */ - netdef->forwardIfs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; - netdef->forwardIfs[ii].device.pci.domain = virt_fns[ii]->domain; - netdef->forwardIfs[ii].device.pci.bus = virt_fns[ii]->bus; - netdef->forwardIfs[ii].device.pci.slot = virt_fns[ii]->slot; - netdef->forwardIfs[ii].device.pci.function = virt_fns[ii]->function; + netdef->forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI; + netdef->forward.ifs[ii].device.pci.domain = virt_fns[ii]->domain; + netdef->forward.ifs[ii].device.pci.bus = virt_fns[ii]->bus; + netdef->forward.ifs[ii].device.pci.slot = virt_fns[ii]->slot; + netdef->forward.ifs[ii].device.pci.function = virt_fns[ii]->function; } } @@ -3535,16 +3535,16 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) goto error; } - if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE) || - (netdef->forwardType == VIR_NETWORK_FORWARD_NAT) || - (netdef->forwardType == VIR_NETWORK_FORWARD_ROUTE)) { + if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) || + (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) || + (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE)) { /* for these forward types, the actual net type really *is* *NETWORK; we just keep the info from the portgroup in * iface->data.network.actual */ if (iface->data.network.actual) iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; - } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) && + } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) && netdef->bridge) { /* <forward type='bridge'/> <bridge name='xxx'/> @@ -3587,7 +3587,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } } - } else if (netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + } else if (netdef->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) { if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { @@ -3596,15 +3596,15 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_HOSTDEV; - if (netdef->nForwardPfs > 0 && netdef->nForwardIfs <= 0 && + if (netdef->forward.npfs > 0 && netdef->forward.nifs <= 0 && networkCreateInterfacePool(netdef) < 0) { goto error; } /* pick first dev with 0 connections */ - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].connections == 0) { - dev = &netdef->forwardIfs[ii]; + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].connections == 0) { + dev = &netdef->forward.ifs[ii]; break; } } @@ -3619,7 +3619,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) iface->data.network.actual->data.hostdev.def.parent.data.net = iface; iface->data.network.actual->data.hostdev.def.info = &iface->info; iface->data.network.actual->data.hostdev.def.mode = VIR_DOMAIN_HOSTDEV_MODE_SUBSYS; - iface->data.network.actual->data.hostdev.def.managed = netdef->managed; + iface->data.network.actual->data.hostdev.def.managed = netdef->forward.managed ? 1 : 0; iface->data.network.actual->data.hostdev.def.source.subsys.type = dev->type; iface->data.network.actual->data.hostdev.def.source.subsys.u.pci = dev->device.pci; @@ -3648,10 +3648,10 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } } - } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) || - (netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) || - (netdef->forwardType == VIR_NETWORK_FORWARD_VEPA) || - (netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH)) { + } else if ((netdef->forward.type == VIR_NETWORK_FORWARD_BRIDGE) || + (netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) || + (netdef->forward.type == VIR_NETWORK_FORWARD_VEPA) || + (netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH)) { /* <forward type='bridge|private|vepa|passthrough'> are all * VIR_DOMAIN_NET_TYPE_DIRECT. @@ -3665,7 +3665,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) /* Set type=direct and appropriate <source mode='xxx'/> */ iface->data.network.actual->type = actualType = VIR_DOMAIN_NET_TYPE_DIRECT; - switch (netdef->forwardType) { + switch (netdef->forward.type) { case VIR_NETWORK_FORWARD_BRIDGE: iface->data.network.actual->data.direct.mode = VIR_NETDEV_MACVLAN_MODE_BRIDGE; break; @@ -3707,7 +3707,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) /* If there is only a single device, just return it (caller will detect * any error if exclusive use is required but could not be acquired). */ - if ((netdef->nForwardIfs <= 0) && (netdef->nForwardPfs <= 0)) { + if ((netdef->forward.nifs <= 0) && (netdef->forward.npfs <= 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' uses a direct mode, but " "has no forward dev and no interface pool"), @@ -3716,7 +3716,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } else { /* pick an interface from the pool */ - if (netdef->nForwardPfs > 0 && netdef->nForwardIfs == 0 && + if (netdef->forward.npfs > 0 && netdef->forward.nifs == 0 && networkCreateInterfacePool(netdef) < 0) { goto error; } @@ -3727,25 +3727,25 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) * just search for the one with the lowest number of * connections. */ - if ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || - ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && + if ((netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH) || + ((netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) && iface->data.network.actual->virtPortProfile && (iface->data.network.actual->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH))) { /* pick first dev with 0 connections */ - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].connections == 0) { - dev = &netdef->forwardIfs[ii]; + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].connections == 0) { + dev = &netdef->forward.ifs[ii]; break; } } } else { /* pick least used dev */ - dev = &netdef->forwardIfs[0]; - for (ii = 1; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].connections < dev->connections) - dev = &netdef->forwardIfs[ii]; + dev = &netdef->forward.ifs[0]; + for (ii = 1; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].connections < dev->connections) + dev = &netdef->forward.ifs[ii]; } } /* dev points at the physical device we want to use */ @@ -3885,11 +3885,11 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) goto success; } - if (netdef->nForwardPfs > 0 && netdef->nForwardIfs == 0 && + if (netdef->forward.npfs > 0 && netdef->forward.nifs == 0 && networkCreateInterfacePool(netdef) < 0) { goto error; } - if (netdef->nForwardIfs == 0) { + if (netdef->forward.nifs == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' uses a direct or hostdev mode, " "but has no forward dev and no interface pool"), @@ -3909,11 +3909,11 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } /* find the matching interface and increment its connections */ - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].type + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV && - STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; + STREQ(actualDev, netdef->forward.ifs[ii].device.dev)) { + dev = &netdef->forward.ifs[ii]; break; } } @@ -3931,8 +3931,8 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) * must be 0 in those cases. */ if ((dev->connections > 0) && - ((netdef->forwardType == VIR_NETWORK_FORWARD_PASSTHROUGH) || - ((netdef->forwardType == VIR_NETWORK_FORWARD_PRIVATE) && + ((netdef->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH) || + ((netdef->forward.type == VIR_NETWORK_FORWARD_PRIVATE) && iface->data.network.actual->virtPortProfile && (iface->data.network.actual->virtPortProfile->virtPortType == VIR_NETDEV_VPORT_PROFILE_8021QBH)))) { @@ -3960,12 +3960,12 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) } /* find the matching interface and increment its connections */ - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].type + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && virDevicePCIAddressEqual(&hostdev->source.subsys.u.pci, - &netdef->forwardIfs[ii].device.pci)) { - dev = &netdef->forwardIfs[ii]; + &netdef->forward.ifs[ii].device.pci)) { + dev = &netdef->forward.ifs[ii]; break; } } @@ -3987,7 +3987,7 @@ networkNotifyActualDevice(virDomainNetDefPtr iface) * current connections count must be 0 in those cases. */ if ((dev->connections > 0) && - netdef->forwardType == VIR_NETWORK_FORWARD_HOSTDEV) { + netdef->forward.type == VIR_NETWORK_FORWARD_HOSTDEV) { virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' claims the PCI device at " "domain=%d bus=%d slot=%d function=%d " @@ -4062,7 +4062,7 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) goto success; } - if (netdef->nForwardIfs == 0) { + if (netdef->forward.nifs == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("network '%s' uses a direct/hostdev mode, but " "has no forward dev and no interface pool"), @@ -4081,11 +4081,11 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) goto error; } - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].type + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV && - STREQ(actualDev, netdef->forwardIfs[ii].device.dev)) { - dev = &netdef->forwardIfs[ii]; + STREQ(actualDev, netdef->forward.ifs[ii].device.dev)) { + dev = &netdef->forward.ifs[ii]; break; } } @@ -4112,12 +4112,12 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) goto error; } - for (ii = 0; ii < netdef->nForwardIfs; ii++) { - if (netdef->forwardIfs[ii].type + for (ii = 0; ii < netdef->forward.nifs; ii++) { + if (netdef->forward.ifs[ii].type == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI && virDevicePCIAddressEqual(&hostdev->source.subsys.u.pci, - &netdef->forwardIfs[ii].device.pci)) { - dev = &netdef->forwardIfs[ii]; + &netdef->forward.ifs[ii].device.pci)) { + dev = &netdef->forward.ifs[ii]; break; } } @@ -4202,7 +4202,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) } netdef = network->def; - switch (netdef->forwardType) { + switch (netdef->forward.type) { case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: @@ -4227,8 +4227,8 @@ networkGetNetworkAddress(const char *netname, char **netaddr) case VIR_NETWORK_FORWARD_PRIVATE: case VIR_NETWORK_FORWARD_VEPA: case VIR_NETWORK_FORWARD_PASSTHROUGH: - if ((netdef->nForwardIfs > 0) && netdef->forwardIfs) - dev_name = netdef->forwardIfs[0].device.dev; + if ((netdef->forward.nifs > 0) && netdef->forward.ifs) + dev_name = netdef->forward.ifs[0].device.dev; if (!dev_name) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index f9fa442..e0f9b6f 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -7725,7 +7725,7 @@ static virNetworkPtr vboxNetworkDefineCreateXML(virConnectPtr conn, const char * virSocketAddr netmask; if ((!def) || - (def->forwardType != VIR_NETWORK_FORWARD_NONE) || + (def->forward.type != VIR_NETWORK_FORWARD_NONE) || (def->nips == 0 || !def->ips)) goto cleanup; @@ -8129,7 +8129,7 @@ static char *vboxNetworkGetXMLDesc(virNetworkPtr network, VBOX_UTF8_TO_UTF16(networkNameUtf8 , &networkNameUtf16); - def->forwardType = VIR_NETWORK_FORWARD_NONE; + def->forward.type = VIR_NETWORK_FORWARD_NONE; data->vboxObj->vtbl->FindDHCPServerByNetworkName(data->vboxObj, networkNameUtf16, -- 1.7.11.7
virNetworkDefUpdateForward requires separate functions to parse and clear a virNetworkForwardDef by itself, but they were previously just inlined in the virNetworkDef parse and free functions. This patch makes them into separate functions. --- src/conf/network_conf.c | 515 ++++++++++++++++++++++++++---------------------- 1 file changed, 280 insertions(+), 235 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 64b0557..8142e98 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -176,7 +176,24 @@ virNetworkDNSDefClear(virNetworkDNSDefPtr def) } } -void virNetworkDefFree(virNetworkDefPtr def) +static void +virNetworkForwardDefClear(virNetworkForwardDefPtr def) +{ + int ii; + + for (ii = 0 ; ii < def->npfs && def->pfs ; ii++) { + virNetworkForwardPfDefClear(&def->pfs[ii]); + } + VIR_FREE(def->pfs); + + for (ii = 0 ; ii < def->nifs && def->ifs ; ii++) { + virNetworkForwardIfDefClear(&def->ifs[ii]); + } + VIR_FREE(def->ifs); +} + +void +virNetworkDefFree(virNetworkDefPtr def) { int ii; @@ -187,15 +204,7 @@ void virNetworkDefFree(virNetworkDefPtr def) VIR_FREE(def->bridge); VIR_FREE(def->domain); - for (ii = 0 ; ii < def->forward.npfs && def->forward.pfs ; ii++) { - virNetworkForwardPfDefClear(&def->forward.pfs[ii]); - } - VIR_FREE(def->forward.pfs); - - for (ii = 0 ; ii < def->forward.nifs && def->forward.ifs ; ii++) { - virNetworkForwardIfDefClear(&def->forward.ifs[ii]); - } - VIR_FREE(def->forward.ifs); + virNetworkForwardDefClear(&def->forward); for (ii = 0 ; ii < def->nips && def->ips ; ii++) { virNetworkIpDefClear(&def->ips[ii]); @@ -1269,6 +1278,211 @@ cleanup: return result; } +static int +virNetworkForwardDefParseXML(const char *networkName, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virNetworkForwardDefPtr def) +{ + int ii, ret = -1; + xmlNodePtr *forwardIfNodes = NULL; + xmlNodePtr *forwardPfNodes = NULL; + xmlNodePtr *forwardAddrNodes = NULL; + int nForwardIfs, nForwardAddrs, nForwardPfs; + char *forwardDev = NULL; + char *forwardManaged = NULL; + char *type = NULL; + xmlNodePtr save = ctxt->node; + + ctxt->node = node; + + if (!(type = virXPathString("string(./@mode)", ctxt))) { + def->type = VIR_NETWORK_FORWARD_NAT; + } else { + if ((def->type = virNetworkForwardTypeFromString(type)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown forwarding type '%s'"), type); + goto cleanup; + } + VIR_FREE(type); + } + + forwardManaged = virXPathString("string(./@managed)", ctxt); + if (forwardManaged != NULL && + STRCASEEQ(forwardManaged, "yes")) { + def->managed = true; + } + + /* bridge and hostdev modes can use a pool of physical interfaces */ + nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes); + if (nForwardIfs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <interface> element found in <forward> of network %s"), + networkName); + goto cleanup; + } + + nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes); + if (nForwardAddrs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <address> element found in <forward> of network %s"), + networkName); + goto cleanup; + } + + nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes); + if (nForwardPfs < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid <pf> element found in <forward> of network %s"), + networkName); + goto cleanup; + } + + if (((nForwardIfs > 0) + (nForwardAddrs > 0) + (nForwardPfs > 0)) > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("<address>, <interface>, and <pf> elements in <forward> " + "of network %s are mutually exclusive"), + networkName); + goto cleanup; + } + + forwardDev = virXPathString("string(./@dev)", ctxt); + if (forwardDev && (nForwardAddrs > 0 || nForwardPfs > 0)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("the <forward> 'dev' attribute cannot be used when " + "<address> or <pf> sub-elements are present " + "in network %s")); + goto cleanup; + } + + if (nForwardIfs > 0 || forwardDev) { + + if (VIR_ALLOC_N(def->ifs, MAX(nForwardIfs, 1)) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (forwardDev) { + def->ifs[0].device.dev = forwardDev; + def->ifs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + forwardDev = NULL; + def->nifs++; + } + + /* parse each <interface> */ + for (ii = 0; ii < nForwardIfs; ii++) { + forwardDev = virXMLPropString(forwardIfNodes[ii], "dev"); + if (!forwardDev) { + virReportError(VIR_ERR_XML_ERROR, + _("Missing required dev attribute in " + "<forward> <interface> element of network %s"), + networkName); + goto cleanup; + } + + if ((ii == 0) && (def->nifs == 1)) { + /* both <forward dev='x'> and <interface dev='x'/> are + * present. If they don't match, it's an error. + */ + if (STRNEQ(forwardDev, def->ifs[0].device.dev)) { + virReportError(VIR_ERR_XML_ERROR, + _("<forward dev='%s'> must match first " + "<interface dev='%s'/> in network %s"), + def->ifs[0].device.dev, + forwardDev, networkName); + goto cleanup; + } + VIR_FREE(forwardDev); + continue; + } + + def->ifs[ii].device.dev = forwardDev; + forwardDev = NULL; + def->ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; + def->nifs++; + } + + } else if (nForwardAddrs > 0) { + + if (VIR_ALLOC_N(def->ifs, nForwardAddrs) < 0) { + virReportOOMError(); + goto cleanup; + } + + for (ii = 0; ii < nForwardAddrs; ii++) { + if (!(type = virXMLPropString(forwardAddrNodes[ii], "type"))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing address type in network %s"), + networkName); + goto cleanup; + } + + if ((def->ifs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("unknown address type '%s' in network %s"), + type, networkName); + goto cleanup; + } + + switch (def->ifs[ii].type) { + case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: + if (virDevicePCIAddressParseXML(forwardAddrNodes[ii], + &def->ifs[ii].device.pci) < 0) { + goto cleanup; + } + break; + + /* Add USB case here if we ever find a reason to support it */ + + default: + virReportError(VIR_ERR_XML_ERROR, + _("unsupported address type '%s' in network %s"), + type, networkName); + goto cleanup; + } + VIR_FREE(type); + def->nifs++; + } + + } else if (nForwardPfs > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("Only one <pf> element is allowed in <forward> of network %s"), + networkName); + goto cleanup; + } else if (nForwardPfs == 1) { + + if (VIR_ALLOC_N(def->pfs, nForwardPfs) < 0) { + virReportOOMError(); + goto cleanup; + } + + forwardDev = virXMLPropString(*forwardPfNodes, "dev"); + if (!forwardDev) { + virReportError(VIR_ERR_XML_ERROR, + _("Missing required dev attribute " + "in <pf> element of network '%s'"), + networkName); + goto cleanup; + } + + def->pfs->dev = forwardDev; + forwardDev = NULL; + def->npfs++; + + } + + ret = 0; +cleanup: + VIR_FREE(type); + VIR_FREE(forwardDev); + VIR_FREE(forwardManaged); + VIR_FREE(forwardPfNodes); + VIR_FREE(forwardIfNodes); + VIR_FREE(forwardAddrNodes); + ctxt->node = save; + return ret; +} + static virNetworkDefPtr virNetworkDefParseXML(xmlXPathContextPtr ctxt) { @@ -1277,17 +1491,11 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) char *stp = NULL; xmlNodePtr *ipNodes = NULL; xmlNodePtr *portGroupNodes = NULL; - xmlNodePtr *forwardIfNodes = NULL; - xmlNodePtr *forwardPfNodes = NULL; - xmlNodePtr *forwardAddrNodes = NULL; + int nIps, nPortGroups; xmlNodePtr dnsNode = NULL; xmlNodePtr virtPortNode = NULL; xmlNodePtr forwardNode = NULL; - int nIps, nPortGroups, nForwardIfs, nForwardPfs, nForwardAddrs; char *ipv6nogwStr = NULL; - char *forwardDev = NULL; - char *forwardManaged = NULL; - char *type = NULL; xmlNodePtr save = ctxt->node; xmlNodePtr bandwidthNode = NULL; xmlNodePtr vlanNode; @@ -1353,6 +1561,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) /* Parse bridge information */ def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt); stp = virXPathString("string(./bridge[1]/@stp)", ctxt); + def->stp = (stp && STREQ(stp, "off")) ? 0 : 1; if (virXPathULong("string(./bridge[1]/@delay)", ctxt, &def->delay) < 0) def->delay = 0; @@ -1437,246 +1646,82 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) VIR_FREE(ipNodes); forwardNode = virXPathNode("./forward", ctxt); - if (!forwardNode) { - def->forward.type = VIR_NETWORK_FORWARD_NONE; - def->stp = (stp && STREQ(stp, "off")) ? 0 : 1; - } else { - ctxt->node = forwardNode; - tmp = virXPathString("string(./@mode)", ctxt); - if (tmp) { - if ((def->forward.type = virNetworkForwardTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown forwarding type '%s'"), tmp); - VIR_FREE(tmp); - goto error; - } - VIR_FREE(tmp); - } else { - def->forward.type = VIR_NETWORK_FORWARD_NAT; - } - - forwardDev = virXPathString("string(./@dev)", ctxt); - forwardManaged = virXPathString("string(./@managed)", ctxt); - if (forwardManaged != NULL) { - if (STRCASEEQ(forwardManaged, "yes")) - def->forward.managed = true; - } + if (forwardNode && + virNetworkForwardDefParseXML(def->name, forwardNode, ctxt, &def->forward) < 0) { + goto error; + } - /* all of these modes can use a pool of physical interfaces */ - nForwardIfs = virXPathNodeSet("./interface", ctxt, &forwardIfNodes); - nForwardPfs = virXPathNodeSet("./pf", ctxt, &forwardPfNodes); - nForwardAddrs = virXPathNodeSet("./address", ctxt, &forwardAddrNodes); + /* Validate some items in the main NetworkDef that need to align + * with the chosen forward mode. + */ + switch (def->forward.type) { + case VIR_NETWORK_FORWARD_NONE: + break; - if (nForwardIfs < 0 || nForwardPfs < 0 || nForwardAddrs < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("No interface pool or SRIOV physical device given")); + case VIR_NETWORK_FORWARD_ROUTE: + case VIR_NETWORK_FORWARD_NAT: + /* It's pointless to specify L3 forwarding without specifying + * the network we're on. + */ + if (def->nips == 0) { + virReportError(VIR_ERR_XML_ERROR, + _("%s forwarding requested, " + "but no IP address provided for network '%s'"), + virNetworkForwardTypeToString(def->forward.type), + def->name); goto error; } - - if ((nForwardIfs > 0) && (nForwardAddrs > 0)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Address and interface attributes are mutually exclusive")); + if (def->forward.nifs > 1) { + virReportError(VIR_ERR_XML_ERROR, + _("multiple forwarding interfaces specified " + "for network '%s', only one is supported"), + def->name); goto error; } + break; - if ((nForwardPfs > 0) && ((nForwardIfs > 0) || (nForwardAddrs > 0))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Address/interface attributes and Physical function are mutually exclusive ")); + case VIR_NETWORK_FORWARD_PRIVATE: + case VIR_NETWORK_FORWARD_VEPA: + case VIR_NETWORK_FORWARD_PASSTHROUGH: + case VIR_NETWORK_FORWARD_HOSTDEV: + if (def->bridge) { + virReportError(VIR_ERR_XML_ERROR, + _("bridge name not allowed in %s mode (network '%s')"), + virNetworkForwardTypeToString(def->forward.type), + def->name); goto error; } - - if (nForwardPfs == 1) { - if (VIR_ALLOC_N(def->forward.pfs, nForwardPfs) < 0) { - virReportOOMError(); - goto error; - } - - if (forwardDev) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("A forward Dev should not be used when using a SRIOV PF")); - goto error; - } - - forwardDev = virXMLPropString(*forwardPfNodes, "dev"); - if (!forwardDev) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing required dev attribute in network '%s' pf element"), - def->name); - goto error; - } - - def->forward.pfs->dev = forwardDev; - forwardDev = NULL; - def->forward.npfs++; - } else if (nForwardPfs > 1) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Use of more than one physical interface is not allowed")); + /* fall through to next case */ + case VIR_NETWORK_FORWARD_BRIDGE: + if (def->delay || stp) { + virReportError(VIR_ERR_XML_ERROR, + _("bridge delay/stp options only allowed in route, nat, and isolated mode, not in %s (network '%s')"), + virNetworkForwardTypeToString(def->forward.type), + def->name); goto error; } - if (nForwardAddrs > 0) { - int ii; - - if (VIR_ALLOC_N(def->forward.ifs, nForwardAddrs) < 0) { - virReportOOMError(); - goto error; - } - - if (forwardDev) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("A forward Dev should not be used when using address attribute")); - goto error; - } - - for (ii = 0; ii < nForwardAddrs; ii++) { - type = virXMLPropString(forwardAddrNodes[ii], "type"); - - if (type) { - if ((def->forward.ifs[ii].type = virNetworkForwardHostdevDeviceTypeFromString(type)) < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown address type '%s'"), type); - goto error; - } - } else { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("No type specified for device address")); - goto error; - } - - switch (def->forward.ifs[ii].type) { - case VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_PCI: - if (virDevicePCIAddressParseXML(forwardAddrNodes[ii], &(def->forward.ifs[ii].device.pci)) < 0) - goto error; - break; - - /* Add USB case here */ - - default: - virReportError(VIR_ERR_XML_ERROR, - _("unknown address type '%s'"), type); - goto error; - } - VIR_FREE(type); - def->forward.nifs++; - } - } - else if (nForwardIfs > 0 || forwardDev) { - int ii; - - /* allocate array to hold all the portgroups */ - if (VIR_ALLOC_N(def->forward.ifs, MAX(nForwardIfs, 1)) < 0) { - virReportOOMError(); - goto error; - } - - if (forwardDev) { - def->forward.ifs[0].device.dev = forwardDev; - def->forward.ifs[0].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; - forwardDev = NULL; - def->forward.nifs++; - } - - /* parse each forwardIf */ - for (ii = 0; ii < nForwardIfs; ii++) { - forwardDev = virXMLPropString(forwardIfNodes[ii], "dev"); - if (!forwardDev) { - virReportError(VIR_ERR_XML_ERROR, - _("Missing required dev attribute in network '%s' forward interface element"), - def->name); - goto error; - } - - if ((ii == 0) && (def->forward.nifs == 1)) { - /* both forwardDev and an interface element are present. - * If they don't match, it's an error. */ - if (STRNEQ(forwardDev, def->forward.ifs[0].device.dev)) { - virReportError(VIR_ERR_XML_ERROR, - _("forward dev '%s' must match first interface element dev '%s' in network '%s'"), - def->forward.ifs[0].device.dev, - forwardDev, def->name); - goto error; - } - VIR_FREE(forwardDev); - continue; - } - - def->forward.ifs[ii].device.dev = forwardDev; - forwardDev = NULL; - def->forward.ifs[ii].type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV; - def->forward.nifs++; - } - } - VIR_FREE(type); - VIR_FREE(forwardDev); - VIR_FREE(forwardManaged); - VIR_FREE(forwardPfNodes); - VIR_FREE(forwardIfNodes); - VIR_FREE(forwardAddrNodes); - switch (def->forward.type) { - case VIR_NETWORK_FORWARD_ROUTE: - case VIR_NETWORK_FORWARD_NAT: - /* It's pointless to specify L3 forwarding without specifying - * the network we're on. - */ - if (def->nips == 0) { - virReportError(VIR_ERR_XML_ERROR, - _("%s forwarding requested, but no IP address provided for network '%s'"), - virNetworkForwardTypeToString(def->forward.type), - def->name); - goto error; - } - if (def->forward.nifs > 1) { - virReportError(VIR_ERR_XML_ERROR, - _("multiple forwarding interfaces specified for network '%s', only one is supported"), - def->name); - goto error; - } - def->stp = (stp && STREQ(stp, "off")) ? 0 : 1; - break; - case VIR_NETWORK_FORWARD_PRIVATE: - case VIR_NETWORK_FORWARD_VEPA: - case VIR_NETWORK_FORWARD_PASSTHROUGH: - case VIR_NETWORK_FORWARD_HOSTDEV: - if (def->bridge) { - virReportError(VIR_ERR_XML_ERROR, - _("bridge name not allowed in %s mode (network '%s')"), - virNetworkForwardTypeToString(def->forward.type), - def->name); - goto error; - } - /* fall through to next case */ - case VIR_NETWORK_FORWARD_BRIDGE: - if (def->delay || stp) { - virReportError(VIR_ERR_XML_ERROR, - _("bridge delay/stp options only allowed in route, nat, and isolated mode, not in %s (network '%s')"), - virNetworkForwardTypeToString(def->forward.type), - def->name); - goto error; - } - if (def->bridge && (def->forward.nifs || def->forward.npfs)) { - virReportError(VIR_ERR_XML_ERROR, - _("A network with forward mode='%s' can specify " - "a bridge name or a forward dev, but not " - "both (network '%s')"), - virNetworkForwardTypeToString(def->forward.type), - def->name); - goto error; - } - break; + if (def->bridge && (def->forward.nifs || def->forward.npfs)) { + virReportError(VIR_ERR_XML_ERROR, + _("A network with forward mode='%s' can specify " + "a bridge name or a forward dev, but not " + "both (network '%s')"), + virNetworkForwardTypeToString(def->forward.type), + def->name); + goto error; } + break; } + VIR_FREE(stp); ctxt->node = save; return def; - error: +error: VIR_FREE(stp); virNetworkDefFree(def); VIR_FREE(ipNodes); VIR_FREE(portGroupNodes); - VIR_FREE(forwardIfNodes); - VIR_FREE(forwardPfNodes); VIR_FREE(ipv6nogwStr); - VIR_FREE(forwardDev); ctxt->node = save; return NULL; } -- 1.7.11.7
On 12/07/2012 01:56 PM, Laine Stump wrote:
Patches 1,2, and 4-7 refactor the network XML parsing functions to have a more uniform calling sequence for different subelements of <network>. This makes it simpler to write new virNetworkUpdate*() backend functions for those bits (because they're then more similar to existing functions.)
Patch 3 actually adds some new backends for updating subelements that weren't previously implemented - <dns> host/srv/txt records.
The intent is to followup on patched 4-7 with backends for updating <ip>, <forward>, and <bridge>; while I haven't gotten to those yet, I'd rather get these changes in sooner than later, both for more testing, and to make future backports simpler. Do I need to rebase the DHCPv6 patches with this update in mind?
Gene
On 12/07/2012 02:39 PM, Gene Czarcinski wrote:
On 12/07/2012 01:56 PM, Laine Stump wrote:
Patches 1,2, and 4-7 refactor the network XML parsing functions to have a more uniform calling sequence for different subelements of <network>. This makes it simpler to write new virNetworkUpdate*() backend functions for those bits (because they're then more similar to existing functions.)
Patch 3 actually adds some new backends for updating subelements that weren't previously implemented - <dns> host/srv/txt records.
The intent is to followup on patched 4-7 with backends for updating <ip>, <forward>, and <bridge>; while I haven't gotten to those yet, I'd rather get these changes in sooner than later, both for more testing, and to make future backports simpler. Do I need to rebase the DHCPv6 patches with this update in mind?
I had just tried doing a git am of your patches on top of mine to see. there was only a small problem with the first, which was easily resolved. The 2nd was a bit more involved, but I just got it handled now. I'll try to send the rebased patches late tonight (two holiday parties to go to now) and you can give them a try.
participants (2)
-
Gene Czarcinski -
Laine Stump