[libvirt] [PATCH 0/3] revert publicly visible parts of IP address "peer" patches

Due to the feature not working, and a couple questions about the design that haven't been resolved and may require publicly visible changes, these patches temporarily revert the three patches that add support for the ip element "peer" attribute to qemu and lxc. (Note that I left in the patch that changes virNetDevSetIPAddress(), because it is correct (except a problem on FreeBSD that only occurs at runtime, and only if a peer address is given, which won't happen as long as this feature is removed). I posted patches yesterday that fixed most of the problems, but they are too numerous to consider pushing before release, and anyway don't fix everything. Laine Stump (3): Revert "qemu domain allow to set ip address, peer address and route" Revert "lxc domain allow to set peer address" Revert "libvirt domain xml allow to set peer address" docs/formatdomain.html.in | 12 +----------- docs/schemas/domaincommon.rng | 5 ----- src/conf/domain_conf.c | 14 +------------- src/conf/domain_conf.h | 1 - src/lxc/lxc_container.c | 2 +- src/qemu/qemu_interface.c | 41 +---------------------------------------- 6 files changed, 4 insertions(+), 71 deletions(-) -- 2.5.5

This reverts commit 6e244c659fb004503fd2a94319dae8fa6fecf045, which added support to qemu for the "peer" attribute in domain interface <ip> elements. It's being removed temporarily for the release of libvirt 1.3.4 because the feature doesn't work, and there are concerns that it may need to be modified in an externally visible manner which could create backward compatibility problems. Conflicts: tests/qemuxml2argvmock.c - a mock of virNetDevSetOnline() was added which may be assumed by other tests added since the original commit, so it isn't being reverted. --- src/qemu/qemu_interface.c | 41 +---------------------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/src/qemu/qemu_interface.c b/src/qemu/qemu_interface.c index a4e9d86..ef789fa 100644 --- a/src/qemu/qemu_interface.c +++ b/src/qemu/qemu_interface.c @@ -411,7 +411,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, bool template_ifname = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *tunpath = "/dev/net/tun"; - size_t i; if (net->backend.tap) { tunpath = net->backend.tap; @@ -448,45 +447,6 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, if (virNetDevSetMAC(net->ifname, &tapmac) < 0) goto cleanup; - for (i = 0; i < net->nips; i++) { - virDomainNetIpDefPtr ip = net->ips[i]; - unsigned int prefix = (ip->prefix > 0) ? ip->prefix : - VIR_SOCKET_ADDR_DEFAULT_PREFIX; - char *ipStr = virSocketAddrFormat(&ip->address); - - VIR_DEBUG("Adding IP address '%s/%u' to '%s'", - ipStr, ip->prefix, net->ifname); - - if (virNetDevSetIPAddress(net->ifname, &ip->address, &ip->peer, prefix) < 0) { - virReportError(VIR_ERR_SYSTEM_ERROR, - _("Failed to set IP address '%s' on %s"), - ipStr, net->ifname); - VIR_FREE(ipStr); - goto cleanup; - } - VIR_FREE(ipStr); - } - - if (net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP || - net->linkstate == VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT) { - if (virNetDevSetOnline(net->ifname, true) < 0) - goto cleanup; - - /* Set the routes */ - for (i = 0; i < net->nroutes; i++) { - virNetworkRouteDefPtr route = net->routes[i]; - - if (virNetDevAddRoute(net->ifname, - virNetworkRouteDefGetAddress(route), - virNetworkRouteDefGetPrefix(route), - virNetworkRouteDefGetGateway(route), - virNetworkRouteDefGetMetric(route)) < 0) { - goto cleanup; - } - } - } - - if (net->script && virNetDevRunEthernetScript(net->ifname, net->script) < 0) goto cleanup; @@ -506,6 +466,7 @@ qemuInterfaceEthernetConnect(virDomainDefPtr def, cleanup: if (ret < 0) { + size_t i; for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++) VIR_FORCE_CLOSE(tapfd[i]); if (template_ifname) -- 2.5.5

