Re: [libvirt] [PATCH 2/3] qemu: move runtime netdev validation into a separate function

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.

> +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.

> + > + 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/^ //

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; - } +}