[libvirt] dnsmasq SRV/TXT RR and Host xml parsing upgrade and bug fix
The set of patches aims to three kind of jobs. First, for SRV RR, in order to strictly comform to RFC 2782, the patch changed and upgraded codes in following parts. The format of SRV RR: _Service._Proto.Name TTL Class SRV Priority Weight Port Target 1,The protocol and service labels are prepended with an underscore. e.g: _ldap._tcp.example.com 2,The range of Port,Priority,Weight is 0-65535. 3,A Target of "." means that the service is decidedly not available at this domain. Also, trying to invoke dnsmasq command line based on its manpage. --srv-host=<_service>.<_prot>.[<domain>],[<target>[,<port>[,<priority>[,<weight>]]]] _service and protocol are mandatory options. The reset of them are optional. When the target is not given in dnsmasq, it comform to the third item above. For TXT RR, we changed the text field from mandatory to optional based on dnsmasq manpage: --txt-record=<name>[[,<text>],<text>] Second, changed the way of dns element as well as its children elements parsing via virXPathNodeSet(). It has advantages when multiple children elements there is. Third, fixed related bug and unexposed bugs https://bugzilla.redhat.com/show_bug.cgi?id=836326
Replace virNetworkDNSSrvRecordsDefPtr type with virNetworkDNSSrvRecordsDefPtr * type for srvrecords field in struct _virNetworkDNSDef. Create two new helper functions to allocate and free memory of type virNetworkDNSSrvRecordsDefPtr. Fix a bug when any of values of port, priority, weight is not given in SRV xml description, uninitialized of these three variables are used which causes the failure of creating dnsmasq subprocess. before: ... <dns> <srv service='kerberos' protocol='tcp' target='pony.demo.redhat.com' port='88'/> </dns> ... --srv-host=kerberos.tcp,pony.demo.redhat.com,88,1095468768,32544 after: --srv-host=kerberos.tcp,pony.demo.redhat.com,88,, --- src/conf/network_conf.c | 109 +++++++++++++++++++++++++++--------------- src/conf/network_conf.h | 5 ++- src/network/bridge_driver.c | 22 ++++---- 3 files changed, 85 insertions(+), 51 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 515bc36..012be6a 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -140,10 +140,10 @@ 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); + 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); } } VIR_FREE(def->srvrecords); @@ -569,6 +569,40 @@ error: return ret; } +void +virNetworkDNSSrvDefFree(virNetworkDNSSrvRecordsDefPtr srv) +{ + if (!srv) + return; + + VIR_FREE(srv->domain); + VIR_FREE(srv->service); + VIR_FREE(srv->protocol); + VIR_FREE(srv->target); + + VIR_FREE(srv); +} + +virNetworkDNSSrvRecordsDefPtr +virNetworkDNSSrvDefAlloc(void) +{ + virNetworkDNSSrvRecordsDefPtr srv = NULL; + + if (VIR_ALLOC(srv) < 0) { + virReportOOMError(); + return NULL; + } + + /* The default for port is one */ + srv->port = 1; + + /* The defaults for weight and priority are zero */ + srv->priority = 0; + srv->weight = 0; + + return srv; +} + static int virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, xmlNodePtr cur, @@ -583,6 +617,11 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, int weight; int ret = 0; + virNetworkDNSSrvRecordsDefPtr srv = NULL; + + if (!(srv = virNetworkDNSSrvDefAlloc())) + goto error; + if (!(service = virXMLPropString(cur, "service"))) { virNetworkReportError(VIR_ERR_XML_DETAIL, "%s", _("Missing required service attribute in dns srv record")); @@ -614,37 +653,28 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, 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; + srv->service = service; + srv->protocol = protocol; /* Following attributes are optional but we had to make sure they're NULL above */ if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { - xmlNodePtr save_ctxt = ctxt->node; + srv->target = target; + srv->domain = domain; + xmlNodePtr save_ctxt = ctxt->node; ctxt->node = cur; - if (virXPathInt("string(./@port)", ctxt, &port)) - def->srvrecords[def->nsrvrecords].port = port; + if (virXPathInt("string(./@port)", ctxt, &port) == 0) + srv->port = port; - if (virXPathInt("string(./@priority)", ctxt, &priority)) - def->srvrecords[def->nsrvrecords].priority = priority; + if (virXPathInt("string(./@priority)", ctxt, &priority) == 0) + srv->priority = priority; - if (virXPathInt("string(./@weight)", ctxt, &weight)) - def->srvrecords[def->nsrvrecords].weight = weight; + if (virXPathInt("string(./@weight)", ctxt, &weight) == 0) + srv->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->nsrvrecords++; + def->srvrecords[def->nsrvrecords++] = srv; goto cleanup; @@ -653,6 +683,7 @@ error: VIR_FREE(service); VIR_FREE(protocol); VIR_FREE(target); + virNetworkDNSSrvDefFree(srv); ret = -1; @@ -1306,21 +1337,21 @@ virNetworkDNSDefFormat(virBufferPtr buf, } for (i = 0 ; i < def->nsrvrecords ; i++) { - if (def->srvrecords[i].service && def->srvrecords[i].protocol) { + if (def->srvrecords[i]->service && def->srvrecords[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->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); virBufferAsprintf(buf, "/>\n"); } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4339a69..10b8328 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -83,6 +83,9 @@ struct _virNetworkDNSSrvRecordsDef { int weight; }; +void virNetworkDNSSrvDefFree(virNetworkDNSSrvRecordsDefPtr srv); +virNetworkDNSSrvRecordsDefPtr virNetworkDNSSrvDefAlloc(void); + struct _virNetworkDNSHostsDef { virSocketAddr ip; int nnames; @@ -97,7 +100,7 @@ struct _virNetworkDNSDef { unsigned int nhosts; virNetworkDNSHostsDefPtr hosts; unsigned int nsrvrecords; - virNetworkDNSSrvRecordsDefPtr srvrecords; + virNetworkDNSSrvRecordsDefPtr *srvrecords; }; typedef struct _virNetworkDNSDef *virNetworkDNSDefPtr; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7e8de19..df999b9 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -526,31 +526,31 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, } 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) { + if (dns->srvrecords[i]->service && dns->srvrecords[i]->protocol) { + if (dns->srvrecords[i]->port) { + if (virAsprintf(&recordPort, "%d", dns->srvrecords[i]->port) < 0) { virReportOOMError(); goto cleanup; } } - if (dns->srvrecords[i].priority) { - if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i].priority) < 0) { + if (dns->srvrecords[i]->priority) { + if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i]->priority) < 0) { virReportOOMError(); goto cleanup; } } - if (dns->srvrecords[i].weight) { - if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i].weight) < 0) { + if (dns->srvrecords[i]->weight) { + if (virAsprintf(&recordWeight, "%d", dns->srvrecords[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->srvrecords[i]->service, + dns->srvrecords[i]->protocol, + dns->srvrecords[i]->domain ? dns->srvrecords[i]->domain : "", + dns->srvrecords[i]->target ? dns->srvrecords[i]->target : "", recordPort ? recordPort : "", recordPriority ? recordPriority : "", recordWeight ? recordWeight : "") < 0) { -- 1.7.7.5
Refacor the virNetworkDNSSrvDefParseXML function. Prefix service and protocol with a underscore during parsing according to RFC2782. <srv service='kerberos' protocol='tcp'/> before: "--srv-host=kerberos.tcp" after: "--srv-host=_kerberos._tcp" domain is optional which will override the value of --domain options to dnsmasq if supplied to form the following format <_service>.<_prot>.[<domain>] --srv-host=_kerberos._tcp.pony.demo.redhat.com,pony domain has nothing to do with port,priority,weight, so extract it from if clause. target is optional. If it is not supplied, it means the SRV RR is invalid in this DNS zone. Values of port,priority,weight are meaningful only when target is supplied. These three values have toplimit 65535, so checking the range of them. The default for port is 1 and the defaults for weight and priority are 0. --- src/conf/network_conf.c | 200 +++++++++++++++++++++++++++++++---------------- src/conf/network_conf.h | 3 + 2 files changed, 136 insertions(+), 67 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 012be6a..f6694ed 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -603,92 +603,143 @@ virNetworkDNSSrvDefAlloc(void) return srv; } -static int -virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def, - xmlNodePtr cur, - xmlXPathContextPtr ctxt) +static char * +virNetworkDNSSrvDefParseServiceAndProtocol(xmlNodePtr cur, + const char *attr) { - char *domain = NULL; - char *service = NULL; - char *protocol = NULL; - char *target = NULL; - int port; - int priority; - int weight; - int ret = 0; - - virNetworkDNSSrvRecordsDefPtr srv = NULL; + char *value = NULL; + char *ret = NULL; + const char *prefix = "_"; + int offset = 1; + size_t length; - if (!(srv = virNetworkDNSSrvDefAlloc())) - goto error; + if (!cur || !attr) + return NULL; - if (!(service = virXMLPropString(cur, "service"))) { + if (!(value = virXMLPropString(cur, attr))) { virNetworkReportError(VIR_ERR_XML_DETAIL, - "%s", _("Missing required service attribute in dns srv record")); + _("Missing required '%s' attribute in dns srv record"), + attr); goto error; } - if (strlen(service) > DNS_RECORD_LENGTH_SRV) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - _("Service name is too long, limit is %d bytes"), - DNS_RECORD_LENGTH_SRV); - goto error; - } + length = strlen(value); - if (!(protocol = virXMLPropString(cur, "protocol"))) { + if (STREQ(attr, "service") && + (length == 0 || length > DNS_RECORD_LENGTH_SRV)) { virNetworkReportError(VIR_ERR_XML_DETAIL, - _("Missing required protocol attribute in dns srv record '%s'"), service); + _("Service name should not be null, limit is %d bytes"), + DNS_RECORD_LENGTH_SRV); goto error; } /* Check whether protocol value is the supported one */ - if (STRNEQ(protocol, "tcp") && (STRNEQ(protocol, "udp"))) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - _("Invalid protocol attribute value '%s'"), protocol); - goto error; + if (STREQ(attr, "protocol")) { + if (length == 0 || (STRNEQ(value, "tcp") && STRNEQ(value, "udp"))) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("Protocol should not be null, 'tcp' or 'udp'")); + goto error; + } } - if (VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + 1) < 0) { + /* Prefix the service and protocol with '_'. + */ + if (VIR_ALLOC_N(ret, length + offset + 1) < 0) { virReportOOMError(); goto error; } + strcpy(ret, prefix); + strcat(ret + offset, value); + VIR_FREE(value); + + return ret; +error: + VIR_FREE(value); + VIR_FREE(ret); + return NULL; +} + +static char * +virNetworkDNSSrvDefParseTarget(xmlNodePtr cur) +{ + char *target = NULL; + + if (!cur) + return NULL; + + /* Target is optional, we ignore null string "" + * According to RFC 2782, a target of "." means that the service + * is decidedly not available at this domain. We just ignore "." + * given by user explicitly because dnsmasq implements it via + * not feeding target value in command line. + */ + target = virXMLPropString(cur, "target"); + + if (target && (!strlen(target) || *target == '.')) + VIR_FREE(target); + + return target; +} + +static virNetworkDNSSrvRecordsDefPtr +virNetworkDNSSrvDefParseXML(xmlNodePtr cur, + xmlXPathContextPtr ctxt) +{ + virNetworkDNSSrvRecordsDefPtr srv = NULL; + + if (!(srv = virNetworkDNSSrvDefAlloc())) + goto error; + + if (!(srv->service = + virNetworkDNSSrvDefParseServiceAndProtocol(cur, "service"))) + goto error; + + if (!(srv->protocol = + virNetworkDNSSrvDefParseServiceAndProtocol(cur, "protocol"))) + goto error; - srv->service = service; - srv->protocol = protocol; + /* Domain is optional. If domain is NULL, + * SRV will use the domain label from <domain>. + */ + srv->domain = virXMLPropString(cur, "domain"); - /* Following attributes are optional but we had to make sure they're NULL above */ - if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) { - srv->target = target; - srv->domain = domain; + srv->target = virNetworkDNSSrvDefParseTarget(cur); + /* Without target, the service provided by SRV RR will be + * treated as not availiable, giving up parsing. + */ + if (srv->target) { xmlNodePtr save_ctxt = ctxt->node; ctxt->node = cur; - if (virXPathInt("string(./@port)", ctxt, &port) == 0) - srv->port = port; - if (virXPathInt("string(./@priority)", ctxt, &priority) == 0) - srv->priority = priority; + if (virXPathInt("string(./@port)", ctxt, &srv->port) == 0) + if (srv->port < 0 || srv->port > DNS_SRV_PORT_MAX) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("Service port on target is out of range(0-65535)")); + goto error; + } - if (virXPathInt("string(./@weight)", ctxt, &weight) == 0) - srv->weight = weight; - ctxt->node = save_ctxt; - } + if (virXPathInt("string(./@priority)", ctxt, &srv->priority) == 0) + if (srv->priority < 0 || srv->priority > DNS_SRV_PRIORITY_MAX) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("SRV priority value is out of range(0-65535)")); + goto error; + } - def->srvrecords[def->nsrvrecords++] = srv; + if (virXPathInt("string(./@weight)", ctxt, &srv->weight) == 0) + if (srv->weight < 0 || srv->weight > DNS_SRV_WEIGHT_MAX) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("SRV weight value is out of range(0-65535)")); + goto error; + } - goto cleanup; + ctxt->node = save_ctxt; + } + return srv; error: - VIR_FREE(domain); - VIR_FREE(service); - VIR_FREE(protocol); - VIR_FREE(target); virNetworkDNSSrvDefFree(srv); - - ret = -1; - -cleanup: - return ret; + return NULL; } static int @@ -696,17 +747,36 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, xmlNodePtr node, xmlXPathContextPtr ctxt) { + xmlNodePtr *nodes = NULL; xmlNodePtr cur; + int i, n; int ret = -1; char *name = NULL; char *value = NULL; virNetworkDNSDefPtr def = NULL; - if (VIR_ALLOC(def) < 0) { - virReportOOMError(); + if (VIR_ALLOC(def) < 0) + goto no_memory; + + if ((n = virXPathNodeSet("./dns/srv", ctxt, &nodes)) < 0) { + virNetworkReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract srv nodes")); goto error; } + if (n && VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + n) < 0) + goto no_memory; + for (i = 0; i < n; i++) { + virNetworkDNSSrvRecordsDefPtr srv; + + srv = virNetworkDNSSrvDefParseXML(nodes[i], ctxt); + if (!srv) + goto error; + + def->srvrecords[def->nsrvrecords++] = srv; + } + VIR_FREE(nodes); + cur = node->children; while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && @@ -728,10 +798,8 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, goto error; } - if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) { - virReportOOMError(); - goto error; - } + if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) + goto no_memory; def->txtrecords[def->ntxtrecords].name = name; def->txtrecords[def->ntxtrecords].value = value; @@ -739,11 +807,6 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, 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) @@ -754,6 +817,9 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, } ret = 0; +no_memory: + virReportOOMError(); + error: if (ret < 0) { VIR_FREE(name); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 10b8328..345c99f 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -25,6 +25,9 @@ # define __NETWORK_CONF_H__ # define DNS_RECORD_LENGTH_SRV (512 - 30) /* Limit minus overhead as mentioned in RFC-2782 */ +# define DNS_SRV_PORT_MAX 65535 +# define DNS_SRV_PRIORITY_MAX 65535 +# define DNS_SRV_WEIGHT_MAX 65535 # include <libxml/parser.h> # include <libxml/tree.h> -- 1.7.7.5
bridge_driver.c: Only the service and potocol are mandatory argument. If domain or target is not supplied in xml, we should not include the period and comma in command line, so we don't provide them in dnsmasq command line and leave the work to dnsmasq. <srv service='kerberos' protocol='tcp'/> before: --srv-host=kerberos.tcp..,,, It will report error in this case after: --srv-host=_kerberos._tcp network_conf.c: Because we initialized these three values, we should dumpxml their values anytime. --- src/conf/network_conf.c | 31 +++++++++++++++-------------- src/network/bridge_driver.c | 44 ++++++++++++++++++++++++++++++------------ 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index f6694ed..99c6fc1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -755,8 +755,10 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, char *value = NULL; virNetworkDNSDefPtr def = NULL; - if (VIR_ALLOC(def) < 0) - goto no_memory; + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto error; + } if ((n = virXPathNodeSet("./dns/srv", ctxt, &nodes)) < 0) { virNetworkReportError(VIR_ERR_INTERNAL_ERROR, @@ -764,8 +766,10 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, goto error; } - if (n && VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + n) < 0) - goto no_memory; + if (n && VIR_REALLOC_N(def->srvrecords, def->nsrvrecords + n) < 0) { + virReportOOMError(); + goto error; + } for (i = 0; i < n; i++) { virNetworkDNSSrvRecordsDefPtr srv; @@ -798,8 +802,10 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, goto error; } - if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) - goto no_memory; + if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) { + virReportOOMError(); + goto error; + } def->txtrecords[def->ntxtrecords].name = name; def->txtrecords[def->ntxtrecords].value = value; @@ -817,9 +823,6 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, } ret = 0; -no_memory: - virReportOOMError(); - error: if (ret < 0) { VIR_FREE(name); @@ -1412,12 +1415,10 @@ virNetworkDNSDefFormat(virBufferPtr buf, 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); + + virBufferAsprintf(buf, " port='%d'", def->srvrecords[i]->port); + virBufferAsprintf(buf, " priority='%d'", def->srvrecords[i]->priority); + virBufferAsprintf(buf, " weight='%d'", def->srvrecords[i]->weight); virBufferAsprintf(buf, "/>\n"); } diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index df999b9..95a9b96 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -459,6 +459,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; int ii; + char *domain = NULL; + char *target = NULL; char *record = NULL; char *recordPort = NULL; char *recordWeight = NULL; @@ -527,38 +529,52 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, 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) { + if (dns->srvrecords[i]->domain) { + if (virAsprintf(&domain, ".%s", + dns->srvrecords[i]->domain) < 0) { virReportOOMError(); goto cleanup; } } - if (dns->srvrecords[i]->priority) { - if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i]->priority) < 0) { + + if (dns->srvrecords[i]->target) { + if (virAsprintf(&target, ",%s", + dns->srvrecords[i]->target) < 0) { virReportOOMError(); goto cleanup; } - } - if (dns->srvrecords[i]->weight) { - if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i]->weight) < 0) { + if (virAsprintf(&recordPort, ",%d", + dns->srvrecords[i]->port) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&recordPriority, ",%d", + dns->srvrecords[i]->priority) < 0) { + virReportOOMError(); + goto cleanup; + } + if (virAsprintf(&recordWeight, ",%d", + dns->srvrecords[i]->weight) < 0) { virReportOOMError(); goto cleanup; } } - if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s", + 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 : "", - recordPort ? recordPort : "", - recordPriority ? recordPriority : "", - recordWeight ? recordWeight : "") < 0) { + domain ? domain : "", + target ? target : "", + recordPort ? recordPort : "", + recordPriority ? recordPriority : "", + recordWeight ? recordWeight : "") < 0) { virReportOOMError(); goto cleanup; } virCommandAddArgPair(cmd, "--srv-host", record); + VIR_FREE(domain); + VIR_FREE(target); VIR_FREE(record); VIR_FREE(recordPort); VIR_FREE(recordWeight); @@ -666,6 +682,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, ret = 0; cleanup: + VIR_FREE(domain); + VIR_FREE(target); VIR_FREE(record); VIR_FREE(recordPort); VIR_FREE(recordWeight); -- 1.7.7.5
Change the type of txtrecords from virNetworkDNSTxtRecordsDefPtr to virNetworkDNSTxtRecordsDefPtr *. Refacotr the virNetworkDNSDefParseXML to create a new TXT xml Parsing function. Make value (text field in TXT RR) optional according to dnsmasq manpage. --txt-record=<name>[[,<text>],<text>] --- src/conf/network_conf.c | 111 +++++++++++++++++++++++++++---------------- src/conf/network_conf.h | 2 +- src/network/bridge_driver.c | 16 +++++- 3 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 99c6fc1..b720c9f 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -125,8 +125,8 @@ 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); + VIR_FREE(def->txtrecords[def->ntxtrecords]->name); + VIR_FREE(def->txtrecords[def->ntxtrecords]->value); } } VIR_FREE(def->txtrecords); @@ -512,6 +512,44 @@ virNetworkDHCPRangeDefParseXML(const char *networkName, return 0; } +static virNetworkDNSTxtRecordsDefPtr +virNetworkDNSTxtDefParseXML(xmlNodePtr cur) + +{ + virNetworkDNSTxtRecordsDefPtr txt = NULL; + + if (VIR_ALLOC(txt) < 0) { + virReportOOMError(); + return NULL; + } + + if (!(txt->name = virXMLPropString(cur, "name"))) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + "%s", _("Missing required name attribute in dns txt record")); + goto error; + } + + /* The text field for dnsmasq is optional */ + txt->value = virXMLPropString(cur, "value"); + if (txt->value && *txt->value == '\0') + txt->value = NULL; + + if (strchr(txt->name, ' ') != NULL) { + virNetworkReportError(VIR_ERR_XML_DETAIL, + _("spaces are not allowed in DNS TXT record names (name is '%s')"), txt->name); + goto error; + } + + return txt; +error: + if (txt) { + VIR_FREE(txt->name); + VIR_FREE(txt->value); + } + VIR_FREE(txt); + return NULL; +} + static int virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def, xmlNodePtr node) @@ -751,8 +789,6 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, xmlNodePtr cur; int i, n; int ret = -1; - char *name = NULL; - char *value = NULL; virNetworkDNSDefPtr def = NULL; if (VIR_ALLOC(def) < 0) { @@ -781,38 +817,31 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, } VIR_FREE(nodes); - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "txt")) { - if (!(name = virXMLPropString(cur, "name"))) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - "%s", _("Missing required name attribute in dns txt record")); - goto error; - } - if (!(value = virXMLPropString(cur, "value"))) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - _("Missing required value attribute in dns txt record '%s'"), name); - goto error; - } + if ((n = virXPathNodeSet("./dns/txt", ctxt, &nodes)) < 0) { + virNetworkReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract txt nodes")); + goto error; + } - if (strchr(name, ' ') != NULL) { - virNetworkReportError(VIR_ERR_XML_DETAIL, - _("spaces are not allowed in DNS TXT record names (name is '%s')"), name); - goto error; - } + if (n && VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + n) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < n; i++) { + virNetworkDNSTxtRecordsDefPtr txt; - if (VIR_REALLOC_N(def->txtrecords, def->ntxtrecords + 1) < 0) { - virReportOOMError(); - goto error; - } + txt = virNetworkDNSTxtDefParseXML(nodes[i]); + if (!txt) + goto error; - def->txtrecords[def->ntxtrecords].name = name; - def->txtrecords[def->ntxtrecords].value = value; - def->ntxtrecords++; - name = NULL; - value = NULL; - } else if (cur->type == XML_ELEMENT_NODE && + def->txtrecords[def->ntxtrecords++] = txt; + } + VIR_FREE(nodes); + + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name, BAD_CAST "host")) { ret = virNetworkDNSHostsDefParseXML(def, cur); if (ret < 0) @@ -824,13 +853,11 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, ret = 0; error: - if (ret < 0) { - VIR_FREE(name); - VIR_FREE(value); + if (ret < 0) virNetworkDNSDefFree(def); - } else { + else *dnsdef = def; - } + return ret; } @@ -1400,9 +1427,11 @@ virNetworkDNSDefFormat(virBufferPtr buf, virBufferAddLit(buf, " <dns>\n"); for (i = 0 ; i < def->ntxtrecords ; i++) { - virBufferAsprintf(buf, " <txt name='%s' value='%s' />\n", - def->txtrecords[i].name, - def->txtrecords[i].value); + virBufferAsprintf(buf, " <txt name='%s'", def->txtrecords[i]->name); + if (def->txtrecords[i]->value) + virBufferAsprintf(buf, " value='%s'", def->txtrecords[i]->value); + + virBufferAsprintf(buf, "/>\n"); } for (i = 0 ; i < def->nsrvrecords ; i++) { diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 345c99f..16fbe81 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -99,7 +99,7 @@ typedef struct _virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr; struct _virNetworkDNSDef { unsigned int ntxtrecords; - virNetworkDNSTxtRecordsDefPtr txtrecords; + virNetworkDNSTxtRecordsDefPtr *txtrecords; unsigned int nhosts; virNetworkDNSHostsDefPtr hosts; unsigned int nsrvrecords; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 95a9b96..c0b05bd 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -459,6 +459,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int r, ret = -1; int nbleases = 0; int ii; + char *txtValue = NULL; char *domain = NULL; char *target = NULL; char *record = NULL; @@ -522,9 +523,17 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, int i; for (i = 0; i < dns->ntxtrecords; i++) { - virCommandAddArgFormat(cmd, "--txt-record=%s,%s", - dns->txtrecords[i].name, - dns->txtrecords[i].value); + if (dns->txtrecords[i]->value) { + if (virAsprintf(&txtValue, ",%s", + dns->txtrecords[i]->value) < 0) { + virReportOOMError(); + goto cleanup; + } + } + virCommandAddArgFormat(cmd, "--txt-record=%s%s", + dns->txtrecords[i]->name, + txtValue ? txtValue : ""); + VIR_FREE(txtValue); } for (i = 0; i < dns->nsrvrecords; i++) { @@ -682,6 +691,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, ret = 0; cleanup: + VIR_FREE(txtValue); VIR_FREE(domain); VIR_FREE(target); VIR_FREE(record); -- 1.7.7.5
Change the type of host filed in struct _virNetworkDNSDef from virNetworkDNSHostsDefPtr to virNetworkDNSHostsDefPtr *. Create new function virNetworkDNSHostsDefParseXML to parse host xml defination. Create virNetworkDNSHostsDefFree to free type data of type virNetworkDNSHostsDefPtr. --- src/conf/network_conf.c | 98 ++++++++++++++++++++++++------------------ src/conf/network_conf.h | 5 +- src/network/bridge_driver.c | 2 +- 3 files changed, 60 insertions(+), 45 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b720c9f..1d0ae50 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -132,9 +132,9 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def) VIR_FREE(def->txtrecords); 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); + 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); } } VIR_FREE(def->hosts); @@ -550,33 +550,42 @@ error: return NULL; } -static int -virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def, - xmlNodePtr node) +void virNetworkDNSHostsDefFree(virNetworkDNSHostsDefPtr host) +{ + int i; + + if (!host) + return; + + for (i = 0; i < host->nnames; i++) + VIR_FREE(host->names[i]); + + VIR_FREE(host->names); + VIR_FREE(host); +} + +static virNetworkDNSHostsDefPtr +virNetworkDNSHostsDefParseXML(xmlNodePtr node) { xmlNodePtr cur; - char *ip; + char *ip = NULL; virSocketAddr inaddr; - int ret = -1; + virNetworkDNSHostsDefPtr host = NULL; + + if (VIR_ALLOC(host) < 0) { + virReportOOMError(); + goto error; + } if (!(ip = virXMLPropString(node, "ip")) || (virSocketAddrParse(&inaddr, ip, AF_UNSPEC) < 0)) { virNetworkReportError(VIR_ERR_XML_DETAIL, _("Missing IP address in DNS host definition")); - VIR_FREE(ip); goto error; } - VIR_FREE(ip); + host->ip = inaddr; - 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) { + if (VIR_ALLOC(host->names) < 0) { virReportOOMError(); goto error; } @@ -586,25 +595,24 @@ virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def, 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(host->names, host->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++; + host->names[host->nnames] = strdup((char *)cur->children->content); + host->nnames++; } } cur = cur->next; } - def->nhosts++; - - ret = 0; - + return host; error: - return ret; + VIR_FREE(ip); + virNetworkDNSHostsDefFree(host); + return NULL; } void @@ -782,11 +790,9 @@ error: static int virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, - xmlNodePtr node, xmlXPathContextPtr ctxt) { xmlNodePtr *nodes = NULL; - xmlNodePtr cur; int i, n; int ret = -1; virNetworkDNSDefPtr def = NULL; @@ -838,18 +844,26 @@ virNetworkDNSDefParseXML(virNetworkDNSDefPtr *dnsdef, } VIR_FREE(nodes); + if ((n = virXPathNodeSet("./dns/host", ctxt, &nodes)) < 0) { + virNetworkReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot extract host nodes")); + goto error; + } - cur = node->children; - while (cur != NULL) { - if (cur->type == XML_ELEMENT_NODE && - xmlStrEqual(cur->name, BAD_CAST "host")) { - ret = virNetworkDNSHostsDefParseXML(def, cur); - if (ret < 0) - goto error; - } + if (n && VIR_REALLOC_N(def->hosts, def->nhosts + n) < 0) { + virReportOOMError(); + goto error; + } + for (i = 0; i < n; i++) { + virNetworkDNSHostsDefPtr host; - cur = cur->next; + host = virNetworkDNSHostsDefParseXML(nodes[i]); + if (!host) + goto error; + + def->hosts[def->nhosts++] = host; } + VIR_FREE(nodes); ret = 0; error: @@ -1136,7 +1150,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) dnsNode = virXPathNode("./dns", ctxt); if (dnsNode != NULL) { - if (virNetworkDNSDefParseXML(&def->dns, dnsNode, ctxt) < 0) + if (virNetworkDNSDefParseXML(&def->dns, ctxt) < 0) goto error; } @@ -1457,13 +1471,13 @@ virNetworkDNSDefFormat(virBufferPtr buf, int ii, j; for (ii = 0 ; ii < def->nhosts; ii++) { - char *ip = virSocketAddrFormat(&def->hosts[ii].ip); + char *ip = virSocketAddrFormat(&def->hosts[ii]->ip); virBufferAsprintf(buf, " <host ip='%s'>\n", ip); - for (j = 0; j < def->hosts[ii].nnames; j++) + for (j = 0; j < def->hosts[ii]->nnames; j++) virBufferAsprintf(buf, " <hostname>%s</hostname>\n", - def->hosts[ii].names[j]); + def->hosts[ii]->names[j]); virBufferAsprintf(buf, " </host>\n"); VIR_FREE(ip); diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 16fbe81..b11fc3c 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -89,19 +89,20 @@ struct _virNetworkDNSSrvRecordsDef { void virNetworkDNSSrvDefFree(virNetworkDNSSrvRecordsDefPtr srv); virNetworkDNSSrvRecordsDefPtr virNetworkDNSSrvDefAlloc(void); +typedef struct _virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr; struct _virNetworkDNSHostsDef { virSocketAddr ip; int nnames; char **names; }; -typedef struct _virNetworkDNSHostsDef *virNetworkDNSHostsDefPtr; +void virNetworkDNSHostsDefFree(virNetworkDNSHostsDefPtr host); struct _virNetworkDNSDef { unsigned int ntxtrecords; virNetworkDNSTxtRecordsDefPtr *txtrecords; unsigned int nhosts; - virNetworkDNSHostsDefPtr hosts; + virNetworkDNSHostsDefPtr *hosts; unsigned int nsrvrecords; virNetworkDNSSrvRecordsDefPtr *srvrecords; }; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c0b05bd..ca64d1d 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -436,7 +436,7 @@ networkBuildDnsmasqHostsfile(dnsmasqContext *dctx, if (dnsdef) { for (i = 0; i < dnsdef->nhosts; i++) { - virNetworkDNSHostsDefPtr host = &(dnsdef->hosts[i]); + virNetworkDNSHostsDefPtr 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) -- 1.7.7.5
On 07/08/2012 06:53 PM, Guannan Ren wrote:
The set of patches aims to three kind of jobs.
First, for SRV RR, in order to strictly comform to RFC 2782, the patch changed and upgraded codes in following parts. The format of SRV RR: _Service._Proto.Name TTL Class SRV Priority Weight Port Target
1,The protocol and service labels are prepended with an underscore. e.g: _ldap._tcp.example.com 2,The range of Port,Priority,Weight is 0-65535. 3,A Target of "." means that the service is decidedly not available at this domain.
Also, trying to invoke dnsmasq command line based on its manpage. --srv-host=<_service>.<_prot>.[<domain>],[<target>[,<port>[,<priority>[,<weight>]]]] _service and protocol are mandatory options. The reset of them are optional. When the target is not given in dnsmasq, it comform to the third item above.
For TXT RR, we changed the text field from mandatory to optional based on dnsmasq manpage: --txt-record=<name>[[,<text>],<text>]
Second, changed the way of dns element as well as its children elements parsing via virXPathNodeSet(). It has advantages when multiple children elements there is.
Third, fixed related bug and unexposed bugs https://bugzilla.redhat.com/show_bug.cgi?id=836326
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/09/2012 02:44 PM, Guannan Ren wrote:
On 07/08/2012 06:53 PM, Guannan Ren wrote:
The set of patches aims to three kind of jobs.
First, for SRV RR, in order to strictly comform to RFC 2782, the patch changed and upgraded codes in following parts. The format of SRV RR: _Service._Proto.Name TTL Class SRV Priority Weight Port Target
1,The protocol and service labels are prepended with an underscore. e.g: _ldap._tcp.example.com 2,The range of Port,Priority,Weight is 0-65535. 3,A Target of "." means that the service is decidedly not available at this domain.
Also, trying to invoke dnsmasq command line based on its manpage. --srv-host=<_service>.<_prot>.[<domain>],[<target>[,<port>[,<priority>[,<weight>]]]] _service and protocol are mandatory options. The reset of them are optional. When the target is not given in dnsmasq, it comform to the third item above.
For TXT RR, we changed the text field from mandatory to optional based on dnsmasq manpage: --txt-record=<name>[[,<text>],<text>]
Second, changed the way of dns element as well as its children elements parsing via virXPathNodeSet(). It has advantages when multiple children elements there is.
Third, fixed related bug and unexposed bugs https://bugzilla.redhat.com/show_bug.cgi?id=836326
Hi, Ping, anyone would like to review the patchset. The codes strictly conforms to AFC 2782 and dnsmasq manpage. :) http://www.ietf.org/rfc/rfc2782.txt Guannan Ren
participants (1)
-
Guannan Ren