On 9/13/19 4:02 PM, Michal Prívozník wrote:
On 9/13/19 4:52 PM, Laine Stump wrote:
--- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5300,6 +5300,86 @@ qemuDomainWatchdogDefValidate(const virDomainWatchdogDef *dev, }
+
3 empty lines instead of 2.
:-O
+int +qemuDomainValidateActualNetDef(const virDomainNetDef *net, + virQEMUCapsPtr qemuCaps) +{ + /* + * Validations that can only be properly checked at runtime (after + * an <interface type='network'> has been resolved to its actual + * type. + * + * (In its current form this function can still be called before + * the actual type has been resolved (e.g. at domain definition + * time), but only if the validations would SUCCEED for + * type='network'.) + */ + virDomainNetType actualType = virDomainNetGetActualType(net); + + /* Only tap/macvtap devices support multiqueue. */ + if (net->driver.virtio.queues > 1) {
I don't think that this is right. Take VIR_DOMAIN_NET_TYPE_USER for instance. It doesn't allow anything but queues == 0.
Right, I hadn't thought of the other non-tap types.
+ + if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_DIRECT || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || + actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("multiqueue network is not supported for: %s"), + virDomainNetTypeToString(actualType)); + return -1; + } +> + if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
This is actually where a single queue can be permitred. At least according to the original code.
+ qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
NULL is never passed to qemuCaps, so no need to check it.
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("multiqueue network is not supported for vhost-user " + "with this QEMU binary")); + return -1; + } + } + + /* + * Only standard tap devices support nwfilter rules, and even then only + * when *not* connected to an OVS bridge or midonet (indicated by having + * a <virtualport> element in the config) + */ + if (net->filter) { + virNetDevVPortProfilePtr vport = virDomainNetGetActualVirtPortProfile(net); + if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filterref is not supported for " + "network interfaces of type %s"), + virDomainNetTypeToString(actualType)); + return -1; + } + if (vport && vport->virtPortType != VIR_NETDEV_VPORT_PROFILE_NONE) { + /* currently none of the defined virtualport types support iptables */ + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("filterref is not supported for " + "network interfaces with virtualport type %s"), + virNetDevVPortTypeToString(vport->virtPortType)); + return -1; + } + } + + if (net->backend.tap && + !(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || + actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || + actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Custom tap device path is not supported for: %s"), + virDomainNetTypeToString(actualType)); + return -1; + } + + return 0; + }
s/^ //
That's strange - I ran make syntax-check multiple times, and it always finds those...
ACK with this squashed in:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ebbe1a85db..fa0dd888c8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -5300,7 +5300,6 @@ qemuDomainWatchdogDefValidate(const virDomainWatchdogDef *dev, }
- int qemuDomainValidateActualNetDef(const virDomainNetDef *net, virQEMUCapsPtr qemuCaps) @@ -5318,8 +5317,7 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, virDomainNetType actualType = virDomainNetGetActualType(net);
/* Only tap/macvtap devices support multiqueue. */ - if (net->driver.virtio.queues > 1) { - + if (net->driver.virtio.queues > 0) { if (!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT || @@ -5331,8 +5329,9 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, return -1; }
- if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER && - qemuCaps && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) { + if (net->driver.virtio.queues > 1 && + actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("multiqueue network is not supported for vhost-user " "with this QEMU binary")); @@ -5377,7 +5376,7 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net, }
return 0; - } +}
That all looks agreeable to me. Thanks!