From 9658e70f7d957570bb0682a9aadc64ae67b3b201 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Tue, 21 Jun 2016 15:20:57 -0400 Subject: [PATCH] conf/openvz: eliminate incorrect/undocumented use of When support for was added in commit 9a4b705f back in 2010, it erroneously looked at for a user-specified guest-side interface name. This was never documented though. (that attribute already existed at the time in the data.ethernet union member of virDomainNetDef, but apparently had no practical use - it was only used as a storage place for a NetDef's bridge name during qemuDomainXMLToNative(), but even then that was never used for anything). When support for similar guest-side device naming was added to the lxc driver several years later, it was put in a new subelement . In the intervening years, since there was no validation that ethernet.dev was NULL in the other drivers that didn't actually use it, innocent souls who were adding other features assuming they needed to account for non-NULL ethernet.dev when really they didn't, so little bits of the usual pointless cargo-cult code showed up. This patch not only switches the openvz driver to use the documented notation for naming the guest-side device (just in case anyone is still using the openvz driver), and logs an error if anyone tries to set for a type='ethernet' interface, it also removes the cargo-cult uses of ethernet.dev and , and eliminates if from the RNG and from virDomainNetDef. NB: I decided on this course of action after mentioning the inconsistency here: https://www.redhat.com/archives/libvir-list/2016-May/msg02038.html and getting encouragement do eliminate it in a later IRC discussion with danpb. --- docs/schemas/domaincommon.rng | 3 -- src/conf/domain_conf.c | 40 ++++++++++++-------- src/conf/domain_conf.h | 3 -- src/openvz/openvz_driver.c | 5 +-- src/qemu/qemu_hotplug.c | 6 +-- tests/xml2sexprdata/xml2sexpr-net-routed.xml | 1 - 6 files changed, 27 insertions(+), 31 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 162c2e0164..b81b558d91 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2142,9 +2142,6 @@ - - - diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3a29aaeeb4..a1d835d8c7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1748,10 +1748,6 @@ virDomainNetDefClear(virDomainNetDefPtr def) VIR_FREE(def->model); switch (def->type) { - case VIR_DOMAIN_NET_TYPE_ETHERNET: - VIR_FREE(def->data.ethernet.dev); - break; - case VIR_DOMAIN_NET_TYPE_VHOSTUSER: virDomainChrSourceDefFree(def->data.vhostuser); def->data.vhostuser = NULL; @@ -1788,6 +1784,7 @@ virDomainNetDefClear(virDomainNetDefPtr def) virDomainHostdevDefClear(&def->data.hostdev.def); break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -9019,12 +9016,31 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, def->type == VIR_DOMAIN_NET_TYPE_BRIDGE && xmlStrEqual(cur->name, BAD_CAST "source")) { bridge = virXMLPropString(cur, "bridge"); - } else if (!dev && - (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - def->type == VIR_DOMAIN_NET_TYPE_DIRECT) && + } else if (!dev && def->type == VIR_DOMAIN_NET_TYPE_DIRECT && xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if (!dev && def->type == VIR_DOMAIN_NET_TYPE_ETHERNET && + xmlStrEqual(cur->name, BAD_CAST "source")) { + /* This clause is only necessary because from 2010 to + * 2016 it was possible (but never documented) to + * configure the name of the guest-side interface of + * an openvz domain with . That + * was blatant misuse of , so was likely + * (hopefully) never used, but just in case there was + * somebody using it, we need to generate an error. If + * the openvz driver is ever deprecated, this clause + * can be removed from here. + */ + if ((dev = virXMLPropString(cur, "dev"))) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid attempt to set " + "device name with . " + "Use (for host-side) " + "or (for guest-side) instead."), + dev, dev, dev); + goto error; + } } else if (!vhostuser_path && !vhostuser_mode && !vhostuser_type && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER && xmlStrEqual(cur->name, BAD_CAST "source")) { @@ -9274,13 +9290,6 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } break; - case VIR_DOMAIN_NET_TYPE_ETHERNET: - if (dev != NULL) { - def->data.ethernet.dev = dev; - dev = NULL; - } - break; - case VIR_DOMAIN_NET_TYPE_BRIDGE: if (bridge == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -9409,6 +9418,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } break; + case VIR_DOMAIN_NET_TYPE_ETHERNET: case VIR_DOMAIN_NET_TYPE_USER: case VIR_DOMAIN_NET_TYPE_LAST: break; @@ -20802,8 +20812,6 @@ virDomainNetDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - virBufferEscapeString(buf, "\n", - def->data.ethernet.dev); break; case VIR_DOMAIN_NET_TYPE_VHOSTUSER: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b9dc174c63..d599551a6e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -930,9 +930,6 @@ struct _virDomainNetDef { char *vhost; } backend; union { - struct { - char *dev; - } ethernet; virDomainChrSourceDefPtr vhostuser; struct { char *address; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b66883f066..b114246fa8 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -862,9 +862,8 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, /* if net is ethernet and the user has specified guest interface name, * let's use it; otherwise generate a new one */ - if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET && - net->data.ethernet.dev != NULL) { - if (VIR_STRDUP(guest_ifname, net->data.ethernet.dev) == -1) + if (net->ifname_guest) { + if (VIR_STRDUP(guest_ifname, net->ifname_guest) < 0) goto cleanup; } else { guest_ifname = openvzGenerateContainerVethName(veid); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f695903f1c..e0b82306dc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2345,11 +2345,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, break; case VIR_DOMAIN_NET_TYPE_ETHERNET: - if (STRNEQ_NULLABLE(olddev->data.ethernet.dev, - newdev->data.ethernet.dev)) { - needReconnect = true; - } - break; + break; case VIR_DOMAIN_NET_TYPE_SERVER: case VIR_DOMAIN_NET_TYPE_CLIENT: diff --git a/tests/xml2sexprdata/xml2sexpr-net-routed.xml b/tests/xml2sexprdata/xml2sexpr-net-routed.xml index f34dbaa5a2..2adc3a7941 100644 --- a/tests/xml2sexprdata/xml2sexpr-net-routed.xml +++ b/tests/xml2sexprdata/xml2sexpr-net-routed.xml @@ -20,7 +20,6 @@ -