[libvirt] [PATCHv3 00/16] Network configuration for lxc containers

Hi all, Here is a rebased version of v2. Nothing changed except the 'since' version number in the added doc that has been updated. -- Cedric Cédric Bosdonnat (16): Forgot to cleanup ifname_guest* in domain network def parsing Domain conf: allow more than one IP address for net devices LXC: set IP addresses to veth devices in the container lxc conf2xml: convert IP addresses Allow network capabilities hostdev to configure IP addresses lxc conf2xml: convert ip addresses for hostdev NICs Domain network devices can now have a <gateway> element lxc conf2xml: convert lxc.network.ipv[46].gateway LXC: use the new net devices gateway definition LXC: honour network devices link state Wrong place for virDomainNetIpsFormat virNetDevSetIPv4Address: libnl implementation Renamed virNetDevSetIPv4Address to virNetDevSetIPAddress virNetDevAddRoute: implementation using netlink virNetDevClearIPv4Address: netlink implementation Renamed virNetDevClearIPv4Address to virNetDevClearIPAddress docs/formatdomain.html.in | 39 +++ docs/schemas/domaincommon.rng | 55 +++- src/conf/domain_conf.c | 214 +++++++++++++-- src/conf/domain_conf.h | 22 +- src/libvirt_private.syms | 7 +- src/lxc/lxc_container.c | 74 ++++- src/lxc/lxc_native.c | 173 ++++++++---- src/network/bridge_driver.c | 4 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 6 +- src/qemu/qemu_driver.c | 25 +- src/qemu/qemu_hotplug.c | 6 +- src/uml/uml_conf.c | 2 +- src/util/virnetdev.c | 305 ++++++++++++++++++--- src/util/virnetdev.h | 12 +- src/util/virnetlink.c | 38 +++ src/util/virnetlink.h | 2 + src/vbox/vbox_common.c | 3 +- src/xenconfig/xen_common.c | 15 +- src/xenconfig/xen_sxpr.c | 12 +- .../lxcconf2xmldata/lxcconf2xml-physnetwork.config | 4 + tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 3 + tests/lxcconf2xmldata/lxcconf2xml-simple.config | 4 + tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 3 + tests/lxcxml2xmldata/lxc-hostdev.xml | 3 + tests/lxcxml2xmldata/lxc-idmap.xml | 3 + 26 files changed, 880 insertions(+), 156 deletions(-) -- 1.8.4.5

