Re: [PATCH 1/2] network: backend for virNetworkUpdate of dhcp range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 20, 2012 at 10:25:40PM -0400, Laine Stump wrote:
> The dhcp range element is contained in the <dhcp> element of one of a
> network's <ip> elements. There can be multiple <range>
> elements. Because there are only two attributes (start and end), and
> those are exactly what you would use to identify a particular range,
> it doesn't really make sense to modify an existing element, so
> VIR_NETWORK_UPDATE_COMMAND_MODIFY isn't supported for this section,
> only ADD_FIRST, ADD_LAST, and DELETE.
> 
> Since virsh already has support for understanding all the defined
> sections, this new backend is automatically supported by virsh. You
> would use it like this:
> 
>   virsh net-update mynet add ip-dhcp-range \
>         "<range start='1.2.3.4' end='1.2.3.20'/>" --live --config
> 
> The bridge driver also already supports all sections, so it's doing
> the correct thing in this case as well - since the dhcp range is
> placed on the dnsmasq commandline, the bridge driver recreates the
> dnsmasq commandline, and re-runs dnsmasq whenever a range is
> added/deleted (and AFFECT_LIVE is specified in the flags).
> ---
>  src/conf/network_conf.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 88 insertions(+), 4 deletions(-)
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index f33834f..7fc559f 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -2494,14 +2494,98 @@ cleanup:
>  
>  static int
>  virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def,
> -                               unsigned int command ATTRIBUTE_UNUSED,
> +                               unsigned int command,
>                                 int parentIndex ATTRIBUTE_UNUSED,
> -                               xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED,
> +                               xmlXPathContextPtr ctxt,
>                                 /* virNetworkUpdateFlags */
>                                 unsigned int fflags ATTRIBUTE_UNUSED)
>  {
> -    virNetworkDefUpdateNoSupport(def, "ip dhcp range");
> -    return -1;
> +    int ii, ret = -1;
> +    virNetworkIpDefPtr ipdef = virNetworkIpDefByIndex(def, parentIndex);
> +    virNetworkDHCPRangeDef range;
> +
> +    memset(&range, 0, sizeof(range));
> +
> +    if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "range") < 0)
> +        goto cleanup;
> +
> +    /* ipdef is the ip element that needs its range array updated */
> +    if (!ipdef)
> +        goto cleanup;
> +
> +    /* parse the xml into a virNetworkDHCPRangeDef */
> +    if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
> +
> +        virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +                       _("dhcp ranges cannot be modified, "
> +                         "only added or deleted"));
> +        goto cleanup;
> +    }
> +
> +    if (virNetworkDHCPRangeDefParse(def->name, ctxt->node, &range) < 0)
> +        goto cleanup;
> +
> +    /* check if an entry with same name/address/ip already exists */
> +    for (ii = 0; ii < ipdef->nranges; ii++) {
> +        if (virSocketAddrEqual(&range.start, &ipdef->ranges[ii].start) &&
> +            virSocketAddrEqual(&range.end, &ipdef->ranges[ii].end)) {
> +            break;
> +        }
> +    }
> +
> +    if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
> +        (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
> +
> +        if (ii < ipdef->nranges) {
> +            char *startip = virSocketAddrFormat(&range.start);
> +            char *endip = virSocketAddrFormat(&range.end);
> +
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("there is an existing dhcp range entry in "
> +                             "network '%s' that matches "
> +                             "\"<range start='%s' end='%s'/>\""),
> +                           def->name,
> +                           startip ? startip : "unknown",
> +                           endip ? endip : "unknown");
> +            goto cleanup;
> +        }
> +
> +        /* add to beginning/end of list */
> +        if (VIR_REALLOC_N(ipdef->ranges, ipdef->nranges +1) < 0) {
> +            virReportOOMError();
> +            goto cleanup;
> +        }
> +
> +        if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) {
> +            ipdef->ranges[ipdef->nranges] = range;
> +        } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */
> +            memmove(ipdef->ranges + 1, ipdef->ranges,
> +                    sizeof(*ipdef->ranges) * ipdef->nranges);
> +            ipdef->ranges[0] = range;
> +        }
> +        ipdef->nranges++;
> +        memset(&range, 0, sizeof(range));
> +
> +    } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
> +
> +        if (ii == ipdef->nranges) {
> +            virReportError(VIR_ERR_OPERATION_INVALID,
> +                           _("couldn't locate a matching dhcp range entry "
> +                             "in network '%s'"), def->name);
> +            goto cleanup;
> +        }
> +
> +        /* remove it */
> +        /* NB: nothing to clear from a RangeDef that's being freed */
> +        memmove(ipdef->ranges + ii, ipdef->ranges + ii + 1,
> +                sizeof(*ipdef->ranges) * (ipdef->nranges - ii - 1));
> +        ipdef->nranges--;
> +        ignore_value(VIR_REALLOC_N(ipdef->ranges, ipdef->nranges));
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    return ret;
>  }
>  
>  static int

  That's okay, ACK, but we will need to fix the fact that passing a
random unsupported command value returns 0 and not an error.

  i will push for rc2

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]