Re: [PATCH] network: disallow <bandwidth>/<mac> for bridged/macvtap networks

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

 



On 24/01/14 14:18 +0200, Laine Stump wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1057321 pointed out that
we weren't honoring the <bandwidth> element in libvirt networks using
<forward mode='bridge'/>. In fact, these networks are just a method of
giving a libvirt network name to an existing Linux host bridge on the
system, and even if it were technically possible for us to set
network-wide bandwidth limits for all the taps on a bridge, it's
probably not a polite thing to do since libvirt is just using a bridge
that was created by someone else for other purposes. So the proper
thing is to just log an error when someone tries to put a <bandwidth>
element in that type of network.

While looking through the network XML documentation and comparing it
to the networkValidate function, I noticed that we also ignore the
presence of a mac address in the config, even though we do nothing
with it in this case either.

This patch updates networkValidate() (which is called any time a
persistent network is defined, or a transient network created) to log
an error and fail if it finds either a <bandwidth> or <mac> element
and the network forward mode is anything except 'route'. 'nat', or
nothing. (Yes, neither of those elements is acceptable for any macvtap
mode, nor for a hostdev network).

NB: This does *not* cause failure to start any existing network that
contains one of those elements, so someone might have erroneously
defined such a network in the past, and that network will continue to
function unmodified. I considered it too disruptive to suddenly break
working configs on the next reboot after a libvirt upgrade.

Reviewed-by: Adam Litke <alitke@xxxxxxxxxx>

---
src/network/bridge_driver.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0b43a67..3b9b58d 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2407,8 +2407,17 @@ networkValidate(virNetworkDriverStatePtr driver,
        virNetworkSetBridgeMacAddr(def);
    } else {
        /* They are also the only types that currently support setting
-         * an IP address for the host-side device (bridge)
+         * a MAC or IP address for the host-side device (bridge), DNS
+         * configuration, or network-wide bandwidth limits.
         */
+        if (def->mac_specified) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported <mac> element in network %s "
+                             "with forward mode='%s'"),
+                           def->name,
+                           virNetworkForwardTypeToString(def->forward.type));
+            return -1;
+        }
        if (virNetworkDefGetIpByIndex(def, AF_UNSPEC, 0)) {
            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                           _("Unsupported <ip> element in network %s "
@@ -2433,6 +2442,14 @@ networkValidate(virNetworkDriverStatePtr driver,
                           virNetworkForwardTypeToString(def->forward.type));
            return -1;
        }
+        if (def->bandwidth) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("Unsupported network-wide <bandwidth> element "
+                             "in network %s with forward mode='%s'"),
+                           def->name,
+                           virNetworkForwardTypeToString(def->forward.type));
+            return -1;
+        }
    }

    /* We only support dhcp on one IPv4 address and
--
1.8.5.3

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

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




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