--- src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2810c05..b5c761f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7622,6 +7622,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(vhostuser_path); VIR_FREE(vhostuser_mode); VIR_FREE(ifname); + VIR_FREE(ifname_guest); + VIR_FREE(ifname_guest_actual); VIR_FREE(dev); virDomainActualNetDefFree(actual); VIR_FREE(script); -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:53PM +0200, Cédric Bosdonnat wrote:
--- src/conf/domain_conf.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2810c05..b5c761f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7622,6 +7622,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(vhostuser_path); VIR_FREE(vhostuser_mode); VIR_FREE(ifname); + VIR_FREE(ifname_guest); + VIR_FREE(ifname_guest_actual); VIR_FREE(dev); virDomainActualNetDefFree(actual); VIR_FREE(script);
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Add the possibility to have more than one IP address configured for a domain network interface. IP addresses can also have a prefix to define the corresponding netmask. --- docs/formatdomain.html.in | 22 +++++++ docs/schemas/domaincommon.rng | 11 +++- src/conf/domain_conf.c | 123 +++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 16 ++++- src/libvirt_private.syms | 2 + src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 6 +- src/qemu/qemu_driver.c | 25 ++++++-- src/qemu/qemu_hotplug.c | 6 +- src/uml/uml_conf.c | 2 +- src/vbox/vbox_common.c | 3 +- src/xenconfig/xen_common.c | 15 ++--- src/xenconfig/xen_sxpr.c | 12 ++-- tests/lxcxml2xmldata/lxc-idmap.xml | 2 + 14 files changed, 195 insertions(+), 52 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 0099ce7..8e3a522 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4240,6 +4240,28 @@ qemu-kvm -net nic,model=? /dev/null <span class="since">Since 0.9.5</span> </p> + <h5><a name="ipconfig">IP configuration</a></h5> +<pre> + ... + <devices> + <interface type='network'> + <source network='default'/> + <target dev='vnet0'/> + <b><ip address='192.168.122.5' prefix='24'/></b> + </interface> + </devices> + ... +</pre> + + <p> + <span class="since">Since 1.2.10</span> the network devices can be provided + zero or more IP addresses to set + on the target device. Note that some hypervisors or network device types + will simply ignore them or only use the first one. The <code>address</code> + attribute can hold either an IPv4 or IPv6 address. The <code>prefix</code> + is not mandatory since some hypervisors do not handle it. + </p> + <h5><a name="elementVhostuser">vhost-user interface</a></h5> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 20d81ae..a82d705 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2290,14 +2290,19 @@ <empty/> </element> </optional> - <optional> + <zeroOrMore> <element name="ip"> <attribute name="address"> - <ref name="ipv4Addr"/> + <ref name="ipAddr"/> </attribute> + <optional> + <attribute name="prefix"> + <ref name="ipPrefix"/> + </attribute> + </optional> <empty/> </element> - </optional> + </zeroOrMore> <optional> <element name="script"> <attribute name="path"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b5c761f..c7ab962 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1367,6 +1367,8 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def) void virDomainNetDefFree(virDomainNetDefPtr def) { + size_t i; + if (!def) return; @@ -1375,7 +1377,6 @@ void virDomainNetDefFree(virDomainNetDefPtr def) switch (def->type) { case VIR_DOMAIN_NET_TYPE_ETHERNET: VIR_FREE(def->data.ethernet.dev); - VIR_FREE(def->data.ethernet.ipaddr); break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -1396,7 +1397,6 @@ void virDomainNetDefFree(virDomainNetDefPtr def) case VIR_DOMAIN_NET_TYPE_BRIDGE: VIR_FREE(def->data.bridge.brname); - VIR_FREE(def->data.bridge.ipaddr); break; case VIR_DOMAIN_NET_TYPE_INTERNAL: @@ -1424,6 +1424,10 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def->ifname_guest); VIR_FREE(def->ifname_guest_actual); + for (i = 0; i < def->nips; i++) + virDomainNetIpDefFree(def->ips[i]); + VIR_FREE(def->ips); + virDomainDeviceInfoClear(&def->info); VIR_FREE(def->filter); @@ -1435,6 +1439,12 @@ void virDomainNetDefFree(virDomainNetDefPtr def) VIR_FREE(def); } +void virDomainNetIpDefFree(virDomainNetIpDefPtr def) +{ + VIR_FREE(def->address); + VIR_FREE(def); +} + void ATTRIBUTE_NONNULL(1) virDomainChrSourceDefClear(virDomainChrSourceDefPtr def) { @@ -6927,6 +6937,29 @@ virDomainActualNetDefParseXML(xmlNodePtr node, #define NET_MODEL_CHARS \ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_-" + +int +virDomainNetAppendIpAddress(virDomainNetDefPtr def, + const char *address, + unsigned int prefix) +{ + virDomainNetIpDefPtr ipDef = NULL; + if (VIR_ALLOC(ipDef) < 0) + return -1; + + if (VIR_STRDUP(ipDef->address, address) < 0) + return -1; + + ipDef->prefix = prefix; + + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ipDef) < 0) { + virDomainNetIpDefFree(ipDef); + return -1; + } + + return 0; +} + /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition * @return 0 on success, -1 on failure @@ -6974,6 +7007,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, virDomainActualNetDefPtr actual = NULL; xmlNodePtr oldnode = ctxt->node; int ret, val; + unsigned int prefix = 0; + size_t i; + size_t nips = 0; + virDomainNetIpDefPtr *ips = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -7062,11 +7099,44 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { address = virXMLPropString(cur, "address"); port = virXMLPropString(cur, "port"); - } else if (!address && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && - xmlStrEqual(cur->name, BAD_CAST "ip")) { - address = virXMLPropString(cur, "address"); + } else if (xmlStrEqual(cur->name, BAD_CAST "ip")) { + /* Parse the prefix in every case */ + char *prefixStr = NULL; + unsigned int prefixValue = 0; + + if ((prefixStr = virXMLPropString(cur, "prefix")) && + (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid network prefix: '%s'"), + prefixStr); + VIR_FREE(prefixStr); + goto error; + } + VIR_FREE(prefixStr); + + /* Previous behavior: make sure this address it the first one + in the resulting list */ + if (!address && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + + address = virXMLPropString(cur, "address"); + prefix = prefixValue; + } else { + /* All other <ip/> elements will be added after */ + virDomainNetIpDefPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + goto error; + + ip->address = virXMLPropString(cur, "address"); + ip->prefix = prefixValue; + + if (ip->address != NULL && + VIR_APPEND_ELEMENT(ips, nips, ip) < 0) + goto error; + } } else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -7267,8 +7337,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, dev = NULL; } if (address != NULL) { - def->data.ethernet.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); } break; @@ -7282,8 +7352,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.bridge.brname = bridge; bridge = NULL; if (address != NULL) { - def->data.bridge.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); } break; @@ -7381,6 +7451,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break; } + for (i = 0; i < nips; i++) { + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ips[i]) < 0) + goto error; + } + if (script != NULL) { def->script = script; script = NULL; @@ -7643,6 +7718,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(linkstate); VIR_FREE(addrtype); VIR_FREE(trustGuestRxFilters); + VIR_FREE(ips); virNWFilterHashTableFree(filterparams); return def; @@ -16631,6 +16707,21 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, return 0; } +static void +virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) +{ + size_t i; + + /* Output IP addresses */ + for (i = 0; i < nips; i++) { + virBufferAsprintf(buf, "<ip address='%s'", + ips[i]->address); + if (ips[i]->prefix != 0) + virBufferAsprintf(buf, " prefix='%u'", ips[i]->prefix); + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainHostdevDefFormatCaps(virBufferPtr buf, virDomainHostdevDefPtr def) @@ -16736,7 +16827,6 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf, return 0; } - /* virDomainActualNetDefFormat() - format the ActualNetDef * info inside an <actual> element, as required for internal storage * of domain status @@ -16973,9 +17063,6 @@ virDomainNetDefFormat(virBufferPtr buf, case VIR_DOMAIN_NET_TYPE_ETHERNET: virBufferEscapeString(buf, "<source dev='%s'/>\n", def->data.ethernet.dev); - if (def->data.ethernet.ipaddr) - virBufferAsprintf(buf, "<ip address='%s'/>\n", - def->data.ethernet.ipaddr); break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: @@ -16992,10 +17079,6 @@ virDomainNetDefFormat(virBufferPtr buf, case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferEscapeString(buf, "<source bridge='%s'/>\n", def->data.bridge.brname); - if (def->data.bridge.ipaddr) { - virBufferAsprintf(buf, "<ip address='%s'/>\n", - def->data.bridge.ipaddr); - } break; case VIR_DOMAIN_NET_TYPE_SERVER: @@ -17043,6 +17126,8 @@ virDomainNetDefFormat(virBufferPtr buf, return -1; } + virDomainNetIpsFormat(buf, def->ips, def->nips); + virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); if (def->ifname && diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index afa3da6..6ecf639 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -471,6 +471,15 @@ typedef enum { VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST } virDomainHostdevCapsType; +typedef struct _virDomainNetIpDef virDomainNetIpDef; +typedef virDomainNetIpDef *virDomainNetIpDefPtr; +struct _virDomainNetIpDef { + char *address; /* ipv4 or ipv6 address */ + unsigned int prefix; /* number of 1 bits in the net mask */ +}; + +void virDomainNetIpDefFree(virDomainNetIpDefPtr def); + typedef struct _virDomainHostdevCaps virDomainHostdevCaps; typedef virDomainHostdevCaps *virDomainHostdevCapsPtr; struct _virDomainHostdevCaps { @@ -925,7 +934,6 @@ struct _virDomainNetDef { union { struct { char *dev; - char *ipaddr; } ethernet; virDomainChrSourceDefPtr vhostuser; struct { @@ -947,7 +955,6 @@ struct _virDomainNetDef { } network; struct { char *brname; - char *ipaddr; } bridge; struct { char *name; @@ -977,6 +984,8 @@ struct _virDomainNetDef { virNetDevVlan vlan; int trustGuestRxFilters; /* enum virTristateBool */ int linkstate; + size_t nips; + virDomainNetIpDefPtr *ips; }; /* Used for prefix of ifname of any network name generated dynamically @@ -2513,6 +2522,9 @@ virNetDevBandwidthPtr virDomainNetGetActualBandwidth(virDomainNetDefPtr iface); virNetDevVlanPtr virDomainNetGetActualVlan(virDomainNetDefPtr iface); bool virDomainNetGetActualTrustGuestRxFilters(virDomainNetDefPtr iface); +int virDomainNetAppendIpAddress(virDomainNetDefPtr def, + const char *address, + unsigned int prefix); int virDomainControllerInsert(virDomainDefPtr def, virDomainControllerDefPtr controller) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d6265ac..b55bf35 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -321,6 +321,7 @@ virDomainLockFailureTypeFromString; virDomainLockFailureTypeToString; virDomainMemballoonModelTypeFromString; virDomainMemballoonModelTypeToString; +virDomainNetAppendIpAddress; virDomainNetDefFormat; virDomainNetDefFree; virDomainNetFind; @@ -336,6 +337,7 @@ virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; virDomainNetInsert; +virDomainNetIpDefFree; virDomainNetRemove; virDomainNetRemoveHostdev; virDomainNetTypeToString; diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 856c9f5..ea45e96 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -230,7 +230,7 @@ openvzReadNetworkConf(virDomainDefPtr def, goto error; net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; - if (VIR_STRDUP(net->data.ethernet.ipaddr, token) < 0) + if (virDomainNetAppendIpAddress(net, token, 0) < 0) goto error; if (VIR_APPEND_ELEMENT_COPY(def->nets, def->nnets, net) < 0) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b62273a..a0dfbee 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -855,7 +855,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && - net->data.ethernet.ipaddr == NULL)) { + net->nips == 0)) { virBuffer buf = VIR_BUFFER_INITIALIZER; int veid = openvzGetVEID(vpsid); @@ -906,9 +906,9 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, virCommandAddArg(cmd, "--netif_add"); virCommandAddArgBuffer(cmd, &buf); } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && - net->data.ethernet.ipaddr != NULL) { + net->nips > 0 && net->ips[0]->address != NULL) { /* --ipadd ip */ - virCommandAddArgList(cmd, "--ipadd", net->data.ethernet.ipaddr, NULL); + virCommandAddArgList(cmd, "--ipadd", net->ips[0]->address, NULL); } /* TODO: processing NAT and physical device */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7c9b1ab..6da9ceb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6261,6 +6261,8 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, (brname = virDomainNetGetActualBridgeName(net))) { char *brnamecopy; + size_t j; + if (VIR_STRDUP(brnamecopy, brname) < 0) goto cleanup; @@ -6271,20 +6273,31 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->script = NULL; net->data.ethernet.dev = brnamecopy; - net->data.ethernet.ipaddr = NULL; + for (j = 0; j < net->nips; j++) { + virDomainNetIpDefFree(net->ips[j]); + } + VIR_FREE(net->ips); + net->nips = 0; + } else { /* actualType is either NETWORK or DIRECT. In either * case, the best we can do is NULL everything out. */ + size_t j; virDomainActualNetDefFree(net->data.network.actual); memset(net, 0, sizeof(*net)); net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->script = NULL; net->data.ethernet.dev = NULL; - net->data.ethernet.ipaddr = NULL; + for (j = 0; j < net->nips; j++) { + virDomainNetIpDefFree(net->ips[j]); + } + VIR_FREE(net->ips); + net->nips = 0; } } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { + size_t j; VIR_FREE(net->data.direct.linkdev); memset(net, 0, sizeof(*net)); @@ -6292,18 +6305,20 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->script = NULL; net->data.ethernet.dev = NULL; - net->data.ethernet.ipaddr = NULL; + for (j = 0; j < net->nips; j++) { + virDomainNetIpDefFree(net->ips[j]); + } + VIR_FREE(net->ips); + net->nips = 0; } else if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { char *script = net->script; char *brname = net->data.bridge.brname; - char *ipaddr = net->data.bridge.ipaddr; memset(net, 0, sizeof(*net)); net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; net->script = script; net->data.ethernet.dev = brname; - net->data.ethernet.ipaddr = ipaddr; } VIR_FREE(net->virtPortProfile); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e504ec..3b31b77 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2090,8 +2090,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, newdev->data.ethernet.dev) || - STRNEQ_NULLABLE(olddev->data.ethernet.ipaddr, - newdev->data.ethernet.ipaddr)) { + STRNEQ_NULLABLE(olddev->nips > 0 ? olddev->ips[0]->address + : "", + newdev->nips > 0 ? newdev->ips[0]->address + : "")) { needReconnect = true; } break; diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index a99e8e9..ac6a7da 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -175,7 +175,7 @@ umlBuildCommandLineNet(virConnectPtr conn, if (def->ifname) { virBufferAdd(&buf, def->ifname, -1); } - if (def->data.ethernet.ipaddr) { + if (def->nips > 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("IP address not supported for ethernet interface")); goto error; diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 44270ff..ffc450f 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1322,7 +1322,8 @@ vboxAttachNetwork(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine) } else if (def->nets[i]->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { VIR_DEBUG("NIC(%zu): brname: %s", i, def->nets[i]->data.bridge.brname); VIR_DEBUG("NIC(%zu): script: %s", i, def->nets[i]->script); - VIR_DEBUG("NIC(%zu): ipaddr: %s", i, def->nets[i]->data.bridge.ipaddr); + if (def->nets[i]->nips > 0) + VIR_DEBUG("NIC(%zu): ipaddr: %s", i, def->nets[i]->ips[0]->address); } gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter); diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 32954f3..8347cf7 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -922,12 +922,9 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) goto cleanup; - if (ip[0] && VIR_STRDUP(net->data.bridge.ipaddr, ip) < 0) - goto cleanup; - } else { - if (ip[0] && VIR_STRDUP(net->data.ethernet.ipaddr, ip) < 0) - goto cleanup; } + if (ip[0] && virDomainNetAppendIpAddress(net, ip, 0) < 0) + goto cleanup; if (script && script[0] && VIR_STRDUP(net->script, script) < 0) @@ -1225,16 +1222,16 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferAsprintf(&buf, ",bridge=%s", net->data.bridge.brname); - if (net->data.bridge.ipaddr) - virBufferAsprintf(&buf, ",ip=%s", net->data.bridge.ipaddr); + if (net->nips > 0) + virBufferAsprintf(&buf, ",ip=%s", net->ips[0]->address); virBufferAsprintf(&buf, ",script=%s", DEFAULT_VIF_SCRIPT); break; case VIR_DOMAIN_NET_TYPE_ETHERNET: if (net->script) virBufferAsprintf(&buf, ",script=%s", net->script); - if (net->data.ethernet.ipaddr) - virBufferAsprintf(&buf, ",ip=%s", net->data.ethernet.ipaddr); + if (net->nips > 0) + virBufferAsprintf(&buf, ",ip=%s", net->ips[0]->address); break; case VIR_DOMAIN_NET_TYPE_NETWORK: diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index 6623ea8..8e64afb 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -565,14 +565,14 @@ xenParseSxprNets(virDomainDefPtr def, VIR_STRDUP(net->script, tmp2) < 0) goto cleanup; tmp = sexpr_node(node, "device/vif/ip"); - if (VIR_STRDUP(net->data.bridge.ipaddr, tmp) < 0) + if (tmp && virDomainNetAppendIpAddress(net, tmp, 0) < 0) goto cleanup; } else { net->type = VIR_DOMAIN_NET_TYPE_ETHERNET; if (VIR_STRDUP(net->script, tmp2) < 0) goto cleanup; tmp = sexpr_node(node, "device/vif/ip"); - if (VIR_STRDUP(net->data.ethernet.ipaddr, tmp) < 0) + if (tmp && virDomainNetAppendIpAddress(net, tmp, 0) < 0) goto cleanup; } @@ -1898,8 +1898,8 @@ xenFormatSxprNet(virConnectPtr conn, script = def->script; virBufferEscapeSexpr(buf, "(script '%s')", script); - if (def->data.bridge.ipaddr != NULL) - virBufferEscapeSexpr(buf, "(ip '%s')", def->data.bridge.ipaddr); + if (def->nips > 0) + virBufferEscapeSexpr(buf, "(ip '%s')", def->ips[0]->address); break; case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -1932,8 +1932,8 @@ xenFormatSxprNet(virConnectPtr conn, if (def->script) virBufferEscapeSexpr(buf, "(script '%s')", def->script); - if (def->data.ethernet.ipaddr != NULL) - virBufferEscapeSexpr(buf, "(ip '%s')", def->data.ethernet.ipaddr); + if (def->nips > 0) + virBufferEscapeSexpr(buf, "(ip '%s')", def->ips[0]->address); break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: diff --git a/tests/lxcxml2xmldata/lxc-idmap.xml b/tests/lxcxml2xmldata/lxc-idmap.xml index 946d363..a52da0b 100644 --- a/tests/lxcxml2xmldata/lxc-idmap.xml +++ b/tests/lxcxml2xmldata/lxc-idmap.xml @@ -28,6 +28,8 @@ <interface type='bridge'> <mac address='00:16:3e:0f:ef:8a'/> <source bridge='bri0'/> + <ip address='192.168.122.12' prefix='24'/> + <ip address='192.168.122.13' prefix='24'/> <target dev='veth0'/> <guest dev='eth2'/> </interface> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:54PM +0200, Cédric Bosdonnat wrote:
Add the possibility to have more than one IP address configured for a domain network interface. IP addresses can also have a prefix to define the corresponding netmask. --- docs/formatdomain.html.in | 22 +++++++ docs/schemas/domaincommon.rng | 11 +++- src/conf/domain_conf.c | 123 +++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 16 ++++- src/libvirt_private.syms | 2 + src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 6 +- src/qemu/qemu_driver.c | 25 ++++++-- src/qemu/qemu_hotplug.c | 6 +- src/uml/uml_conf.c | 2 +- src/vbox/vbox_common.c | 3 +- src/xenconfig/xen_common.c | 15 ++--- src/xenconfig/xen_sxpr.c | 12 ++-- tests/lxcxml2xmldata/lxc-idmap.xml | 2 + 14 files changed, 195 insertions(+), 52 deletions(-)
I think it is probably worth a followup patch to make drivers report VIR_ERR_CONFIG_UNSUPPORTED in the case where nips > 1 and the driver only supports nips==1. Ideally we'd also have drivers report an error for the network types they don't support IPs with, but that's much more work and we don't even have good reporting of errors for the network types themselves, let alone IP addrs. So we can ignore that for now.
+int +virDomainNetAppendIpAddress(virDomainNetDefPtr def, + const char *address, + unsigned int prefix) +{ + virDomainNetIpDefPtr ipDef = NULL; + if (VIR_ALLOC(ipDef) < 0) + return -1; + + if (VIR_STRDUP(ipDef->address, address) < 0) + return -1;
Oh, you leak ipDef on OOM here.
+ + ipDef->prefix = prefix; + + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ipDef) < 0) { + virDomainNetIpDefFree(ipDef); + return -1; + } + + return 0; +}
@@ -7062,11 +7099,44 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { address = virXMLPropString(cur, "address"); port = virXMLPropString(cur, "port"); - } else if (!address && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && - xmlStrEqual(cur->name, BAD_CAST "ip")) { - address = virXMLPropString(cur, "address"); + } else if (xmlStrEqual(cur->name, BAD_CAST "ip")) { + /* Parse the prefix in every case */ + char *prefixStr = NULL; + unsigned int prefixValue = 0; + + if ((prefixStr = virXMLPropString(cur, "prefix")) && + (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid network prefix: '%s'"), + prefixStr); + VIR_FREE(prefixStr); + goto error; + } + VIR_FREE(prefixStr); + + /* Previous behavior: make sure this address it the first one + in the resulting list */ + if (!address && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + + address = virXMLPropString(cur, "address"); + prefix = prefixValue; + } else { + /* All other <ip/> elements will be added after */ + virDomainNetIpDefPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + goto error; + + ip->address = virXMLPropString(cur, "address"); + ip->prefix = prefixValue; + + if (ip->address != NULL && + VIR_APPEND_ELEMENT(ips, nips, ip) < 0) + goto error; + }
I'm not sure I understand why you have this if/else here. You are parsing the addresses in order in the XML, and appending to the list of IP address, so sure the first address will always be first in the list and so there's no need for the if/else.
} else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -7267,8 +7337,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, dev = NULL; } if (address != NULL) { - def->data.ethernet.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); }
This actually causes the address to be put at the end of the list surely ?
break;
@@ -7282,8 +7352,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.bridge.brname = bridge; bridge = NULL; if (address != NULL) { - def->data.bridge.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); }
And this.
break;
@@ -7381,6 +7451,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break; }
+ for (i = 0; i < nips; i++) { + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ips[i]) < 0) + goto error; + }
Why not just assign 'def->ips = ips' and def->nips = nips, instead of doing more memory allocation in a loop here ?
+ if (script != NULL) { def->script = script; script = NULL; @@ -7643,6 +7718,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(linkstate); VIR_FREE(addrtype); VIR_FREE(trustGuestRxFilters); + VIR_FREE(ips); virNWFilterHashTableFree(filterparams);
return def; @@ -16631,6 +16707,21 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, return 0; }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index afa3da6..6ecf639 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -471,6 +471,15 @@ typedef enum { VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST } virDomainHostdevCapsType;
+typedef struct _virDomainNetIpDef virDomainNetIpDef; +typedef virDomainNetIpDef *virDomainNetIpDefPtr; +struct _virDomainNetIpDef { + char *address; /* ipv4 or ipv6 address */ + unsigned int prefix; /* number of 1 bits in the net mask */ +};
This ought to really use virSocketAddr, but perhaps you do that later in this patch series anyway.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e504ec..3b31b77 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2090,8 +2090,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, newdev->data.ethernet.dev) || - STRNEQ_NULLABLE(olddev->data.ethernet.ipaddr, - newdev->data.ethernet.ipaddr)) { + STRNEQ_NULLABLE(olddev->nips > 0 ? olddev->ips[0]->address + : "", + newdev->nips > 0 ? newdev->ips[0]->address + : "")) {
I think could just use NULL instead of "" here. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hi Daniel, On Wed, 2014-10-22 at 11:03 +0100, Daniel P. Berrange wrote:
On Fri, Oct 10, 2014 at 02:03:54PM +0200, Cédric Bosdonnat wrote:
Add the possibility to have more than one IP address configured for a domain network interface. IP addresses can also have a prefix to define the corresponding netmask. --- docs/formatdomain.html.in | 22 +++++++ docs/schemas/domaincommon.rng | 11 +++- src/conf/domain_conf.c | 123 +++++++++++++++++++++++++++++++------ src/conf/domain_conf.h | 16 ++++- src/libvirt_private.syms | 2 + src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 6 +- src/qemu/qemu_driver.c | 25 ++++++-- src/qemu/qemu_hotplug.c | 6 +- src/uml/uml_conf.c | 2 +- src/vbox/vbox_common.c | 3 +- src/xenconfig/xen_common.c | 15 ++--- src/xenconfig/xen_sxpr.c | 12 ++-- tests/lxcxml2xmldata/lxc-idmap.xml | 2 + 14 files changed, 195 insertions(+), 52 deletions(-)
I think it is probably worth a followup patch to make drivers report VIR_ERR_CONFIG_UNSUPPORTED in the case where nips > 1 and the driver only supports nips==1.
I'll add those errors indeed.
Ideally we'd also have drivers report an error for the network types they don't support IPs with, but that's much more work and we don't even have good reporting of errors for the network types themselves, let alone IP addrs. So we can ignore that for now.
Ok, but good to keep that in mind, indeed.
+int +virDomainNetAppendIpAddress(virDomainNetDefPtr def, + const char *address, + unsigned int prefix) +{ + virDomainNetIpDefPtr ipDef = NULL; + if (VIR_ALLOC(ipDef) < 0) + return -1; + + if (VIR_STRDUP(ipDef->address, address) < 0) + return -1;
Oh, you leak ipDef on OOM here.
Nice catch!
+ + ipDef->prefix = prefix; + + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ipDef) < 0) { + virDomainNetIpDefFree(ipDef); + return -1; + } + + return 0; +}
@@ -7062,11 +7099,44 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, xmlStrEqual(cur->name, BAD_CAST "source")) { address = virXMLPropString(cur, "address"); port = virXMLPropString(cur, "port"); - } else if (!address && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - def->type == VIR_DOMAIN_NET_TYPE_BRIDGE) && - xmlStrEqual(cur->name, BAD_CAST "ip")) { - address = virXMLPropString(cur, "address"); + } else if (xmlStrEqual(cur->name, BAD_CAST "ip")) { + /* Parse the prefix in every case */ + char *prefixStr = NULL; + unsigned int prefixValue = 0; + + if ((prefixStr = virXMLPropString(cur, "prefix")) && + (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid network prefix: '%s'"), + prefixStr); + VIR_FREE(prefixStr); + goto error; + } + VIR_FREE(prefixStr); + + /* Previous behavior: make sure this address it the first one + in the resulting list */ + if (!address && + (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || + def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)) { + + address = virXMLPropString(cur, "address"); + prefix = prefixValue; + } else { + /* All other <ip/> elements will be added after */ + virDomainNetIpDefPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + goto error; + + ip->address = virXMLPropString(cur, "address"); + ip->prefix = prefixValue; + + if (ip->address != NULL && + VIR_APPEND_ELEMENT(ips, nips, ip) < 0) + goto error; + }
I'm not sure I understand why you have this if/else here. You are parsing the addresses in order in the XML, and appending to the list of IP address, so sure the first address will always be first in the list and so there's no need for the if/else.
The content of address is written before the content of ips in the list. And the problem comes with the fact that some network types are using an 'address' attribute to store the IP.
} else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -7267,8 +7337,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, dev = NULL; } if (address != NULL) { - def->data.ethernet.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); }
This actually causes the address to be put at the end of the list surely ?
As the list is still empty at that time, this address will be in the first position
break;
@@ -7282,8 +7352,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.bridge.brname = bridge; bridge = NULL; if (address != NULL) { - def->data.bridge.ipaddr = address; - address = NULL; + virDomainNetAppendIpAddress(def, address, prefix); + VIR_FREE(address); }
And this.
same here
break;
@@ -7381,6 +7451,11 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, break; }
+ for (i = 0; i < nips; i++) { + if (VIR_APPEND_ELEMENT(def->ips, def->nips, ips[i]) < 0) + goto error; + }
Why not just assign 'def->ips = ips' and def->nips = nips, instead of doing more memory allocation in a loop here ?
Because we may already have an address in def->ips due to the other things mentioned before.
+ if (script != NULL) { def->script = script; script = NULL; @@ -7643,6 +7718,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(linkstate); VIR_FREE(addrtype); VIR_FREE(trustGuestRxFilters); + VIR_FREE(ips); virNWFilterHashTableFree(filterparams);
return def; @@ -16631,6 +16707,21 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, return 0; }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index afa3da6..6ecf639 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -471,6 +471,15 @@ typedef enum { VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST } virDomainHostdevCapsType;
+typedef struct _virDomainNetIpDef virDomainNetIpDef; +typedef virDomainNetIpDef *virDomainNetIpDefPtr; +struct _virDomainNetIpDef { + char *address; /* ipv4 or ipv6 address */ + unsigned int prefix; /* number of 1 bits in the net mask */ +};
This ought to really use virSocketAddr, but perhaps you do that later in this patch series anyway.
No, but I'll switch to it, indeed.
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1e504ec..3b31b77 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2090,8 +2090,10 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, newdev->data.ethernet.dev) || - STRNEQ_NULLABLE(olddev->data.ethernet.ipaddr, - newdev->data.ethernet.ipaddr)) { + STRNEQ_NULLABLE(olddev->nips > 0 ? olddev->ips[0]->address + : "", + newdev->nips > 0 ? newdev->ips[0]->address + : "")) {
I think could just use NULL instead of "" here.
Indeed, I can't even remember why I put "". Regards, -- Cedric

Hi Daniel, On Wed, 2014-10-22 at 11:03 +0100, Daniel P. Berrange wrote:
I think it is probably worth a followup patch to make drivers report VIR_ERR_CONFIG_UNSUPPORTED in the case where nips > 1 and the driver only supports nips==1.
Shouldn't we just VIR_WARN this? That would avoid breaking as we could get the first ip address... -- Cedric

On Tue, Nov 04, 2014 at 01:43:58PM +0100, Cedric Bosdonnat wrote:
Hi Daniel,
On Wed, 2014-10-22 at 11:03 +0100, Daniel P. Berrange wrote:
I think it is probably worth a followup patch to make drivers report VIR_ERR_CONFIG_UNSUPPORTED in the case where nips > 1 and the driver only supports nips==1.
Shouldn't we just VIR_WARN this? That would avoid breaking as we could get the first ip address...
No, it is policy that if we don't support something we must always return an error to the application. If we only warned, then there is no way for an application to detect if the feature works or not. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Uses the new virDomainNetDef ips to set the IP addresses on the network interfaces in the container. --- src/lxc/lxc_container.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2af2674..608d39f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -496,7 +496,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, char **veths) { int rc = 0; - size_t i; + size_t i, j; char *newname = NULL; virDomainNetDefPtr netDef; bool privNet = vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] == @@ -517,6 +517,28 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, if (rc < 0) goto error_out; + for (j = 0; j < netDef->nips; j++) { + virDomainNetIpDefPtr ip = netDef->ips[j]; + unsigned int prefix = (ip->prefix > 0) ? ip->prefix : 24; + virSocketAddr address; + int family = AF_INET; + + if (strchr(ip->address, ':')) + family = AF_INET6; + + if (virSocketAddrParse(&address, ip->address, family) < 0) + goto error_out; + + VIR_DEBUG("Adding IP address '%s/%u' to '%s'", + ip->address, ip->prefix, newname); + if (virNetDevSetIPv4Address(newname, &address, prefix) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Failed to set IP address '%s' on %s"), + ip->address, newname); + goto error_out; + } + } + VIR_DEBUG("Enabling %s", newname); rc = virNetDevSetOnline(newname, true); if (rc < 0) -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:55PM +0200, Cédric Bosdonnat wrote:
Uses the new virDomainNetDef ips to set the IP addresses on the network interfaces in the container. --- src/lxc/lxc_container.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2af2674..608d39f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -496,7 +496,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, char **veths) { int rc = 0; - size_t i; + size_t i, j; char *newname = NULL; virDomainNetDefPtr netDef; bool privNet = vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] == @@ -517,6 +517,28 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, if (rc < 0) goto error_out;
+ for (j = 0; j < netDef->nips; j++) { + virDomainNetIpDefPtr ip = netDef->ips[j]; + unsigned int prefix = (ip->prefix > 0) ? ip->prefix : 24; + virSocketAddr address; + int family = AF_INET; + + if (strchr(ip->address, ':')) + family = AF_INET6; + + if (virSocketAddrParse(&address, ip->address, family) < 0) + goto error_out;
Hmm, so this again makes me think we should have another patch in the series just before this, but after the first XML patch, which converts the virDomainConf to use virSocketAddr directly. I think it would be good practice for us to extend the XML to include teh address family too, instead of just doing strchr for ':' to detect IPv6.
+ + VIR_DEBUG("Adding IP address '%s/%u' to '%s'", + ip->address, ip->prefix, newname); + if (virNetDevSetIPv4Address(newname, &address, prefix) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Failed to set IP address '%s' on %s"), + ip->address, newname); + goto error_out; + } + } +
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 10/22/2014 06:05 AM, Daniel P. Berrange wrote:
On Fri, Oct 10, 2014 at 02:03:55PM +0200, Cédric Bosdonnat wrote:
Uses the new virDomainNetDef ips to set the IP addresses on the network interfaces in the container. --- src/lxc/lxc_container.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 2af2674..608d39f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -496,7 +496,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, char **veths) { int rc = 0; - size_t i; + size_t i, j; char *newname = NULL; virDomainNetDefPtr netDef; bool privNet = vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] == @@ -517,6 +517,28 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, if (rc < 0) goto error_out;
+ for (j = 0; j < netDef->nips; j++) { + virDomainNetIpDefPtr ip = netDef->ips[j]; + unsigned int prefix = (ip->prefix > 0) ? ip->prefix : 24; + virSocketAddr address; + int family = AF_INET; + + if (strchr(ip->address, ':')) + family = AF_INET6; + + if (virSocketAddrParse(&address, ip->address, family) < 0) + goto error_out;
Hmm, so this again makes me think we should have another patch in the series just before this, but after the first XML patch, which converts the virDomainConf to use virSocketAddr directly. I think it would be good practice for us to extend the XML to include teh address family too, instead of just doing strchr for ':' to detect IPv6.
FYI: See virSocketAddrNumericFamily
+ + VIR_DEBUG("Adding IP address '%s/%u' to '%s'", + ip->address, ip->prefix, newname); + if (virNetDevSetIPv4Address(newname, &address, prefix) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Failed to set IP address '%s' on %s"), + ip->address, newname); + goto error_out; + } + } +
Regards, Daniel

--- src/lxc/lxc_native.c | 153 ++++++++++++++++-------- tests/lxcconf2xmldata/lxcconf2xml-simple.config | 2 + tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 2 + 3 files changed, 106 insertions(+), 51 deletions(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 41e069f..f695a00 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -413,93 +413,124 @@ lxcCreateHostdevDef(int mode, int type, const char *data) return hostdev; } +typedef struct { + virDomainDefPtr def; + char *type; + char *link; + char *mac; + char *flag; + char *macvlanmode; + char *vlanid; + char *name; + char **ips; + size_t nips; + bool privnet; + size_t networks; +} lxcNetworkParseData; + static int -lxcAddNetworkDefinition(virDomainDefPtr def, - const char *type, - const char *linkdev, - const char *mac, - const char *flag, - const char *macvlanmode, - const char *vlanid, - const char *name) +lxcAddNetworkDefinition(lxcNetworkParseData *data) { virDomainNetDefPtr net = NULL; virDomainHostdevDefPtr hostdev = NULL; bool isPhys, isVlan = false; + size_t nips = 0; + virDomainNetIpDefPtr *ips = NULL; + virDomainNetIpDefPtr ip = NULL; + char **ipparts = NULL; + size_t i; - if ((type == NULL) || STREQ(type, "empty") || STREQ(type, "") || - STREQ(type, "none")) + if ((data->type == NULL) || STREQ(data->type, "empty") || + STREQ(data->type, "") || STREQ(data->type, "none")) return 0; - isPhys = STREQ(type, "phys"); - isVlan = STREQ(type, "vlan"); - if (type != NULL && (isPhys || isVlan)) { - if (!linkdev) { + /* Add the IP addresses */ + for (i = 0; i < data->nips; i++) { + if (VIR_ALLOC(ip) < 0) + goto error; + + ipparts = virStringSplit(data->ips[i], "/", 2); + if (virStringListLength(ipparts) != 2 || + strlen(ipparts[0]) == 0 || + virStrToLong_ui(ipparts[1], NULL, 10, &ip->prefix) < 0) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid CIDR address: '%s'"), data->ips[i]); + goto error; + } + + if (VIR_STRDUP(ip->address, ipparts[0]) < 0) + goto error; + + if (VIR_APPEND_ELEMENT(ips, nips, ip) < 0) + goto error; + + virStringFreeList(ipparts); + } + + isPhys = STREQ(data->type, "phys"); + isVlan = STREQ(data->type, "vlan"); + if (data->type != NULL && (isPhys || isVlan)) { + if (!data->link) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Missing 'link' attribute for NIC")); goto error; } if (!(hostdev = lxcCreateHostdevDef(VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET, - linkdev))) + data->link))) goto error; /* This still requires the user to manually setup the vlan interface * on the host */ - if (isVlan && vlanid) { + if (isVlan && data->vlanid) { VIR_FREE(hostdev->source.caps.u.net.iface); if (virAsprintf(&hostdev->source.caps.u.net.iface, - "%s.%s", linkdev, vlanid) < 0) + "%s.%s", data->link, data->vlanid) < 0) goto error; } - if (VIR_EXPAND_N(def->hostdevs, def->nhostdevs, 1) < 0) + if (VIR_EXPAND_N(data->def->hostdevs, data->def->nhostdevs, 1) < 0) goto error; - def->hostdevs[def->nhostdevs - 1] = hostdev; + data->def->hostdevs[data->def->nhostdevs - 1] = hostdev; } else { - if (!(net = lxcCreateNetDef(type, linkdev, mac, flag, macvlanmode, name))) + if (!(net = lxcCreateNetDef(data->type, data->link, data->mac, + data->flag, data->macvlanmode, + data->name))) goto error; - if (VIR_EXPAND_N(def->nets, def->nnets, 1) < 0) + net->ips = ips; + net->nips = nips; + + if (VIR_EXPAND_N(data->def->nets, data->def->nnets, 1) < 0) goto error; - def->nets[def->nnets - 1] = net; + data->def->nets[data->def->nnets - 1] = net; } return 1; error: + for (i = 0; i < nips; i++) { + virDomainNetIpDefFree(ips[i]); + } + VIR_FREE(ips); + virStringFreeList(ipparts); + virDomainNetIpDefFree(ip); virDomainNetDefFree(net); virDomainHostdevDefFree(hostdev); return -1; } -typedef struct { - virDomainDefPtr def; - char *type; - char *link; - char *mac; - char *flag; - char *macvlanmode; - char *vlanid; - char *name; - bool privnet; - size_t networks; -} lxcNetworkParseData; - static int lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) { lxcNetworkParseData *parseData = data; int status; + size_t i; if (STREQ(name, "lxc.network.type")) { /* Store the previous NIC */ - status = lxcAddNetworkDefinition(parseData->def, parseData->type, - parseData->link, parseData->mac, - parseData->flag, - parseData->macvlanmode, - parseData->vlanid, - parseData->name); + status = lxcAddNetworkDefinition(parseData); if (status < 0) return -1; @@ -517,6 +548,12 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) parseData->vlanid = NULL; parseData->name = NULL; + /* IPs array needs to be free'd as all IPs are dup'ed there */ + for (i = 0; i < parseData->nips; i++) + VIR_FREE(parseData->ips[i]); + VIR_FREE(parseData->ips); + parseData->nips = 0; + /* Keep the new value */ parseData->type = value->str; } @@ -532,7 +569,14 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) parseData->vlanid = value->str; else if (STREQ(name, "lxc.network.name")) parseData->name = value->str; - else if (STRPREFIX(name, "lxc.network")) + else if (STREQ(name, "lxc.network.ipv4") || + STREQ(name, "lxc.network.ipv6")) { + + if (VIR_EXPAND_N(parseData->ips, parseData->nips, 1) < 0) + return -1; + if (VIR_STRDUP(parseData->ips[parseData->nips - 1], value->str) < 0) + return -1; + } else if (STRPREFIX(name, "lxc.network")) VIR_WARN("Unhandled network property: %s = %s", name, value->str); @@ -544,19 +588,20 @@ static int lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) { int status; + int result = -1; + size_t i; lxcNetworkParseData data = {def, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, true, 0}; + NULL, NULL, NULL, NULL, 0, true, 0}; + + if (virConfWalk(properties, lxcNetworkWalkCallback, &data) < 0) + goto cleanup; - virConfWalk(properties, lxcNetworkWalkCallback, &data); /* Add the last network definition found */ - status = lxcAddNetworkDefinition(def, data.type, data.link, - data.mac, data.flag, - data.macvlanmode, - data.vlanid, - data.name); + status = lxcAddNetworkDefinition(&data); + if (status < 0) - return -1; + goto cleanup; else if (status > 0) data.networks++; else if (data.type != NULL && STREQ(data.type, "none")) @@ -566,8 +611,14 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) /* When no network type is provided LXC only adds loopback */ def->features[VIR_DOMAIN_FEATURE_PRIVNET] = VIR_TRISTATE_SWITCH_ON; } + result = 0; - return 0; + cleanup: + for (i = 0; i < data.nips; i++) + VIR_FREE(data.ips[i]); + VIR_FREE(data.ips); + + return result; } static int diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.config b/tests/lxcconf2xmldata/lxcconf2xml-simple.config index b90abc1..d417ba0 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.config +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.config @@ -6,6 +6,8 @@ lxc.network.flags = up lxc.network.link = virbr0 lxc.network.hwaddr = 02:00:15:8f:05:c1 lxc.network.name = eth0 +lxc.network.ipv4 = 192.168.122.2/24 +lxc.network.ipv6 = 2003:db8:1:0:214:1234:fe0b:3596/64 #remove next line if host DNS configuration should not be available to container lxc.mount.entry = proc proc proc nodev,noexec,nosuid 0 0 diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml index 10428ec..a73d05c 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml @@ -37,6 +37,8 @@ <interface type='bridge'> <mac address='02:00:15:8f:05:c1'/> <source bridge='virbr0'/> + <ip address='192.168.122.2' prefix='24'/> + <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/> <guest dev='eth0'/> <link state='up'/> </interface> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:56PM +0200, Cédric Bosdonnat wrote:
--- src/lxc/lxc_native.c | 153 ++++++++++++++++-------- tests/lxcconf2xmldata/lxcconf2xml-simple.config | 2 + tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 2 + 3 files changed, 106 insertions(+), 51 deletions(-)
diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 41e069f..f695a00 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -413,93 +413,124 @@ lxcCreateHostdevDef(int mode, int type, const char *data) return hostdev; }
+typedef struct { + virDomainDefPtr def; + char *type; + char *link; + char *mac; + char *flag; + char *macvlanmode; + char *vlanid; + char *name; + char **ips; + size_t nips; + bool privnet; + size_t networks; +} lxcNetworkParseData; + static int -lxcAddNetworkDefinition(virDomainDefPtr def, - const char *type, - const char *linkdev, - const char *mac, - const char *flag, - const char *macvlanmode, - const char *vlanid, - const char *name) +lxcAddNetworkDefinition(lxcNetworkParseData *data) { virDomainNetDefPtr net = NULL; virDomainHostdevDefPtr hostdev = NULL; bool isPhys, isVlan = false; + size_t nips = 0; + virDomainNetIpDefPtr *ips = NULL; + virDomainNetIpDefPtr ip = NULL; + char **ipparts = NULL; + size_t i;
- if ((type == NULL) || STREQ(type, "empty") || STREQ(type, "") || - STREQ(type, "none")) + if ((data->type == NULL) || STREQ(data->type, "empty") || + STREQ(data->type, "") || STREQ(data->type, "none")) return 0;
- isPhys = STREQ(type, "phys"); - isVlan = STREQ(type, "vlan"); - if (type != NULL && (isPhys || isVlan)) { - if (!linkdev) { + /* Add the IP addresses */ + for (i = 0; i < data->nips; i++) { + if (VIR_ALLOC(ip) < 0) + goto error; + + ipparts = virStringSplit(data->ips[i], "/", 2); + if (virStringListLength(ipparts) != 2 || + strlen(ipparts[0]) == 0 || + virStrToLong_ui(ipparts[1], NULL, 10, &ip->prefix) < 0) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid CIDR address: '%s'"), data->ips[i]); + goto error; + } + + if (VIR_STRDUP(ip->address, ipparts[0]) < 0) + goto error; + + if (VIR_APPEND_ELEMENT(ips, nips, ip) < 0) + goto error; + + virStringFreeList(ipparts); + } + + isPhys = STREQ(data->type, "phys"); + isVlan = STREQ(data->type, "vlan"); + if (data->type != NULL && (isPhys || isVlan)) { + if (!data->link) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Missing 'link' attribute for NIC")); goto error; } if (!(hostdev = lxcCreateHostdevDef(VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET, - linkdev))) + data->link))) goto error;
/* This still requires the user to manually setup the vlan interface * on the host */ - if (isVlan && vlanid) { + if (isVlan && data->vlanid) { VIR_FREE(hostdev->source.caps.u.net.iface); if (virAsprintf(&hostdev->source.caps.u.net.iface, - "%s.%s", linkdev, vlanid) < 0) + "%s.%s", data->link, data->vlanid) < 0) goto error; }
- if (VIR_EXPAND_N(def->hostdevs, def->nhostdevs, 1) < 0) + if (VIR_EXPAND_N(data->def->hostdevs, data->def->nhostdevs, 1) < 0) goto error; - def->hostdevs[def->nhostdevs - 1] = hostdev; + data->def->hostdevs[data->def->nhostdevs - 1] = hostdev; } else { - if (!(net = lxcCreateNetDef(type, linkdev, mac, flag, macvlanmode, name))) + if (!(net = lxcCreateNetDef(data->type, data->link, data->mac, + data->flag, data->macvlanmode, + data->name))) goto error;
- if (VIR_EXPAND_N(def->nets, def->nnets, 1) < 0) + net->ips = ips; + net->nips = nips; + + if (VIR_EXPAND_N(data->def->nets, data->def->nnets, 1) < 0) goto error; - def->nets[def->nnets - 1] = net; + data->def->nets[data->def->nnets - 1] = net; }
return 1;
error: + for (i = 0; i < nips; i++) { + virDomainNetIpDefFree(ips[i]); + } + VIR_FREE(ips); + virStringFreeList(ipparts); + virDomainNetIpDefFree(ip); virDomainNetDefFree(net); virDomainHostdevDefFree(hostdev); return -1; }
-typedef struct { - virDomainDefPtr def; - char *type; - char *link; - char *mac; - char *flag; - char *macvlanmode; - char *vlanid; - char *name; - bool privnet; - size_t networks; -} lxcNetworkParseData; - static int lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) { lxcNetworkParseData *parseData = data; int status; + size_t i;
if (STREQ(name, "lxc.network.type")) { /* Store the previous NIC */ - status = lxcAddNetworkDefinition(parseData->def, parseData->type, - parseData->link, parseData->mac, - parseData->flag, - parseData->macvlanmode, - parseData->vlanid, - parseData->name); + status = lxcAddNetworkDefinition(parseData);
if (status < 0) return -1; @@ -517,6 +548,12 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) parseData->vlanid = NULL; parseData->name = NULL;
+ /* IPs array needs to be free'd as all IPs are dup'ed there */ + for (i = 0; i < parseData->nips; i++) + VIR_FREE(parseData->ips[i]); + VIR_FREE(parseData->ips); + parseData->nips = 0; + /* Keep the new value */ parseData->type = value->str; } @@ -532,7 +569,14 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) parseData->vlanid = value->str; else if (STREQ(name, "lxc.network.name")) parseData->name = value->str; - else if (STRPREFIX(name, "lxc.network")) + else if (STREQ(name, "lxc.network.ipv4") || + STREQ(name, "lxc.network.ipv6")) { + + if (VIR_EXPAND_N(parseData->ips, parseData->nips, 1) < 0) + return -1; + if (VIR_STRDUP(parseData->ips[parseData->nips - 1], value->str) < 0) + return -1;
It would be nice to record the address family here, and then copy that into the XML config explicitly.
diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.config b/tests/lxcconf2xmldata/lxcconf2xml-simple.config index b90abc1..d417ba0 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.config +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.config @@ -6,6 +6,8 @@ lxc.network.flags = up lxc.network.link = virbr0 lxc.network.hwaddr = 02:00:15:8f:05:c1 lxc.network.name = eth0 +lxc.network.ipv4 = 192.168.122.2/24 +lxc.network.ipv6 = 2003:db8:1:0:214:1234:fe0b:3596/64
#remove next line if host DNS configuration should not be available to container lxc.mount.entry = proc proc proc nodev,noexec,nosuid 0 0 diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml index 10428ec..a73d05c 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml @@ -37,6 +37,8 @@ <interface type='bridge'> <mac address='02:00:15:8f:05:c1'/> <source bridge='virbr0'/> + <ip address='192.168.122.2' prefix='24'/> + <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/>
I'd like to see this become <ip family="ipv4" address='192.168.122.2' prefix='24'/> <ip family="ipv6" address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/> Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- docs/formatdomain.html.in | 12 +++++++-- docs/schemas/domaincommon.rng | 23 +++++++++++++++--- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ tests/lxcxml2xmldata/lxc-hostdev.xml | 2 ++ 5 files changed, 80 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e3a522..e07a298 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4249,13 +4249,21 @@ qemu-kvm -net nic,model=? /dev/null <target dev='vnet0'/> <b><ip address='192.168.122.5' prefix='24'/></b> </interface> + ... + <hostdev mode='capabilities' type='net'> + <source> + <interface>eth0</interface> + </source> + <b><ip address='192.168.122.6' prefix='24'/></b> + </hostdev> + </devices> ... </pre> <p> - <span class="since">Since 1.2.10</span> the network devices can be provided - zero or more IP addresses to set + <span class="since">Since 1.2.10</span> the network devices and host devices + with network capabilities can be provided zero or more IP addresses to set on the target device. Note that some hypervisors or network device types will simply ignore them or only use the first one. The <code>address</code> attribute can hold either an IPv4 or IPv6 address. The <code>prefix</code> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a82d705..5d955b0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3769,11 +3769,26 @@ <attribute name="type"> <value>net</value> </attribute> - <element name="source"> - <element name="interface"> - <ref name="deviceName"/> + <interleave> + <element name="source"> + <element name="interface"> + <ref name="deviceName"/> + </element> </element> - </element> + <zeroOrMore> + <element name="ip"> + <attribute name="address"> + <ref name="ipAddr"/> + </attribute> + <optional> + <attribute name="prefix"> + <ref name="ipPrefix"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> + </interleave> </define> <define name="usbproduct"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7ab962..f39be87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1776,6 +1776,8 @@ virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc void virDomainHostdevDefClear(virDomainHostdevDefPtr def) { + size_t i; + if (!def) return; @@ -1800,6 +1802,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) break; case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET: VIR_FREE(def->source.caps.u.net.iface); + for (i = 0; i < def->source.caps.u.net.nips; i++) + virDomainNetIpDefFree(def->source.caps.u.net.ips[i]); + VIR_FREE(def->source.caps.u.net.ips); break; } break; @@ -4678,6 +4683,8 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, virDomainHostdevDefPtr def) { xmlNodePtr sourcenode; + xmlNodePtr *ipnodes = NULL; + int nipnodes; int ret = -1; /* @type is passed in from the caller rather than read from the @@ -4732,6 +4739,40 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, _("Missing <interface> element in hostdev net device")); goto error; } + + /* Parse possible IP addresses */ + if ((nipnodes = virXPathNodeSet("./ip", ctxt, &ipnodes)) < 0) + goto error; + + if (nipnodes) { + size_t i; + for (i = 0; i < nipnodes; i++) { + char *prefixStr = NULL; + virDomainNetIpDefPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + goto error; + + ip->address = virXMLPropString(ipnodes[i], "address"); + + if ((prefixStr = virXMLPropString(ipnodes[i], "prefix")) && + (virStrToLong_ui(prefixStr, NULL, 10, &ip->prefix) < 0)) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid network prefix: '%s'"), + prefixStr); + VIR_FREE(prefixStr); + goto error; + } + VIR_FREE(prefixStr); + + if (ip->address != NULL && + VIR_APPEND_ELEMENT(def->source.caps.u.net.ips, + def->source.caps.u.net.nips, ip) < 0) + goto error; + } + VIR_FREE(ipnodes); + } break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -16751,6 +16792,12 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf, virBufferAdjustIndent(buf, -2); virBufferAddLit(buf, "</source>\n"); + + if (def->source.caps.type == VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET) { + virDomainNetIpsFormat(buf, def->source.caps.u.net.ips, + def->source.caps.u.net.nips); + } + return 0; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6ecf639..bf0e2eb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -493,6 +493,8 @@ struct _virDomainHostdevCaps { } misc; struct { char *iface; + size_t nips; + virDomainNetIpDefPtr *ips; } net; } u; }; diff --git a/tests/lxcxml2xmldata/lxc-hostdev.xml b/tests/lxcxml2xmldata/lxc-hostdev.xml index befe0db..23bf04d 100644 --- a/tests/lxcxml2xmldata/lxc-hostdev.xml +++ b/tests/lxcxml2xmldata/lxc-hostdev.xml @@ -35,6 +35,8 @@ <source> <interface>eth0</interface> </source> + <ip address='192.168.122.2'/> + <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='24'/> </hostdev> </devices> </domain> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:57PM +0200, Cédric Bosdonnat wrote:
--- docs/formatdomain.html.in | 12 +++++++-- docs/schemas/domaincommon.rng | 23 +++++++++++++++--- src/conf/domain_conf.c | 47 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 2 ++ tests/lxcxml2xmldata/lxc-hostdev.xml | 2 ++ 5 files changed, 80 insertions(+), 6 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 8e3a522..e07a298 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4249,13 +4249,21 @@ qemu-kvm -net nic,model=? /dev/null <target dev='vnet0'/> <b><ip address='192.168.122.5' prefix='24'/></b> </interface> + ... + <hostdev mode='capabilities' type='net'> + <source> + <interface>eth0</interface> + </source> + <b><ip address='192.168.122.6' prefix='24'/></b> + </hostdev> + </devices> ... </pre>
<p> - <span class="since">Since 1.2.10</span> the network devices can be provided - zero or more IP addresses to set + <span class="since">Since 1.2.10</span> the network devices and host devices + with network capabilities can be provided zero or more IP addresses to set on the target device. Note that some hypervisors or network device types will simply ignore them or only use the first one. The <code>address</code> attribute can hold either an IPv4 or IPv6 address. The <code>prefix</code> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a82d705..5d955b0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3769,11 +3769,26 @@ <attribute name="type"> <value>net</value> </attribute> - <element name="source"> - <element name="interface"> - <ref name="deviceName"/> + <interleave> + <element name="source"> + <element name="interface"> + <ref name="deviceName"/> + </element> </element> - </element> + <zeroOrMore> + <element name="ip"> + <attribute name="address"> + <ref name="ipAddr"/> + </attribute> + <optional> + <attribute name="prefix"> + <ref name="ipPrefix"/> + </attribute> + </optional> + <empty/> + </element> + </zeroOrMore> + </interleave> </define>
<define name="usbproduct"> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7ab962..f39be87 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1776,6 +1776,8 @@ virDomainHostdevSubsysSCSIiSCSIClear(virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc
void virDomainHostdevDefClear(virDomainHostdevDefPtr def) { + size_t i; + if (!def) return;
@@ -1800,6 +1802,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) break; case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET: VIR_FREE(def->source.caps.u.net.iface); + for (i = 0; i < def->source.caps.u.net.nips; i++) + virDomainNetIpDefFree(def->source.caps.u.net.ips[i]); + VIR_FREE(def->source.caps.u.net.ips); break; } break; @@ -4678,6 +4683,8 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, virDomainHostdevDefPtr def) { xmlNodePtr sourcenode; + xmlNodePtr *ipnodes = NULL; + int nipnodes; int ret = -1;
/* @type is passed in from the caller rather than read from the @@ -4732,6 +4739,40 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, _("Missing <interface> element in hostdev net device")); goto error; } + + /* Parse possible IP addresses */ + if ((nipnodes = virXPathNodeSet("./ip", ctxt, &ipnodes)) < 0) + goto error; + + if (nipnodes) { + size_t i; + for (i = 0; i < nipnodes; i++) { + char *prefixStr = NULL; + virDomainNetIpDefPtr ip = NULL; + + if (VIR_ALLOC(ip) < 0) + goto error; + + ip->address = virXMLPropString(ipnodes[i], "address"); + + if ((prefixStr = virXMLPropString(ipnodes[i], "prefix")) && + (virStrToLong_ui(prefixStr, NULL, 10, &ip->prefix) < 0)) { + + virReportError(VIR_ERR_INVALID_ARG, + _("Invalid network prefix: '%s'"), + prefixStr); + VIR_FREE(prefixStr); + goto error; + } + VIR_FREE(prefixStr); + + if (ip->address != NULL && + VIR_APPEND_ELEMENT(def->source.caps.u.net.ips, + def->source.caps.u.net.nips, ip) < 0) + goto error; + } + VIR_FREE(ipnodes); + }
It would be nice if we can share the parsing of this with the parsingdone in the <network> xml block. Since the XML schema is the same, I'd expect we can have a single function that contains everything from the virXPathNodeSet("./ip") onwards. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Hi Daniel, On Wed, 2014-10-22 at 11:09 +0100, Daniel P. Berrange wrote:
It would be nice if we can share the parsing of this with the parsingdone in the <network> xml block. Since the XML schema is the same, I'd expect we can have a single function that contains everything from the virXPathNodeSet("./ip") onwards.
You surely meant to share the code parsing the interface/ip and hostdev/ip, right? Regards, -- Cedric

On Tue, Oct 28, 2014 at 04:45:21PM -0600, Cedric Bosdonnat wrote:
Hi Daniel,
On Wed, 2014-10-22 at 11:09 +0100, Daniel P. Berrange wrote:
It would be nice if we can share the parsing of this with the parsingdone in the <network> xml block. Since the XML schema is the same, I'd expect we can have a single function that contains everything from the virXPathNodeSet("./ip") onwards.
You surely meant to share the code parsing the interface/ip and hostdev/ip, right?
Yes, I meant <interface> Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/lxc/lxc_native.c | 3 +++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 2 ++ 3 files changed, 7 insertions(+) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index f695a00..8590d63 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -490,6 +490,9 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) goto error; } + hostdev->source.caps.u.net.ips = ips; + hostdev->source.caps.u.net.nips = nips; + if (VIR_EXPAND_N(data->def->hostdevs, data->def->nhostdevs, 1) < 0) goto error; data->def->hostdevs[data->def->nhostdevs - 1] = hostdev; diff --git a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config index ed196e1..94f7c61 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config +++ b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config @@ -1,6 +1,8 @@ lxc.network.type = phys lxc.network.link = eth0 lxc.network.name = eth1 +lxc.network.ipv4 = 192.168.122.2/24 +lxc.network.ipv6 = 2003:db8:1:0:214:1234:fe0b:3596/64 lxc.rootfs = /var/lib/lxc/migrate_test/rootfs lxc.utsname = migrate_test diff --git a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml index cfaceb5..7cd9e6f 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml +++ b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml @@ -25,6 +25,8 @@ <source> <interface>eth0</interface> </source> + <ip address='192.168.122.2' prefix='24'/> + <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/> </hostdev> </devices> </domain> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:58PM +0200, Cédric Bosdonnat wrote:
--- src/lxc/lxc_native.c | 3 +++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 2 ++ 3 files changed, 7 insertions(+)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Network interfaces devices and host devices with net capabilities can now have an IPv4 and/or an IPv6 address configured. --- docs/formatdomain.html.in | 9 ++++++++ docs/schemas/domaincommon.rng | 23 ++++++++++++++++++++ src/conf/domain_conf.c | 42 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ tests/lxcxml2xmldata/lxc-hostdev.xml | 1 + tests/lxcxml2xmldata/lxc-idmap.xml | 1 + 6 files changed, 80 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e07a298..a925b6f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4248,6 +4248,7 @@ qemu-kvm -net nic,model=? /dev/null <source network='default'/> <target dev='vnet0'/> <b><ip address='192.168.122.5' prefix='24'/></b> + <b><gateway ipv4='192.168.122.1'/></b> </interface> ... <hostdev mode='capabilities' type='net'> @@ -4255,6 +4256,7 @@ qemu-kvm -net nic,model=? /dev/null <interface>eth0</interface> </source> <b><ip address='192.168.122.6' prefix='24'/></b> + <b><gateway ipv4='192.168.122.1'/></b> </hostdev> </devices> @@ -4270,6 +4272,13 @@ qemu-kvm -net nic,model=? /dev/null is not mandatory since some hypervisors do not handle it. </p> + <p> + <span class="since">Since 1.2.10</span> a gateway element can also be added + to provide the default gateway to use for the network device. This element + can have either or both <code>ipv4</code> or <code>ipv6</code> attributes. + This is only used by the LXC driver. + </p> + <h5><a name="elementVhostuser">vhost-user interface</a></h5> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5d955b0..a7659af 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2304,6 +2304,11 @@ </element> </zeroOrMore> <optional> + <element name="gateway"> + <ref name="gateway"/> + </element> + </optional> + <optional> <element name="script"> <attribute name="path"> <ref name="filePath"/> @@ -3558,6 +3563,19 @@ </element> </define> + <define name="gateway"> + <optional> + <attribute name="ipv4"> + <ref name="ipv4Addr"/> + </attribute> + </optional> + <optional> + <attribute name="ipv6"> + <ref name="ipv6Addr"/> + </attribute> + </optional> + </define> + <define name="hostdev"> <element name="hostdev"> <interleave> @@ -3788,6 +3806,11 @@ <empty/> </element> </zeroOrMore> + <optional> + <element name="gateway"> + <ref name="gateway"/> + </element> + </optional> </interleave> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f39be87..41d5e7a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1428,6 +1428,9 @@ void virDomainNetDefFree(virDomainNetDefPtr def) virDomainNetIpDefFree(def->ips[i]); VIR_FREE(def->ips); + VIR_FREE(def->gateway_ipv4); + VIR_FREE(def->gateway_ipv6); + virDomainDeviceInfoClear(&def->info); VIR_FREE(def->filter); @@ -1805,6 +1808,8 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def) for (i = 0; i < def->source.caps.u.net.nips; i++) virDomainNetIpDefFree(def->source.caps.u.net.ips[i]); VIR_FREE(def->source.caps.u.net.ips); + VIR_FREE(def->source.caps.u.net.gateway_ipv4); + VIR_FREE(def->source.caps.u.net.gateway_ipv6); break; } break; @@ -4685,6 +4690,7 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, xmlNodePtr sourcenode; xmlNodePtr *ipnodes = NULL; int nipnodes; + xmlNodePtr gwnode; int ret = -1; /* @type is passed in from the caller rather than read from the @@ -4773,6 +4779,12 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED, } VIR_FREE(ipnodes); } + + /* Look for possible gateways */ + if ((gwnode = virXPathNode("./gateway", ctxt))) { + def->source.caps.u.net.gateway_ipv4 = virXMLPropString(gwnode, "ipv4"); + def->source.caps.u.net.gateway_ipv6 = virXMLPropString(gwnode, "ipv6"); + } break; default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -7052,6 +7064,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, size_t i; size_t nips = 0; virDomainNetIpDefPtr *ips = NULL; + char *gateway_ipv4 = NULL; + char *gateway_ipv6 = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -7178,6 +7192,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_APPEND_ELEMENT(ips, nips, ip) < 0) goto error; } + } else if (!gateway_ipv4 && !gateway_ipv6 && + xmlStrEqual(cur->name, BAD_CAST "gateway")) { + gateway_ipv4 = virXMLPropString(cur, "ipv4"); + gateway_ipv6 = virXMLPropString(cur, "ipv6"); } else if (!ifname && xmlStrEqual(cur->name, BAD_CAST "target")) { ifname = virXMLPropString(cur, "dev"); @@ -7496,6 +7514,10 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, if (VIR_APPEND_ELEMENT(def->ips, def->nips, ips[i]) < 0) goto error; } + def->gateway_ipv4 = gateway_ipv4; + gateway_ipv4 = NULL; + def->gateway_ipv6 = gateway_ipv6; + gateway_ipv6 = NULL; if (script != NULL) { def->script = script; @@ -7761,6 +7783,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(trustGuestRxFilters); VIR_FREE(ips); virNWFilterHashTableFree(filterparams); + VIR_FREE(gateway_ipv4); + VIR_FREE(gateway_ipv6); return def; @@ -16763,6 +16787,21 @@ virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) } } +static void +virDomainNetGatewayFormat(virBufferPtr buf, + const char* gateway_ipv4, + const char* gateway_ipv6) +{ + if (gateway_ipv4 || gateway_ipv6) { + virBufferAddLit(buf, "<gateway"); + if (gateway_ipv4) + virBufferAsprintf(buf, " ipv4='%s'", gateway_ipv4); + if (gateway_ipv6) + virBufferAsprintf(buf, " ipv6='%s'", gateway_ipv6); + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainHostdevDefFormatCaps(virBufferPtr buf, virDomainHostdevDefPtr def) @@ -16796,6 +16835,8 @@ virDomainHostdevDefFormatCaps(virBufferPtr buf, if (def->source.caps.type == VIR_DOMAIN_HOSTDEV_CAPS_TYPE_NET) { virDomainNetIpsFormat(buf, def->source.caps.u.net.ips, def->source.caps.u.net.nips); + virDomainNetGatewayFormat(buf, def->source.caps.u.net.gateway_ipv4, + def->source.caps.u.net.gateway_ipv6); } return 0; @@ -17174,6 +17215,7 @@ virDomainNetDefFormat(virBufferPtr buf, } virDomainNetIpsFormat(buf, def->ips, def->nips); + virDomainNetGatewayFormat(buf, def->gateway_ipv4, def->gateway_ipv6); virBufferEscapeString(buf, "<script path='%s'/>\n", def->script); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bf0e2eb..5df97c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -495,6 +495,8 @@ struct _virDomainHostdevCaps { char *iface; size_t nips; virDomainNetIpDefPtr *ips; + char *gateway_ipv4; + char *gateway_ipv6; } net; } u; }; @@ -988,6 +990,8 @@ struct _virDomainNetDef { int linkstate; size_t nips; virDomainNetIpDefPtr *ips; + char *gateway_ipv4; + char *gateway_ipv6; }; /* Used for prefix of ifname of any network name generated dynamically diff --git a/tests/lxcxml2xmldata/lxc-hostdev.xml b/tests/lxcxml2xmldata/lxc-hostdev.xml index 23bf04d..694bc87 100644 --- a/tests/lxcxml2xmldata/lxc-hostdev.xml +++ b/tests/lxcxml2xmldata/lxc-hostdev.xml @@ -37,6 +37,7 @@ </source> <ip address='192.168.122.2'/> <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='24'/> + <gateway ipv4='192.168.122.1' ipv6='2003:db8:1:0:214:1234:fe0b:3595'/> </hostdev> </devices> </domain> diff --git a/tests/lxcxml2xmldata/lxc-idmap.xml b/tests/lxcxml2xmldata/lxc-idmap.xml index a52da0b..80ee432 100644 --- a/tests/lxcxml2xmldata/lxc-idmap.xml +++ b/tests/lxcxml2xmldata/lxc-idmap.xml @@ -30,6 +30,7 @@ <source bridge='bri0'/> <ip address='192.168.122.12' prefix='24'/> <ip address='192.168.122.13' prefix='24'/> + <gateway ipv4='192.168.122.1'/> <target dev='veth0'/> <guest dev='eth2'/> </interface> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:03:59PM +0200, Cédric Bosdonnat wrote:
Network interfaces devices and host devices with net capabilities can now have an IPv4 and/or an IPv6 address configured. --- docs/formatdomain.html.in | 9 ++++++++ docs/schemas/domaincommon.rng | 23 ++++++++++++++++++++ src/conf/domain_conf.c | 42 ++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 ++++ tests/lxcxml2xmldata/lxc-hostdev.xml | 1 + tests/lxcxml2xmldata/lxc-idmap.xml | 1 + 6 files changed, 80 insertions(+)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e07a298..a925b6f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4248,6 +4248,7 @@ qemu-kvm -net nic,model=? /dev/null <source network='default'/> <target dev='vnet0'/> <b><ip address='192.168.122.5' prefix='24'/></b> + <b><gateway ipv4='192.168.122.1'/></b> </interface> ... <hostdev mode='capabilities' type='net'> @@ -4255,6 +4256,7 @@ qemu-kvm -net nic,model=? /dev/null <interface>eth0</interface> </source> <b><ip address='192.168.122.6' prefix='24'/></b> + <b><gateway ipv4='192.168.122.1'/></b>
I think I'd expect this to look more like this: <gateway family="ipv4" address="192.168.122.1"/> <gateway family="ipv6" address="1:2:3::4"/> We can still store it in the internal struct in the way you have it here though.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index bf0e2eb..5df97c4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -495,6 +495,8 @@ struct _virDomainHostdevCaps { char *iface; size_t nips; virDomainNetIpDefPtr *ips; + char *gateway_ipv4; + char *gateway_ipv6; } net; } u; }; @@ -988,6 +990,8 @@ struct _virDomainNetDef { int linkstate; size_t nips; virDomainNetIpDefPtr *ips; + char *gateway_ipv4; + char *gateway_ipv6;
I think these ought to use virSocketAddr though, so we have syntactic validation at time of parsing. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/lxc/lxc_native.c | 19 ++++++++++++++++++- tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 1 + tests/lxcconf2xmldata/lxcconf2xml-simple.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 1 + 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 8590d63..51295fa 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -423,6 +423,8 @@ typedef struct { char *vlanid; char *name; char **ips; + char *gateway_ipv4; + char *gateway_ipv6; size_t nips; bool privnet; size_t networks; @@ -493,6 +495,12 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) hostdev->source.caps.u.net.ips = ips; hostdev->source.caps.u.net.nips = nips; + if (VIR_STRDUP(hostdev->source.caps.u.net.gateway_ipv4, + data->gateway_ipv4) < 0 || + VIR_STRDUP(hostdev->source.caps.u.net.gateway_ipv6, + data->gateway_ipv6) < 0) + goto error; + if (VIR_EXPAND_N(data->def->hostdevs, data->def->nhostdevs, 1) < 0) goto error; data->def->hostdevs[data->def->nhostdevs - 1] = hostdev; @@ -505,6 +513,10 @@ lxcAddNetworkDefinition(lxcNetworkParseData *data) net->ips = ips; net->nips = nips; + if (VIR_STRDUP(net->gateway_ipv4, data->gateway_ipv4) < 0 || + VIR_STRDUP(net->gateway_ipv6, data->gateway_ipv6) < 0) + goto error; + if (VIR_EXPAND_N(data->def->nets, data->def->nnets, 1) < 0) goto error; data->def->nets[data->def->nnets - 1] = net; @@ -579,6 +591,10 @@ lxcNetworkWalkCallback(const char *name, virConfValuePtr value, void *data) return -1; if (VIR_STRDUP(parseData->ips[parseData->nips - 1], value->str) < 0) return -1; + } else if (STREQ(name, "lxc.network.ipv4.gateway")) { + parseData->gateway_ipv4 = value->str; + } else if (STREQ(name, "lxc.network.ipv6.gateway")) { + parseData->gateway_ipv6 = value->str; } else if (STRPREFIX(name, "lxc.network")) VIR_WARN("Unhandled network property: %s = %s", name, @@ -594,7 +610,8 @@ lxcConvertNetworkSettings(virDomainDefPtr def, virConfPtr properties) int result = -1; size_t i; lxcNetworkParseData data = {def, NULL, NULL, NULL, NULL, - NULL, NULL, NULL, NULL, 0, true, 0}; + NULL, NULL, NULL, NULL, NULL, + NULL, 0, true, 0}; if (virConfWalk(properties, lxcNetworkWalkCallback, &data) < 0) goto cleanup; diff --git a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config index 94f7c61..779dac2 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config +++ b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config @@ -2,7 +2,9 @@ lxc.network.type = phys lxc.network.link = eth0 lxc.network.name = eth1 lxc.network.ipv4 = 192.168.122.2/24 +lxc.network.ipv4.gateway = 192.168.122.1 lxc.network.ipv6 = 2003:db8:1:0:214:1234:fe0b:3596/64 +lxc.network.ipv6.gateway = 2003:db8:1:0:214:1234:fe0b:3595 lxc.rootfs = /var/lib/lxc/migrate_test/rootfs lxc.utsname = migrate_test diff --git a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml index 7cd9e6f..b80147f 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml +++ b/tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml @@ -27,6 +27,7 @@ </source> <ip address='192.168.122.2' prefix='24'/> <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/> + <gateway ipv4='192.168.122.1' ipv6='2003:db8:1:0:214:1234:fe0b:3595'/> </hostdev> </devices> </domain> diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.config b/tests/lxcconf2xmldata/lxcconf2xml-simple.config index d417ba0..50a44bb 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.config +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.config @@ -7,7 +7,9 @@ lxc.network.link = virbr0 lxc.network.hwaddr = 02:00:15:8f:05:c1 lxc.network.name = eth0 lxc.network.ipv4 = 192.168.122.2/24 +lxc.network.ipv4.gateway = 192.168.122.1 lxc.network.ipv6 = 2003:db8:1:0:214:1234:fe0b:3596/64 +lxc.network.ipv6.gateway = 2003:db8:1:0:214:1234:fe0b:3595 #remove next line if host DNS configuration should not be available to container lxc.mount.entry = proc proc proc nodev,noexec,nosuid 0 0 diff --git a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml index a73d05c..32494e4 100644 --- a/tests/lxcconf2xmldata/lxcconf2xml-simple.xml +++ b/tests/lxcconf2xmldata/lxcconf2xml-simple.xml @@ -39,6 +39,7 @@ <source bridge='virbr0'/> <ip address='192.168.122.2' prefix='24'/> <ip address='2003:db8:1:0:214:1234:fe0b:3596' prefix='64'/> + <gateway ipv4='192.168.122.1' ipv6='2003:db8:1:0:214:1234:fe0b:3595'/> <guest dev='eth0'/> <link state='up'/> </interface> -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:00PM +0200, Cédric Bosdonnat wrote:
--- src/lxc/lxc_native.c | 19 ++++++++++++++++++- tests/lxcconf2xmldata/lxcconf2xml-physnetwork.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 1 + tests/lxcconf2xmldata/lxcconf2xml-simple.config | 2 ++ tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 1 + 5 files changed, 24 insertions(+), 1 deletion(-)
ACK, though my previous comments will obviously affect this code Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

When a gateway is set on a network device, a new default route via this gateway through the devoce will be added in the container. --- src/lxc/lxc_container.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 608d39f..bbab4af 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -480,6 +480,35 @@ lxcContainerGetNetDef(virDomainDefPtr vmDef, const char *devName) return NULL; } +static int +lxcContainerAddDefaultRoute(const char *ifname, + const char *gateway, + int family) +{ + virSocketAddr address; + virSocketAddr network; + + VIR_DEBUG("Adding default route via %s on dev %s", gateway, ifname); + if (virSocketAddrParse(&address, gateway, family) < 0) + return -1; + + if (family == AF_INET) { + if (virSocketAddrParseIPv4(&network, "0.0.0.0") < 0) + return -1; + } else { + if (virSocketAddrParseIPv6(&network, "::") < 0) + return -1; + } + + if (virNetDevAddRoute(ifname, &network, 0, &address, 0) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Failed adding default route via %s on dev %s"), + gateway, ifname); + return -1; + } + return 0; +} + /** * lxcContainerRenameAndEnableInterfaces: * @nveths: number of interfaces @@ -544,6 +573,17 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, if (rc < 0) goto error_out; + /* Set the gateways */ + if (netDef->gateway_ipv4 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, + AF_INET) < 0) + goto error_out; + + if (netDef->gateway_ipv6 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, + AF_INET6) < 0) + goto error_out; + VIR_FREE(newname); } -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:01PM +0200, Cédric Bosdonnat wrote:
When a gateway is set on a network device, a new default route via this gateway through the devoce will be added in the container. --- src/lxc/lxc_container.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 608d39f..bbab4af 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -480,6 +480,35 @@ lxcContainerGetNetDef(virDomainDefPtr vmDef, const char *devName) return NULL; }
+static int +lxcContainerAddDefaultRoute(const char *ifname, + const char *gateway, + int family) +{ + virSocketAddr address; + virSocketAddr network; + + VIR_DEBUG("Adding default route via %s on dev %s", gateway, ifname); + if (virSocketAddrParse(&address, gateway, family) < 0) + return -1; + + if (family == AF_INET) { + if (virSocketAddrParseIPv4(&network, "0.0.0.0") < 0) + return -1; + } else { + if (virSocketAddrParseIPv6(&network, "::") < 0) + return -1; + }
I wouldn't mind us defining constants for the two strings VIR_SOCKET_ADDR_IPV4_ALL "0.0.0.0" VIR_SOCKET_ADDR_IPV6_ALL "::" Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Don't activate LXC network device if <link state='down'/> has been set in its configuration. --- src/lxc/lxc_container.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index bbab4af..4378b3d 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -568,21 +568,23 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, } } - VIR_DEBUG("Enabling %s", newname); - rc = virNetDevSetOnline(newname, true); - if (rc < 0) - goto error_out; + if (netDef->linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { + VIR_DEBUG("Enabling %s", newname); + rc = virNetDevSetOnline(newname, true); + if (rc < 0) + goto error_out; - /* Set the gateways */ - if (netDef->gateway_ipv4 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, - AF_INET) < 0) - goto error_out; + /* Set the gateways */ + if (netDef->gateway_ipv4 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, + AF_INET) < 0) + goto error_out; - if (netDef->gateway_ipv6 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, - AF_INET6) < 0) - goto error_out; + if (netDef->gateway_ipv6 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, + AF_INET6) < 0) + goto error_out; + } VIR_FREE(newname); } -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:02PM +0200, Cédric Bosdonnat wrote:
Don't activate LXC network device if <link state='down'/> has been set in its configuration. --- src/lxc/lxc_container.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index bbab4af..4378b3d 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -568,21 +568,23 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, } }
- VIR_DEBUG("Enabling %s", newname); - rc = virNetDevSetOnline(newname, true); - if (rc < 0) - goto error_out; + if (netDef->linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { + VIR_DEBUG("Enabling %s", newname); + rc = virNetDevSetOnline(newname, true); + if (rc < 0) + goto error_out;
- /* Set the gateways */ - if (netDef->gateway_ipv4 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, - AF_INET) < 0) - goto error_out; + /* Set the gateways */ + if (netDef->gateway_ipv4 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, + AF_INET) < 0) + goto error_out;
- if (netDef->gateway_ipv6 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, - AF_INET6) < 0) - goto error_out; + if (netDef->gateway_ipv6 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, + AF_INET6) < 0) + goto error_out; + }
VIR_FREE(newname); }
ACK makes sense, we can't configure addresses if the device is down. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Wednesday 22 October 2014 11:15:57 Daniel P. Berrange wrote:
On Fri, Oct 10, 2014 at 02:04:02PM +0200, Cédric Bosdonnat wrote:
Don't activate LXC network device if <link state='down'/> has been set in its configuration. ---
src/lxc/lxc_container.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index bbab4af..4378b3d 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -568,21 +568,23 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef,> }
}
- VIR_DEBUG("Enabling %s", newname); - rc = virNetDevSetOnline(newname, true); - if (rc < 0) - goto error_out; + if (netDef->linkstate != VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN) { + VIR_DEBUG("Enabling %s", newname); + rc = virNetDevSetOnline(newname, true); + if (rc < 0) + goto error_out;
- /* Set the gateways */ - if (netDef->gateway_ipv4 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, - AF_INET) < 0) - goto error_out; + /* Set the gateways */ + if (netDef->gateway_ipv4 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv4, + AF_INET) < 0) + goto error_out;
- if (netDef->gateway_ipv6 && - lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, - AF_INET6) < 0) - goto error_out; + if (netDef->gateway_ipv6 && + lxcContainerAddDefaultRoute(newname, netDef->gateway_ipv6, + AF_INET6) < 0) + goto error_out; + }
VIR_FREE(newname);
}
ACK makes sense, we can't configure addresses if the device is down.
We can, but it doesn't have much sense: [root@kir1 libvirt]# ip l set eth0 down [root@kir1 libvirt]# ip a s eth0 2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN qlen 1000 link/ether 00:19:d1:a3:bf:95 brd ff:ff:ff:ff:ff:ff [root@kir1 libvirt]# ip a a 192.168.11.1/24 dev eth0 [root@kir1 libvirt]# ip a s eth0 2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc pfifo_fast state DOWN qlen 1000 link/ether 00:19:d1:a3:bf:95 brd ff:ff:ff:ff:ff:ff inet 192.168.11.1/24 scope global eth0 [root@kir1 libvirt]# ip l set eth0 up [root@kir1 libvirt]# ip a s eth0 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,UP> mtu 1500 qdisc pfifo_fast state DOWN qlen 1000 link/ether 00:19:d1:a3:bf:95 brd ff:ff:ff:ff:ff:ff inet 192.168.11.1/24 scope global eth0
Regards, Daniel
-- Dmitry Guryanov

