conf/openvz: eliminate incorrect/undocumented use of <source dev='blah'/>

When support for <interface type='ethernet'> was added in commit
9a4b705f back in 2010, it erroneously looked at <source dev='blah'/>
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 <guest
dev='blah'/>.

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
<guest dev='blah'/> 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 <source dev='blah'/> for a type='ethernet'
interface, it also removes the cargo-cult uses of ethernet.dev and
<source dev='blah'/>, 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.

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 162c2e016480f96ef5fd88b055784f60675a7143..b81b558d9174a22358d82b32f7700ba4314fe473 100644 (file)
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2142,9 +2142,6 @@
           <interleave>
             <optional>
               <element name="source">
-                <attribute name="dev">
-                  <ref name="deviceName"/>
-                </attribute>
                 <empty/>
               </element>
             </optional>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3a29aaeeb4f3a9bf474f2a47d8bd061a744d72bf..a1d835d8c704b38777ade2d562d7bfe401f9021d 100644 (file)
--- 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 <source dev='blah'/>.  That
+                 * was blatant misuse of <source>, 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 <interface type='ethernet'> "
+                                     "device name with <source dev='%s'/>. "
+                                     "Use <target dev='%s'/> (for host-side) "
+                                     "or <guest dev='%s'/> (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, "<source dev='%s'/>\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 b9dc174c63c2125374c5c85e43e5891d71f4e11e..d599551a6e0c2c00538a2f3402356bcb288ef59f 100644 (file)
--- 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 b66883f06611424d7c2b2f479ccc1c236182fb6a..b114246fa82993fb3275a6039cb1103b8fb2cba0 100644 (file)
--- 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 f695903f1c1c8d47d74e79d16d0397bed94417e6..e0b82306dc73179f098fa97faadf3b3a52a13dc3 100644 (file)
--- 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 f34dbaa5a2f2e1187de636348b2c7bd1779951f5..2adc3a79410fcdd48369f6998dabc910e5f08578 100644 (file)
--- a/tests/xml2sexprdata/xml2sexpr-net-routed.xml
+++ b/tests/xml2sexprdata/xml2sexpr-net-routed.xml
@@ -20,7 +20,6 @@
     <interface type="ethernet">
       <mac address="00:11:22:33:44:55"/>
       <ip address="172.14.5.6"/>
-      <source dev="eth3"/>
       <script path="vif-routed"/>
       <target dev="vif4.0"/>
     </interface>