This reverts commit afee47d07c521b4bb10a12e253a29363d16ab8f0, which added support to lxc for the "peer" attribute in domain interface <ip> elements. It's being removed temporarily for the release of libvirt 1.3.4 because the feature doesn't work, and there are concerns that it may need to be modified in an externally visible manner which could create backward compatibility problems. --- src/lxc/lxc_container.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index a1deb0c..a909b66 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -520,7 +520,7 @@ static int lxcContainerRenameAndEnableInterfaces(virDomainDefPtr vmDef, VIR_DEBUG("Adding IP address '%s/%u' to '%s'", ipStr, ip->prefix, newname); - if (virNetDevSetIPAddress(newname, &ip->address, &ip->peer, prefix) < 0) { + if (virNetDevSetIPAddress(newname, &ip->address, NULL, prefix) < 0) { virReportError(VIR_ERR_SYSTEM_ERROR, _("Failed to set IP address '%s' on %s"), ipStr, newname); -- 2.5.5

This reverts commit 690969af9ccdf9b2012b97af0462bae8e312c9c9, which added the domain config parts to support a "peer" attribute in domain interface <ip> elements. It's being removed temporarily for the release of libvirt 1.3.4 because the feature doesn't work, and there are concerns that it may need to be modified in an externally visible manner which could create backward compatibility problems. --- docs/formatdomain.html.in | 12 +----------- docs/schemas/domaincommon.rng | 5 ----- src/conf/domain_conf.c | 14 +------------- src/conf/domain_conf.h | 1 - 4 files changed, 2 insertions(+), 30 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index d2db53b..0688616 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4836,7 +4836,6 @@ qemu-kvm -net nic,model=? /dev/null <source network='default'/> <target dev='vnet0'/> <b><ip address='192.168.122.5' prefix='24'/></b> - <b><ip address='192.168.122.5' prefix='24' peer='10.0.0.10'/></b> <b><route family='ipv4' address='192.168.122.0' prefix='24' gateway='192.168.122.1'/></b> <b><route family='ipv4' address='192.168.122.8' gateway='192.168.122.1'/></b> </interface> @@ -4869,16 +4868,7 @@ qemu-kvm -net nic,model=? /dev/null to define the network routes to use for the network device. The attributes of this element are described in the documentation for the <code>route</code> element in <a href="formatnetwork.html#elementsStaticroute">network definitions</a>. - This is used by the LXC driver and <span class="since">Since 1.3.3</span> by the QEMU - driver. - </p> - - <p> - <span class="since">Since 1.3.3</span> ip elements can hold peer attribute to assign - a point-to-point address for the network device. The attributes of this element - are described in the documentation for the <code>ip</code> element in - <a href="formatnetwork.html#elementsAddress">network definitions</a>. - This is only used by the LXC and QEMU drivers. + This is only used by the LXC driver. </p> <h5><a name="elementVhostuser">vhost-user interface</a></h5> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f77bbcf..f143bf0 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2410,11 +2410,6 @@ <ref name="ipPrefix"/> </attribute> </optional> - <optional> - <attribute name="peer"> - <ref name="ipAddr"/> - </attribute> - </optional> <empty/> </element> </zeroOrMore> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d8bed67..d93d981 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5744,7 +5744,7 @@ virDomainNetIpParseXML(xmlNodePtr node) unsigned int prefixValue = 0; char *familyStr = NULL; int family = AF_UNSPEC; - char *address = NULL, *peer = NULL; + char *address = NULL; if (!(prefixStr = virXMLPropString(node, "prefix")) || (virStrToLong_ui(prefixStr, NULL, 10, &prefixValue) < 0)) { @@ -5758,9 +5758,6 @@ virDomainNetIpParseXML(xmlNodePtr node) goto cleanup; } - if ((peer = virXMLPropString(node, "peer")) == NULL) - VIR_DEBUG("Peer is empty"); - familyStr = virXMLPropString(node, "family"); if (familyStr && STREQ(familyStr, "ipv4")) family = AF_INET; @@ -5778,14 +5775,6 @@ virDomainNetIpParseXML(xmlNodePtr node) address); goto cleanup; } - - if ((peer != NULL) && (virSocketAddrParse(&ip->peer, peer, family) < 0)) { - virReportError(VIR_ERR_INVALID_ARG, - _("Failed to parse IP address: '%s'"), - peer); - goto cleanup; - } - ip->prefix = prefixValue; ret = ip; @@ -5795,7 +5784,6 @@ virDomainNetIpParseXML(xmlNodePtr node) VIR_FREE(prefixStr); VIR_FREE(familyStr); VIR_FREE(address); - VIR_FREE(peer); VIR_FREE(ip); return ret; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 31e7a86..1433900 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -514,7 +514,6 @@ typedef struct _virDomainNetIpDef virDomainNetIpDef; typedef virDomainNetIpDef *virDomainNetIpDefPtr; struct _virDomainNetIpDef { virSocketAddr address; /* ipv4 or ipv6 address */ - virSocketAddr peer; /* ipv4 or ipv6 address of peer */ unsigned int prefix; /* number of 1 bits in the net mask */ }; -- 2.5.5

On Fri, Apr 29, 2016 at 12:04:28PM -0400, Laine Stump wrote:
Due to the feature not working, and a couple questions about the design that haven't been resolved and may require publicly visible changes, these patches temporarily revert the three patches that add support for the ip element "peer" attribute to qemu and lxc. (Note that I left in the patch that changes virNetDevSetIPAddress(), because it is correct (except a problem on FreeBSD that only occurs at runtime, and only if a peer address is given, which won't happen as long as this feature is removed).
I posted patches yesterday that fixed most of the problems, but they are too numerous to consider pushing before release, and anyway don't fix everything.
ACK all three 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 (2)
-
Daniel P. Berrange
-
Laine Stump