--- src/conf/domain_conf.c | 60 +++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41d5e7a..990620f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16650,6 +16650,36 @@ virDomainFSDefFormat(virBufferPtr buf, return 0; } +static void +virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) +{ + size_t i; + + /* Output IP addresses */ + for (i = 0; i < nips; i++) { + virBufferAsprintf(buf, "<ip address='%s'", + ips[i]->address); + if (ips[i]->prefix != 0) + virBufferAsprintf(buf, " prefix='%u'", ips[i]->prefix); + virBufferAddLit(buf, "/>\n"); + } +} + +static void +virDomainNetGatewayFormat(virBufferPtr buf, + const char* gateway_ipv4, + const char* gateway_ipv6) +{ + if (gateway_ipv4 || gateway_ipv6) { + virBufferAddLit(buf, "<gateway"); + if (gateway_ipv4) + virBufferAsprintf(buf, " ipv4='%s'", gateway_ipv4); + if (gateway_ipv6) + virBufferAsprintf(buf, " ipv6='%s'", gateway_ipv6); + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevDefPtr def, @@ -16772,36 +16802,6 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, return 0; } -static void -virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) -{ - size_t i; - - /* Output IP addresses */ - for (i = 0; i < nips; i++) { - virBufferAsprintf(buf, "<ip address='%s'", - ips[i]->address); - if (ips[i]->prefix != 0) - virBufferAsprintf(buf, " prefix='%u'", ips[i]->prefix); - virBufferAddLit(buf, "/>\n"); - } -} - -static void -virDomainNetGatewayFormat(virBufferPtr buf, - const char* gateway_ipv4, - const char* gateway_ipv6) -{ - if (gateway_ipv4 || gateway_ipv6) { - virBufferAddLit(buf, "<gateway"); - if (gateway_ipv4) - virBufferAsprintf(buf, " ipv4='%s'", gateway_ipv4); - if (gateway_ipv6) - virBufferAsprintf(buf, " ipv6='%s'", gateway_ipv6); - virBufferAddLit(buf, "/>\n"); - } -} - static int virDomainHostdevDefFormatCaps(virBufferPtr buf, virDomainHostdevDefPtr def) -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:03PM +0200, Cédric Bosdonnat wrote:
--- src/conf/domain_conf.c | 60 +++++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 30 deletions(-)
Could you update the commit message to be a little more explicit about why it is in the wrong place currently.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41d5e7a..990620f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16650,6 +16650,36 @@ virDomainFSDefFormat(virBufferPtr buf, return 0; }
+static void +virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) +{ + size_t i; + + /* Output IP addresses */ + for (i = 0; i < nips; i++) { + virBufferAsprintf(buf, "<ip address='%s'", + ips[i]->address); + if (ips[i]->prefix != 0) + virBufferAsprintf(buf, " prefix='%u'", ips[i]->prefix); + virBufferAddLit(buf, "/>\n"); + } +} + +static void +virDomainNetGatewayFormat(virBufferPtr buf, + const char* gateway_ipv4, + const char* gateway_ipv6) +{ + if (gateway_ipv4 || gateway_ipv6) { + virBufferAddLit(buf, "<gateway"); + if (gateway_ipv4) + virBufferAsprintf(buf, " ipv4='%s'", gateway_ipv4); + if (gateway_ipv6) + virBufferAsprintf(buf, " ipv6='%s'", gateway_ipv6); + virBufferAddLit(buf, "/>\n"); + } +} + static int virDomainHostdevDefFormatSubsys(virBufferPtr buf, virDomainHostdevDefPtr def, @@ -16772,36 +16802,6 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf, return 0; }
-static void -virDomainNetIpsFormat(virBufferPtr buf, virDomainNetIpDefPtr *ips, size_t nips) -{ - size_t i; - - /* Output IP addresses */ - for (i = 0; i < nips; i++) { - virBufferAsprintf(buf, "<ip address='%s'", - ips[i]->address); - if (ips[i]->prefix != 0) - virBufferAsprintf(buf, " prefix='%u'", ips[i]->prefix); - virBufferAddLit(buf, "/>\n"); - } -} - -static void -virDomainNetGatewayFormat(virBufferPtr buf, - const char* gateway_ipv4, - const char* gateway_ipv6) -{ - if (gateway_ipv4 || gateway_ipv6) { - virBufferAddLit(buf, "<gateway"); - if (gateway_ipv4) - virBufferAsprintf(buf, " ipv4='%s'", gateway_ipv4); - if (gateway_ipv6) - virBufferAsprintf(buf, " ipv6='%s'", gateway_ipv6); - virBufferAddLit(buf, "/>\n"); - } -} - static int virDomainHostdevDefFormatCaps(virBufferPtr buf, virDomainHostdevDefPtr def)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Add a default implementation of virNetDevSetIPv4Address using netlink and libnl. This avoids requiring /usr/sbin/ip or /usr/sbin/ifconfig external binaries. --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 136 +++++++++++++++++++++++++++++++++++++++++++++-- src/util/virnetlink.c | 38 +++++++++++++ src/util/virnetlink.h | 2 + 4 files changed, 174 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b55bf35..f96387f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1710,6 +1710,7 @@ virNetlinkEventServiceLocalPid; virNetlinkEventServiceStart; virNetlinkEventServiceStop; virNetlinkEventServiceStopAll; +virNetlinkGetErrorCode; virNetlinkShutdown; virNetlinkStartup; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index db5623a..e4a005a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -786,6 +786,88 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED, #endif /* ! SIOCGIFVLAN */ +#if defined(__linux__) && defined(HAVE_LIBNL) + +static int +virNetDevGetIPAddressBinary(virSocketAddr *addr, void **data, size_t *len) +{ + if (!addr) + return -1; + + switch (VIR_SOCKET_ADDR_FAMILY(addr)) { + case AF_INET: + *data = &addr->data.inet4.sin_addr; + *len = sizeof(struct in_addr); + break; + case AF_INET6: + *data = &addr->data.inet6.sin6_addr; + *len = sizeof(struct in6_addr); + break; + default: + return -1; + } + return 0; +} + +static struct nl_msg +*virNetDevCreateNetlinkAddressMessage(int messageType, + const char *ifname, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *broadcast) +{ + struct nl_msg *nlmsg = NULL; + struct ifaddrmsg ifa; + unsigned int ifindex; + void *addrData = NULL; + void *broadcastData = NULL; + size_t addrDataLen; + + if (virNetDevGetIPAddressBinary(addr, &addrData, &addrDataLen) < 0) + return NULL; + + if (broadcast && virNetDevGetIPAddressBinary(broadcast, &broadcastData, + &addrDataLen) < 0) + return NULL; + + /* Get the interface index */ + if ((ifindex = if_nametoindex(ifname)) == 0) + return NULL; + + if (!(nlmsg = nlmsg_alloc_simple(messageType, + NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL))) { + virReportOOMError(); + return NULL; + } + + memset(&ifa, 0, sizeof(ifa)); + + ifa.ifa_prefixlen = prefix; + ifa.ifa_family = VIR_SOCKET_ADDR_FAMILY(addr); + ifa.ifa_index = ifindex; + ifa.ifa_scope = 0; + + if (nlmsg_append(nlmsg, &ifa, sizeof(ifa), NLMSG_ALIGNTO) < 0) + goto buffer_too_small; + + if (nla_put(nlmsg, IFA_LOCAL, addrDataLen, addrData) < 0) + goto buffer_too_small; + + if (nla_put(nlmsg, IFA_ADDRESS, addrDataLen, addrData) < 0) + goto buffer_too_small; + + if (broadcastData && + nla_put(nlmsg, IFA_BROADCAST, addrDataLen, broadcastData) < 0) + goto buffer_too_small; + + return nlmsg; + + buffer_too_small: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("allocated netlink buffer is too small")); + nlmsg_free(nlmsg); + return NULL; +} /** * virNetDevSetIPv4Address: @@ -799,6 +881,52 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED, * * Returns 0 in case of success or -1 in case of error. */ +int virNetDevSetIPv4Address(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) +{ + virSocketAddr *broadcast = NULL; + int ret = -1; + struct nl_msg *nlmsg = NULL; + struct nlmsghdr *resp = NULL; + unsigned int recvbuflen; + + + /* The caller needs to provide a correct address */ + if (VIR_SOCKET_ADDR_FAMILY(addr) == AF_INET) { + /* compute a broadcast address if this is IPv4 */ + if (VIR_ALLOC(broadcast) < 0) + return -1; + + if (virSocketAddrBroadcastByPrefix(addr, prefix, broadcast) < 0) + goto cleanup; + } + + if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_NEWADDR, ifname, + addr, prefix, + broadcast))) + goto cleanup; + + if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, + NETLINK_ROUTE, 0) < 0) + goto cleanup; + + + if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Error adding IP address to %s"), ifname); + goto cleanup; + } + + ret = 0; + cleanup: + nlmsg_free(nlmsg); + VIR_FREE(resp); + VIR_FREE(broadcast); + return ret; +} + +#else /* defined(__linux__) && defined(HAVE_LIBNL) */ int virNetDevSetIPv4Address(const char *ifname, virSocketAddr *addr, @@ -817,7 +945,7 @@ int virNetDevSetIPv4Address(const char *ifname, !(bcaststr = virSocketAddrFormat(&broadcast)))) { goto cleanup; } -#ifdef IFCONFIG_PATH +# ifdef IFCONFIG_PATH cmd = virCommandNew(IFCONFIG_PATH); virCommandAddArg(cmd, ifname); if (VIR_SOCKET_ADDR_IS_FAMILY(addr, AF_INET6)) @@ -828,14 +956,14 @@ int virNetDevSetIPv4Address(const char *ifname, if (bcaststr) virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArg(cmd, "alias"); -#else +# else cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "addr", "add", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); if (bcaststr) virCommandAddArgList(cmd, "broadcast", bcaststr, NULL); virCommandAddArgList(cmd, "dev", ifname, NULL); -#endif +# endif if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -848,6 +976,8 @@ int virNetDevSetIPv4Address(const char *ifname, return ret; } +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ + /** * virNetDevAddRoute: * @ifname: the interface name diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 29511ad..f56603c 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -276,6 +276,44 @@ int virNetlinkCommand(struct nl_msg *nl_msg, return ret; } +int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen) +{ + struct nlmsgerr *err; + int result = 0; + + if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL) + goto malformed_resp; + + switch (resp->nlmsg_type) { + case NLMSG_ERROR: + err = (struct nlmsgerr *)NLMSG_DATA(resp); + if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err))) + goto malformed_resp; + + switch (err->error) { + case 0: /* ACK */ + break; + + default: + result = err->error; + } + break; + + case NLMSG_DONE: + break; + + default: + goto malformed_resp; + } + + return result; + + malformed_resp: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + return -EINVAL; +} + static void virNetlinkEventServerLock(virNetlinkEventSrvPrivatePtr driver) { diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index c478691..1a3e06d 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -52,6 +52,8 @@ int virNetlinkCommand(struct nl_msg *nl_msg, uint32_t src_pid, uint32_t dst_pid, unsigned int protocol, unsigned int groups); +int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen); + typedef void (*virNetlinkEventHandleCallback)(struct nlmsghdr *, unsigned int length, struct sockaddr_nl *peer, -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:04PM +0200, Cédric Bosdonnat wrote:
Add a default implementation of virNetDevSetIPv4Address using netlink and libnl. This avoids requiring /usr/sbin/ip or /usr/sbin/ifconfig external binaries. --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 136 +++++++++++++++++++++++++++++++++++++++++++++-- src/util/virnetlink.c | 38 +++++++++++++ src/util/virnetlink.h | 2 + 4 files changed, 174 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b55bf35..f96387f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1710,6 +1710,7 @@ virNetlinkEventServiceLocalPid; virNetlinkEventServiceStart; virNetlinkEventServiceStop; virNetlinkEventServiceStopAll; +virNetlinkGetErrorCode; virNetlinkShutdown; virNetlinkStartup;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index db5623a..e4a005a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -786,6 +786,88 @@ int virNetDevGetVLanID(const char *ifname ATTRIBUTE_UNUSED, #endif /* ! SIOCGIFVLAN */
+#if defined(__linux__) && defined(HAVE_LIBNL) + +static int +virNetDevGetIPAddressBinary(virSocketAddr *addr, void **data, size_t *len) +{ + if (!addr) + return -1; + + switch (VIR_SOCKET_ADDR_FAMILY(addr)) { + case AF_INET: + *data = &addr->data.inet4.sin_addr; + *len = sizeof(struct in_addr); + break; + case AF_INET6: + *data = &addr->data.inet6.sin6_addr; + *len = sizeof(struct in6_addr); + break; + default: + return -1; + } + return 0; +} + +static struct nl_msg +*virNetDevCreateNetlinkAddressMessage(int messageType,
I'd normally expect the '*' on the same line as the return type.
+ const char *ifname, + virSocketAddr *addr, + unsigned int prefix, + virSocketAddr *broadcast) +{
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

Renamed virNetDevSetIPv4Address as it also handles IPv6 addresses. --- src/libvirt_private.syms | 2 +- src/lxc/lxc_container.c | 2 +- src/network/bridge_driver.c | 4 ++-- src/util/virnetdev.c | 14 +++++++------- src/util/virnetdev.h | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f96387f..72f84e4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1621,7 +1621,7 @@ virNetDevRxFilterFree; virNetDevRxFilterModeTypeFromString; virNetDevRxFilterModeTypeToString; virNetDevRxFilterNew; -virNetDevSetIPv4Address; +virNetDevSetIPAddress; virNetDevSetMAC; virNetDevSetMTU; virNetDevSetMTUFromDevice; diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 4378b3d..783243d 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -560,7 +560,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, VIR_DEBUG("Adding IP address '%s/%u' to '%s'", ip->address, ip->prefix, newname); - if (virNetDevSetIPv4Address(newname, &address, prefix) < 0) { + if (virNetDevSetIPAddress(newname, &address, prefix) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Failed to set IP address '%s' on %s"), ip->address, newname); diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5f2e778..aa77e13 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1902,8 +1902,8 @@ networkAddAddrToBridge(virNetworkObjPtr network, return -1; } - if (virNetDevSetIPv4Address(network->def->bridge, - &ipdef->address, prefix) < 0) + if (virNetDevSetIPAddress(network->def->bridge, + &ipdef->address, prefix) < 0) return -1; return 0; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index e4a005a..771f11c 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -870,7 +870,7 @@ static struct nl_msg } /** - * virNetDevSetIPv4Address: + * virNetDevSetIPAddress: * @ifname: the interface name * @addr: the IP address (IPv4 or IPv6) * @prefix: number of 1 bits in the netmask @@ -881,9 +881,9 @@ static struct nl_msg * * Returns 0 in case of success or -1 in case of error. */ -int virNetDevSetIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevSetIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) { virSocketAddr *broadcast = NULL; int ret = -1; @@ -928,9 +928,9 @@ int virNetDevSetIPv4Address(const char *ifname, #else /* defined(__linux__) && defined(HAVE_LIBNL) */ -int virNetDevSetIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevSetIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) { virCommandPtr cmd = NULL; char *addrstr = NULL, *bcaststr = NULL; diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 2a6e67d..e1087a4 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -87,9 +87,9 @@ int virNetDevIsOnline(const char *ifname, bool *online) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int virNetDevSetIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevSetIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevAddRoute(const char *ifname, virSocketAddrPtr addr, -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:05PM +0200, Cédric Bosdonnat wrote:
Renamed virNetDevSetIPv4Address as it also handles IPv6 addresses. --- src/libvirt_private.syms | 2 +- src/lxc/lxc_container.c | 2 +- src/network/bridge_driver.c | 4 ++-- src/util/virnetdev.c | 14 +++++++------- src/util/virnetdev.h | 6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-)
ACK, though I could have thought this would be squashed into the same patch that actually adds IPv6 support Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/util/virnetdev.c | 105 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 15 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 771f11c..c919f34 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -926,6 +926,95 @@ int virNetDevSetIPAddress(const char *ifname, return ret; } +/** + * virNetDevAddRoute: + * @ifname: the interface name + * @addr: the IP network address (IPv4 or IPv6) + * @prefix: number of 1 bits in the netmask + * @gateway: via address for route (same as @addr) + * + * Add a route for a network IP address to an interface. This function + * *does not* remove any previously added IP static routes. + * + * Returns 0 in case of success or -1 in case of error. + */ +int +virNetDevAddRoute(const char *ifname, + virSocketAddrPtr addr, + unsigned int prefix, + virSocketAddrPtr gateway, + unsigned int metric) +{ + int ret = -1; + struct nl_msg *nlmsg = NULL; + struct nlmsghdr *resp = NULL; + unsigned int recvbuflen; + unsigned int ifindex; + struct rtmsg rtmsg; + void *gatewayData = NULL; + void *addrData = NULL; + size_t addrDataLen; + int errCode; + + if (virNetDevGetIPAddressBinary(addr, &addrData, &addrDataLen) < 0 || + virNetDevGetIPAddressBinary(gateway, &gatewayData, &addrDataLen) < 0) + goto cleanup; + + /* Get the interface index */ + if ((ifindex = if_nametoindex(ifname)) == 0) + goto cleanup; + + if (!(nlmsg = nlmsg_alloc_simple(RTM_NEWROUTE, + NLM_F_REQUEST | NLM_F_CREATE | + NLM_F_EXCL))) { + virReportOOMError(); + goto cleanup; + } + + memset(&rtmsg, 0, sizeof(rtmsg)); + + rtmsg.rtm_family = VIR_SOCKET_ADDR_FAMILY(addr); + rtmsg.rtm_table = RT_TABLE_MAIN; + rtmsg.rtm_scope = RT_SCOPE_UNIVERSE; + rtmsg.rtm_protocol = RTPROT_BOOT; + rtmsg.rtm_type = RTN_UNICAST; + rtmsg.rtm_dst_len = prefix; + + if (nlmsg_append(nlmsg, &rtmsg, sizeof(rtmsg), NLMSG_ALIGNTO) < 0) + goto buffer_too_small; + + if (prefix > 0 && nla_put(nlmsg, RTA_DST, addrDataLen, addrData) < 0) + goto buffer_too_small; + + if (nla_put(nlmsg, RTA_GATEWAY, addrDataLen, gatewayData) < 0) + goto buffer_too_small; + + if (nla_put_u32(nlmsg, RTA_OIF, ifindex) < 0) + goto buffer_too_small; + + if (metric > 0 && nla_put_u32(nlmsg, RTA_PRIORITY, metric) < 0) + goto buffer_too_small; + + if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, + NETLINK_ROUTE, 0) < 0) + goto cleanup; + + if ((errCode = virNetlinkGetErrorCode(resp, recvbuflen)) < 0) { + virReportSystemError(errCode, _("Error adding route to %s"), ifname); + goto cleanup; + } + + ret = 0; + cleanup: + nlmsg_free(nlmsg); + return ret; + + buffer_too_small: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("allocated netlink buffer is too small")); + goto cleanup; +} + #else /* defined(__linux__) && defined(HAVE_LIBNL) */ int virNetDevSetIPAddress(const char *ifname, @@ -976,21 +1065,6 @@ int virNetDevSetIPAddress(const char *ifname, return ret; } -#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ - -/** - * virNetDevAddRoute: - * @ifname: the interface name - * @addr: the IP network address (IPv4 or IPv6) - * @prefix: number of 1 bits in the netmask - * @gateway: via address for route (same as @addr) - * - * Add a route for a network IP address to an interface. This function - * *does not* remove any previously added IP static routes. - * - * Returns 0 in case of success or -1 in case of error. - */ - int virNetDevAddRoute(const char *ifname, virSocketAddrPtr addr, @@ -1023,6 +1097,7 @@ virNetDevAddRoute(const char *ifname, virCommandFree(cmd); return ret; } +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ /** * virNetDevClearIPv4Address: -- 1.8.4.5

