Re: [libvirt PATCH 09/10] conf: extra validation for <portOptions isolated='yes'/>

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

 



On 2/18/20 12:52 PM, Ján Tomko wrote:
On Sun, Feb 16, 2020 at 11:22:58PM -0500, Laine Stump wrote:
During the hypervisor-agnostic validation of network devices, verify
that the interface type is either "network" or "bridge", and that if
there is any <virtualport>, that it doesn't have any type associated
with it.

This needs to be done both for the parse-time validation and for
runtime validation (after a port has been acquired from any associated
network), because an interface with type='network' could have an
actual type at runtime of "hostdev" or "direct", neither of which
support isolated='true' (yet). Likewise, if an interface is
type='network', then at runtime a <virtualport> with a type that
doesn't support isolated='yes' (e.g. "openvswitch", "802.1Qbh" -
currently *none* of the available virtualport types support it)

Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---
src/conf/domain_conf.c | 56 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 56 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 30b2a53b83..f8ce7d519d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6239,6 +6239,47 @@ virDomainRedirdevDefValidate(const virDomainDef *def,
}


+static int
+virDomainNetDefValidatePortOptions(const char *macstr,
+                                   virDomainNetType type,
+                                   const virNetDevVPortProfile *vport,
+                                   virTristateBool isolatedPort)
+{
+    /*
+     * This function can be called for either a config interface
+     * object (NetDef) or a runtime interface object (ActualNetDef),
+     * by calling it with either, e.g., the "type" (what is in the
+     * config) or the "actualType" (what is determined at runtime by
+     * acquiring a port from the network).
+     */
+    /*
+     * port isolation can only be set for an interface that is
+     * connected to a Linux host bridge (either a libvirt-managed
+     * network, or plain type='bridge')
+     */
+    if (isolatedPort == VIR_TRISTATE_BOOL_YES) {
+        if (!(type == VIR_DOMAIN_NET_TYPE_NETWORK ||
+              type == VIR_DOMAIN_NET_TYPE_BRIDGE)) {

consider:
         if (type != VIR_DOMAIN_NET_TYPE_NETWORK &&
             type != VIR_DOMAIN_NET_TYPE_BRIDGE)

Tomatoes, potatoes, sure :-)


+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("interface %s - <portOptions isolated='yes'/> is not supported for network interfaces with type='%s'"),

Please don't put XML snippets in the error message.

Because... ?

How about:
     ... - isolated ports are not supported ...

Wouldn't that make it more likely that the word "isolated" (and maybe even "port") would be translated, and the resulting error message less useful to the end user? If it's explicitly an XML element or attribute, then at least a translator who knew a bit about libvirt would realize that it's the name of an element/attribute and shouldn't be translated.

(But maybe there was a discussion about this somewhere, it was debated, and I missed it. If that's become a rule of log messages then of course I'll follow it.)


+                           macstr, virDomainNetTypeToString(type));
+            return -1;
+        }
+        /*
+         * also not allowed for anything with <virtualport> setting
+         * (openvswitch or 802.11Qb[gh])
+         */
+        if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("interface %s - <portOptions isolated='yes'/> is not supported for network interfaces with virtualport type='%s'"), +                           macstr, virNetDevVPortTypeToString(vport->virtPortType));

Same here.

+            return -1;
+        }
+    }
+    return 0;
+}
+
+
int
virDomainActualNetDefValidate(const virDomainNetDef *net)
{

Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>

Jano





[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]

  Powered by Linux