Re: [libvirt] [PATCH 2/2] use client id for IPv6 DHCP host definition

On 02/18/2013 07:38 PM, Laine Stump wrote: > On 02/15/2013 02:02 PM, Gene Czarcinski wrote: >> Originally, only a host name was used to associate a >> DHCPv6 request with a specific IPv6 address. Further testing >> demonstrates that this is an unreliable method and, instead, >> a client-id or DUID needs to be used. According to DHCPv6 >> standards, this id can be a duid-LLT, duid-LL, or duid-UUID >> even though dnsmasq will accept almost any text string. >> > Other than the suggestion to use strcspn() instead of a while loop, this > all looks fine to me. ACK with that change made. If you (or someone > else) wants to ACK the short interdiff I've attached that makes that > change, I'll push it. > > >From 0b2f2f0794e931af901bc31e4fe6eadc799bee33 Mon Sep 17 00:00:00 2001 > From: Laine Stump <laine(a)laine.org&gt; > Date: Mon, 18 Feb 2013 21:33:49 -0500 > Subject: [PATCH] Changes to squash into "use client id for IPv6 DHCP host > definition" > > --- > src/conf/network_conf.c | 15 +++++---------- > src/util/virdnsmasq.c | 3 +-- > 2 files changed, 6 insertions(+), 12 deletions(-) > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > index 8657284..12bf4d7 100644 > --- a/src/conf/network_conf.c > +++ b/src/conf/network_conf.c > @@ -710,16 +710,11 @@ virNetworkDHCPHostDefParseXML(const char *networkName, > > id = virXMLPropString(node, "id"); > if (id) { > - char *cp = id; > - > - while (*cp) { > - if ((*cp != ':') && (!c_isxdigit(*cp))) { > - virReportError(VIR_ERR_XML_ERROR, > - _("Cannot use id '%s' in network '%s'"), > - id, networkName); > - goto cleanup; > - } > - cp++; > + char *cp = id + strcspn(id, "0123456789abcdefABCDEF:"); > + if (*cp) { > + virReportError(VIR_ERR_XML_ERROR, > + _("Invalid character '%c' in id '%s' of network '%s'"), > + *cp, id, networkName); That's not quite right. You want to use strspn(), not strcspn().

> +++ b/src/util/virdnsmasq.c > @@ -328,8 +328,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile, > id, ipstr) < 0) > goto alloc_error; > } > - } > - else if (name && mac) { > + } else if (name && mac) { This hunk is good. ACK to your interdiff once you use the correct function.