On Fri, Oct 10, 2014 at 02:04:06PM +0200, Cédric Bosdonnat wrote:
--- src/util/virnetdev.c | 105 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 90 insertions(+), 15 deletions(-)
ACK Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

--- src/util/virnetdev.c | 60 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 15 deletions(-) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index c919f34..0cc7fdd 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1015,6 +1015,47 @@ virNetDevAddRoute(const char *ifname, goto cleanup; } +/** + * virNetDevClearIPv4Address: + * @ifname: the interface name + * @addr: the IP address (IPv4 or IPv6) + * @prefix: number of 1 bits in the netmask + * + * Delete an IP address from an interface. + * + * Returns 0 in case of success or -1 in case of error. + */ +int virNetDevClearIPv4Address(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) +{ + int ret = -1; + struct nl_msg *nlmsg = NULL; + struct nlmsghdr *resp = NULL; + unsigned int recvbuflen; + + if (!(nlmsg = virNetDevCreateNetlinkAddressMessage(RTM_DELADDR, ifname, + addr, prefix, + NULL))) + goto cleanup; + + if (virNetlinkCommand(nlmsg, &resp, &recvbuflen, 0, 0, + NETLINK_ROUTE, 0) < 0) + goto cleanup; + + if (virNetlinkGetErrorCode(resp, recvbuflen) < 0) { + virReportError(VIR_ERR_SYSTEM_ERROR, + _("Error removing IP address from %s"), ifname); + goto cleanup; + } + + ret = 0; + cleanup: + nlmsg_free(nlmsg); + VIR_FREE(resp); + return ret; +} + #else /* defined(__linux__) && defined(HAVE_LIBNL) */ int virNetDevSetIPAddress(const char *ifname, @@ -1097,18 +1138,6 @@ virNetDevAddRoute(const char *ifname, virCommandFree(cmd); return ret; } -#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ - -/** - * virNetDevClearIPv4Address: - * @ifname: the interface name - * @addr: the IP address (IPv4 or IPv6) - * @prefix: number of 1 bits in the netmask - * - * Delete an IP address from an interface. - * - * Returns 0 in case of success or -1 in case of error. - */ int virNetDevClearIPv4Address(const char *ifname, virSocketAddr *addr, @@ -1120,7 +1149,7 @@ int virNetDevClearIPv4Address(const char *ifname, if (!(addrstr = virSocketAddrFormat(addr))) goto cleanup; -#ifdef IFCONFIG_PATH +# ifdef IFCONFIG_PATH cmd = virCommandNew(IFCONFIG_PATH); virCommandAddArg(cmd, ifname); if (VIR_SOCKET_ADDR_IS_FAMILY(addr, AF_INET6)) @@ -1129,12 +1158,12 @@ int virNetDevClearIPv4Address(const char *ifname, virCommandAddArg(cmd, "inet"); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); virCommandAddArg(cmd, "-alias"); -#else +# else cmd = virCommandNew(IP_PATH); virCommandAddArgList(cmd, "addr", "del", NULL); virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix); virCommandAddArgList(cmd, "dev", ifname, NULL); -#endif +# endif if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -1146,6 +1175,7 @@ int virNetDevClearIPv4Address(const char *ifname, return ret; } +#endif /* defined(__linux__) && defined(HAVE_LIBNL) */ /** * virNetDevGetIPv4Address: -- 1.8.4.5

