[libvirt] [PATCH 2/3] qemu: move runtime netdev validation into a separate function
Michal Prívozník
mprivozn at redhat.com
Fri Sep 13 20:02:24 UTC 2019
On 9/13/19 4:52 PM, Laine Stump wrote:
> The same validation should be done for both static network devices and
> hotplugged devices, but they are currently inconsistent. Move all the
> relevant validation from qemuBuildInterfaceCommandLine() into the new
> function qemuDomainValidateActualNetDef() and call the latter from
> the former.
>
> Signed-off-by: Laine Stump <laine at redhat.com>
> ---
> src/qemu/qemu_command.c | 52 +--------------------------
> src/qemu/qemu_domain.c | 80 +++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_domain.h | 4 +++
> 3 files changed, 85 insertions(+), 51 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f795f2e987..2acae3bf33 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -8352,50 +8352,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> if (!bootindex)
> bootindex = net->info.bootIndex;
>
> - /* Currently nothing besides TAP devices supports multiqueue. */
> - if (net->driver.virtio.queues > 0 &&
> - !(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));
> + if (qemuDomainValidateActualNetDef(net, qemuCaps) < 0)
> return -1;
> - }
> -
> - /* and only TAP devices support nwfilter rules */
> - 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;
> - }
>
> switch (actualType) {
> case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -8458,14 +8416,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
> case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
> requireNicdev = true;
>
> - if (net->driver.virtio.queues > 1 &&
> - !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> - _("multi-queue is not supported for vhost-user "
> - "with this QEMU binary"));
> - goto cleanup;
> - }
> -
> if (qemuInterfaceVhostuserConnect(driver, logManager, secManager,
> cmd, def, net, qemuCaps, &chardev) < 0)
> goto cleanup;
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index bd247628cb..ebbe1a85db 100644
> --- 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;
- }
+}
Michal
More information about the libvir-list
mailing list