Make clear that virNetDevClearIPv4Address can also handle IPv6 addresses by changing the name --- src/libvirt_private.syms | 2 +- src/util/virnetdev.c | 14 +++++++------- src/util/virnetdev.h | 6 +++--- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 72f84e4..238ab7c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1598,7 +1598,7 @@ virMacAddrSetRaw; # util/virnetdev.h virNetDevAddRoute; -virNetDevClearIPv4Address; +virNetDevClearIPAddress; virNetDevExists; virNetDevGetIndex; virNetDevGetIPv4Address; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 0cc7fdd..553e8fc 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1016,7 +1016,7 @@ virNetDevAddRoute(const char *ifname, } /** - * virNetDevClearIPv4Address: + * virNetDevClearIPAddress: * @ifname: the interface name * @addr: the IP address (IPv4 or IPv6) * @prefix: number of 1 bits in the netmask @@ -1025,9 +1025,9 @@ virNetDevAddRoute(const char *ifname, * * Returns 0 in case of success or -1 in case of error. */ -int virNetDevClearIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevClearIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) { int ret = -1; struct nl_msg *nlmsg = NULL; @@ -1139,9 +1139,9 @@ virNetDevAddRoute(const char *ifname, return ret; } -int virNetDevClearIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevClearIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) { virCommandPtr cmd = NULL; char *addrstr; diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index e1087a4..4dd7b79 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -98,9 +98,9 @@ int virNetDevAddRoute(const char *ifname, unsigned int metric) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; -int virNetDevClearIPv4Address(const char *ifname, - virSocketAddr *addr, - unsigned int prefix) +int virNetDevClearIPAddress(const char *ifname, + virSocketAddr *addr, + unsigned int prefix) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; int virNetDevGetIPv4Address(const char *ifname, virSocketAddrPtr addr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -- 1.8.4.5

On Fri, Oct 10, 2014 at 2:03 PM, Cédric Bosdonnat <cbosdonnat@suse.com> wrote:
Hi all,
Here is a rebased version of v2. Nothing changed except the 'since' version number in the added doc that has been updated.
-- Cedric
Cédric Bosdonnat (16): Forgot to cleanup ifname_guest* in domain network def parsing Domain conf: allow more than one IP address for net devices LXC: set IP addresses to veth devices in the container lxc conf2xml: convert IP addresses Allow network capabilities hostdev to configure IP addresses lxc conf2xml: convert ip addresses for hostdev NICs Domain network devices can now have a <gateway> element lxc conf2xml: convert lxc.network.ipv[46].gateway LXC: use the new net devices gateway definition LXC: honour network devices link state Wrong place for virDomainNetIpsFormat virNetDevSetIPv4Address: libnl implementation Renamed virNetDevSetIPv4Address to virNetDevSetIPAddress virNetDevAddRoute: implementation using netlink virNetDevClearIPv4Address: netlink implementation Renamed virNetDevClearIPv4Address to virNetDevClearIPAddress
I still think that going down the netlink path is not optimal. As stated before in v2 you can just enter the network namespace and use the host tools to setup networking. This way no tools have to be installed within the container and we'd not depend on netlink with reinventing iproute2 tools.
docs/formatdomain.html.in | 39 +++ docs/schemas/domaincommon.rng | 55 +++- src/conf/domain_conf.c | 214 +++++++++++++-- src/conf/domain_conf.h | 22 +- src/libvirt_private.syms | 7 +- src/lxc/lxc_container.c | 74 ++++- src/lxc/lxc_native.c | 173 ++++++++---- src/network/bridge_driver.c | 4 +- src/openvz/openvz_conf.c | 2 +- src/openvz/openvz_driver.c | 6 +- src/qemu/qemu_driver.c | 25 +- src/qemu/qemu_hotplug.c | 6 +- src/uml/uml_conf.c | 2 +- src/util/virnetdev.c | 305 ++++++++++++++++++--- src/util/virnetdev.h | 12 +- src/util/virnetlink.c | 38 +++ src/util/virnetlink.h | 2 + src/vbox/vbox_common.c | 3 +- src/xenconfig/xen_common.c | 15 +- src/xenconfig/xen_sxpr.c | 12 +- .../lxcconf2xmldata/lxcconf2xml-physnetwork.config | 4 + tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 3 + tests/lxcconf2xmldata/lxcconf2xml-simple.config | 4 + tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 3 + tests/lxcxml2xmldata/lxc-hostdev.xml | 3 + tests/lxcxml2xmldata/lxc-idmap.xml | 3 + 26 files changed, 880 insertions(+), 156 deletions(-)
-- 1.8.4.5
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Thanks, //richard

On Fri, Oct 10, 2014 at 03:13:10PM +0200, Richard Weinberger wrote:
On Fri, Oct 10, 2014 at 2:03 PM, Cédric Bosdonnat <cbosdonnat@suse.com> wrote:
Hi all,
Here is a rebased version of v2. Nothing changed except the 'since' version number in the added doc that has been updated.
-- Cedric
Cédric Bosdonnat (16): Forgot to cleanup ifname_guest* in domain network def parsing Domain conf: allow more than one IP address for net devices LXC: set IP addresses to veth devices in the container lxc conf2xml: convert IP addresses Allow network capabilities hostdev to configure IP addresses lxc conf2xml: convert ip addresses for hostdev NICs Domain network devices can now have a <gateway> element lxc conf2xml: convert lxc.network.ipv[46].gateway LXC: use the new net devices gateway definition LXC: honour network devices link state Wrong place for virDomainNetIpsFormat virNetDevSetIPv4Address: libnl implementation Renamed virNetDevSetIPv4Address to virNetDevSetIPAddress virNetDevAddRoute: implementation using netlink virNetDevClearIPv4Address: netlink implementation Renamed virNetDevClearIPv4Address to virNetDevClearIPAddress
I still think that going down the netlink path is not optimal. As stated before in v2 you can just enter the network namespace and use the host tools to setup networking. This way no tools have to be installed within the container and we'd not depend on netlink with reinventing iproute2 tools.
We have a general aim to avoid using external commands if there are APIs available todo the same job. It is less overhead, gives us better error reporting and we're not susceptible to changes in the CLI tool syntax. So I think using the netlink is actually the right thing here. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (6)
-
Cedric Bosdonnat
-
Cédric Bosdonnat
-
Daniel P. Berrange
-
Dmitry Guryanov
-
John Ferlan
-
Richard Weinberger