On 6/28/21 11:18 AM, zhangjl02 wrote:

From: zhangjl02 <zhangjl02@inspur.com>

Tell whether a port definition is an ovs managed virtual port --- src/qemu/qemu_domain.c | 13 +++++++++++++ src/qemu/qemu_domain.h | 3 +++ 2 files changed, 16 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..da5a226fc2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11575,3 +11575,16 @@ qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg,

return 0; } + +/* + * Check whether the port is an ovs managed port + */ +bool qemuDomainDefIsOvsport(virDomainNetDef *net, + int actualType) { + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); + if ((actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && vport && + vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + return true; + } + return false; +} \ No newline at end of file diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index acf6ca5ab6..81a9bf0cfb 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -1063,3 +1063,6 @@ int qemuDomainNamePathsCleanup(virQEMUDriverConfig *cfg, const char *name, bool bestEffort); + +bool qemuDomainDefIsOvsport(virDomainNetDef *net, + int actualType); \ No newline at end of file

There's nothing QEMU specific in this function. I think it can go into src/conf/domain_conf.c (somewhere near virDomainNetGetActualVirtPortProfile() perhaps) and that check from the function - we have a few places like it, those can be replaced with the function call. Michal

On 6/28/21 11:18 AM, zhangjl02 wrote:

From: zhangjl02 <zhangjl02@inspur.com>

Introduce qos setting and cleaning method. Use ovs command to set qos parameters on specific interface of qemu virtual machine.

When an ovs port is created, we add 'ifname' to external-ids. When setting qos on an ovs port, query its qos and queue. If found, change qos on queried queue and qos, otherwise create new queue and qos. When cleaning qos, query and clean queues and qos in ovs table record by 'ifname' and 'vmid'. --- src/libvirt_private.syms | 2 + src/util/virnetdevopenvswitch.c | 269 ++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 11 ++ 3 files changed, 282 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68e4b6aab8..775f6706b3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2805,8 +2805,10 @@ virNetDevMidonetUnbindPort; virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchGetVhostuserIfname; +virNetDevOpenvswitchInterfaceClearQos; virNetDevOpenvswitchInterfaceGetMaster; virNetDevOpenvswitchInterfaceParseStats; +virNetDevOpenvswitchInterfaceSetQos; virNetDevOpenvswitchInterfaceStats; virNetDevOpenvswitchMaybeUnescapeReply; virNetDevOpenvswitchRemovePort; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index eac68d9556..f27b74f35f 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -30,6 +30,7 @@ #include "virlog.h" #include "virjson.h" #include "virfile.h" +#include "virutil.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -140,6 +141,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, g_autofree char *ifaceid_ex_id = NULL; g_autofree char *profile_ex_id = NULL; g_autofree char *vmid_ex_id = NULL; + g_autofree char *ifname_ex_id = NULL;

virMacAddrFormat(macaddr, macaddrstr); virUUIDFormat(ovsport->interfaceID, ifuuidstr); @@ -149,6 +151,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, macaddrstr); ifaceid_ex_id = g_strdup_printf("external-ids:iface-id=\"%s\"", ifuuidstr); vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); if (ovsport->profileID[0] != '\0') { profile_ex_id = g_strdup_printf("external-ids:port-profile=\"%s\"", ovsport->profileID); @@ -174,6 +177,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname, "--", "set", "Interface", ifname, ifaceid_ex_id, "--", "set", "Interface", ifname, vmid_ex_id, "--", "set", "Interface", ifname, profile_ex_id, + "--", "set", "Interface", ifname, ifname_ex_id, "--", "set", "Interface", ifname, "external-ids:iface-status=active", NULL); @@ -614,3 +618,268 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,

return 0; } + + +/** + * virNetDevOpenvswitchInterfaceSetQos: + * @ifname: on which interface + * @bandwidth: rates to set (may be NULL) + * @swapped: true if IN/OUT should be set contrariwise + * + * Update qos configuration of an OVS port. + * + * If @swapped is set, the IN part of @bandwidth is set on + * @ifname's TX, and vice versa. If it is not set, IN is set on + * RX and OUT on TX. This is because for some types of interfaces + * domain and the host live on the same side of the interface (so + * domain's RX/TX is host's RX/TX), and for some it's swapped + * (domain's RX/TX is hosts's TX/RX). + * + * Return 0 on success, -1 otherwise. + */ +int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, + const virNetDevBandwidth *bandwidth, + const unsigned char *vmid, + bool swapped) +{ + int ret = -1; + char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + virNetDevBandwidthRate *rx = NULL; /* From domain POV */ + virNetDevBandwidthRate *tx = NULL; /* From domain POV */ + virCommand *cmd = NULL; + char *vmid_ex_id = NULL; + char *ifname_ex_id = NULL; + char *average = NULL; + char *peak = NULL; + char *burst = NULL; + char *qos_uuid = NULL; + char *queue_uuid = NULL; + char **lines = NULL; + + if (!bandwidth) { + /* nothing to be enabled */ + ret = 0; + goto cleanup; + } + + if (geteuid() != 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Network bandwidth tuning is not available" + " in session mode")); + return -1; + } + + if (!ifname) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Unable to set bandwidth for interface because " + "device name is unknown")); + return -1; + } + + if (swapped) { + rx = bandwidth->out; + tx = bandwidth->in; + } else { + rx = bandwidth->in; + tx = bandwidth->out; + } + if(!bandwidth->out && !bandwidth->in) { + ret = virNetDevOpenvswitchInterfaceClearQos(ifname, vmid); + // virNetDevBandwidthClear(iframe);

A leftover probably? Also, we don't really use C99 like comments. We're old school and use C89 style.

+ } + if (tx && tx->average) { + average = g_strdup_printf("%llu", tx->average * 8192); + if (tx->burst) + burst = g_strdup_printf("%llu", tx->burst * 8192); + if (tx->peak) + peak = g_strdup_printf("%llu", tx->peak * 8192);

Let me just say that finding ANY documentation for this part of OVS wasn't easy. Initially, I though that these multiplications (to get bits/s) were wrong, but after digging through OVS code - they are indeed correct. Which is not at all consistent with .. [1]

+ + // find queue + cmd = virNetDevOpenvswitchCreateCmd(); + virUUIDFormat(vmid, vmuuidstr); + vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); + virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "queue", vmid_ex_id, ifname_ex_id,NULL); + virCommandSetOutputBuffer(cmd, &queue_uuid); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find queue on port %s"), ifname); + } + + // find qos + cmd = virNetDevOpenvswitchCreateCmd();

You need to free cmd before reusing it, like this: virCommandFree(cmd); The same applies for other variables, that are reused within the funcion (average, burst, peak, ...).

+ virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "qos", vmid_ex_id, ifname_ex_id,NULL); + virCommandSetOutputBuffer(cmd, &qos_uuid); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find qos on port %s"), ifname); + } + + // create qos and set + cmd = virNetDevOpenvswitchCreateCmd(); + if (queue_uuid && *queue_uuid) { + lines = g_strsplit(queue_uuid, "\n", 0); + virCommandAddArgList(cmd, "set", "queue", lines[0], NULL); + }else{ + virCommandAddArgList(cmd, "set", "port", ifname, "qos=@qos1", vmid_ex_id, ifname_ex_id, + "--", "--id=@qos1", "create", "qos", "type=linux-htb", NULL); + virCommandAddArgFormat(cmd, "other_config:min-rate=%s", average); + if(burst){ + virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); + } + if(peak){ + virCommandAddArgFormat(cmd, "other_config:max-rate=%s", peak); + } + virCommandAddArgList(cmd, "queues:0=@queue0", vmid_ex_id, ifname_ex_id, + "--", "--id=@queue0", "create", "queue", NULL); + } + virCommandAddArgFormat(cmd, "other_config:min-rate=%s", average); + if(burst){ + virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); + } + if(peak){ + virCommandAddArgFormat(cmd, "other_config:max-rate=%s", peak); + } + virCommandAddArgList(cmd, vmid_ex_id, ifname_ex_id, NULL); + if (virCommandRun(cmd, NULL) < 0) { + if(*queue_uuid){ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set queue configuration on port %s"), ifname); + } + else + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to create and set qos configuration on port %s"), ifname);

If one branch of if() has curly brackets then the other must have them too. Moreover, if a branch doesn't fit into one line then it must have curly braces. So effectivelly, this should be written as: if (*queue_uuid) { virReportError(.. ..); } else { virReportError(.. ..); }

+ goto cleanup; + } + + if(qos_uuid && *qos_uuid){ + cmd = virNetDevOpenvswitchCreateCmd(); + lines = g_strsplit(qos_uuid, "\n", 0); + virCommandAddArgList(cmd, "set", "qos", lines[0], NULL); + virCommandAddArgFormat(cmd, "other_config:min-rate=%s", average); + if(burst){ + virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); + } + if(peak){ + virCommandAddArgFormat(cmd, "other_config:max-rate=%s", peak); + } + virCommandAddArgList(cmd, vmid_ex_id, ifname_ex_id, NULL); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set qos configuration on port %s"), ifname); + goto cleanup; + } + } + + // ret = virNetDevBandwidthSet(ifname,bandwidth,false,swapped); + } + + if (rx) { + average = g_strdup_printf("%llu", rx->average * 8); + if (rx->burst) + burst = g_strdup_printf("%llu", rx->burst * 8);

1: this. Here the values are in Kbps. Thus we need to multiply by 8. Le sigh.

+ cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); + virCommandAddArgFormat(cmd, "ingress_policing_rate=%s", average); + if (burst) + virCommandAddArgFormat(cmd, "ingress_policing_burst=%s", burst); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set vlan configuration on port %s"), ifname); + goto cleanup; + } + } + + return 0; + + cleanup: + virCommandFree(cmd); + VIR_FREE(average); + VIR_FREE(peak); + VIR_FREE(burst); + VIR_FREE(qos_uuid); + VIR_FREE(queue_uuid); + VIR_FREE(vmid_ex_id); + VIR_FREE(ifname_ex_id); + VIR_FREE(lines);

All these free calls can be left out if you'd use g_auto* for those variables. Then, this cleanup label won't be needed really, becuase it would contain just one line ..

+ return ret;

.. return ret;

+} + +int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, + const unsigned char *vmid){

My comments from above apply for this function too.

+ char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + virCommand *cmd = NULL; + char *vmid_ex_id = NULL; + char *qos_uuid = NULL; + char *queue_uuid = NULL; + char **lines = NULL; + size_t i; + + // find qos + cmd = virNetDevOpenvswitchCreateCmd(); + virUUIDFormat(vmid, vmuuidstr); + vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "qos", vmid_ex_id, NULL); + virCommandSetOutputBuffer(cmd, &qos_uuid); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find qos on port %s"), ifname);

Neither of these virReportError() are coupled with return -1. I kind of understand that, because you want this to be best effort, but maybe this function should be void then?

+ } + + // find queue + cmd = virNetDevOpenvswitchCreateCmd(); + vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "queue", vmid_ex_id, NULL); + virCommandSetOutputBuffer(cmd, &queue_uuid); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find queue on port %s"), ifname); + } + + if (qos_uuid && *qos_uuid) { + lines = g_strsplit(qos_uuid, "\n", 0); + // destroy qos + for (i = 0; lines[i] != NULL; i++) { + const char *line = lines[i]; + if (!*line) { + continue; + } + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "remove", "port", ifname, "qos", line, NULL); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to remove port qos on port %s"), ifname);

This is slightly weird. After 4/4 this code is executed when a guest is shut off. However, I see that the port is removed sonned than this function is called thus running this command fails. This is what I see in the log: 2021-06-29 12:52:32.658+0000: 15442: debug : virCommandRunAsync:2627 : About to run /usr/bin/ovs-vsctl --timeout=5 -- --if-exists del-port vnet2 2021-06-29 12:52:32.662+0000: 15442: debug : virCommandRunAsync:2629 : Command result 0, with PID 29124 2021-06-29 12:52:32.689+0000: 15442: debug : virCommandRun:2473 : Result status 0, stdout: '' stderr: '' 2021-06-29 12:52:32.689+0000: 15442: debug : virCommandRunAsync:2627 : About to run /usr/bin/ovs-vsctl --timeout=5 --no-heading --columns=_uuid find qos 'external-ids:vm-id="4c42854e-3c84-43ed-ae87-9af0df0ecd16"' 2021-06-29 12:52:32.693+0000: 15442: debug : virCommandRunAsync:2629 : Command result 0, with PID 29125 2021-06-29 12:52:32.717+0000: 15442: debug : virCommandRun:2473 : Result status 0, stdout: 'f8b63e05-018d-44bb-9e74-a1ee4a40de79 ' stderr: '' 2021-06-29 12:52:32.717+0000: 15442: debug : virCommandRunAsync:2627 : About to run /usr/bin/ovs-vsctl --timeout=5 --no-heading --columns=_uuid find queue 'external-ids:vm-id="4c42854e-3c84-43ed-ae87-9af0df0ecd16"' 2021-06-29 12:52:32.721+0000: 15442: debug : virCommandRunAsync:2629 : Command result 0, with PID 29126 2021-06-29 12:52:32.745+0000: 15442: debug : virCommandRun:2473 : Result status 0, stdout: 'f098939d-c404-49aa-a9b0-b72631b662a2 ' stderr: '' 2021-06-29 12:52:32.745+0000: 15442: debug : virCommandRunAsync:2627 : About to run /usr/bin/ovs-vsctl --timeout=5 remove port vnet2 qos f8b63e05-018d-44bb-9e74-a1ee4a40de79 2021-06-29 12:52:32.749+0000: 15442: debug : virCommandRunAsync:2629 : Command result 0, with PID 29127 2021-06-29 12:52:32.772+0000: 15442: error : virCommandWait:2745 : internal error: Child process (/usr/bin/ovs-vsctl --timeout=5 remove port vnet2 qos f8b63e05-018d-44bb-9e74-a1ee4a40de79) unexpected exit status 1: ovs-vsctl: no row "vnet2" in table Port 2021-06-29 12:52:32.772+0000: 15442: debug : virCommandRun:2473 : Result status 0, stdout: '' stderr: 'ovs-vsctl: no row "vnet2" in table Port ' 2021-06-29 12:52:32.772+0000: 15442: error : virNetDevOpenvswitchInterfaceClearQos:851 : internal error: Unable to remove port qos on port vnet2

+ } + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "destroy", "qos", line, NULL); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to destroy qos on port %s"), ifname); + } + } + } + // destroy queue + if (queue_uuid && *queue_uuid) { + lines = g_strsplit(queue_uuid, "\n", 0); + for (i = 0; lines[i] != NULL; i++) { + const char *line = lines[i]; + if (!*line) { + continue; + } + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "destroy", "queue", line, NULL); + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to destroy queue on port %s"), ifname); + } + } + } + + virCommandFree(cmd); + VIR_FREE(qos_uuid); + VIR_FREE(queue_uuid); + VIR_FREE(vmid_ex_id); + VIR_FREE(lines); + return 0; +} \ No newline at end of file diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 7525376855..2dcd1aec6b 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -21,6 +21,7 @@ #pragma once

#include "internal.h" +#include "virnetdevbandwidth.h" #include "virnetdevvportprofile.h" #include "virnetdevvlan.h"

@@ -69,3 +70,13 @@ int virNetDevOpenvswitchGetVhostuserIfname(const char *path, int virNetDevOpenvswitchUpdateVlan(const char *ifname, const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; + +int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, + const virNetDevBandwidth *bandwidth, + const unsigned char *vmid, + bool swapped) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; + +int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, + const unsigned char *vmid) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;

Michal

On 6/28/21 11:18 AM, zhangjl02 wrote:

From: zhangjl02 <zhangjl02@inspur.com>

Return 0 directly if the port is ovs managed. When the ovs port is set noqueue, qos config on this port will not work. --- src/qemu/qemu_domain.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index da5a226fc2..2e8236ce7c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11526,6 +11526,8 @@ qemuDomainInterfaceSetDefaultQDisc(virQEMUDriver *driver, actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { + if (qemuDomainDefIsOvsport(net, actualType)) + return 0; if (virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) return -1; }

How about: if (!qemuDomainDefIsOvsport() && virNetDevBandwidthSetRootQDisc() < 0) return -1; Michal

On 6/28/21 11:18 AM, zhangjl02 wrote:

From: zhangjl02 <zhangjl02@inspur.com>

When qos is set or delete, we have to check if the port is an ovs managed port. If true, call the virNetDevOpenvswitchInterfaceSetQos function when qos is set, and call the virNetDevOpenvswitchInterfaceClearQos function when the interface is to be destroyed. --- src/qemu/qemu_command.c | 10 ++++++++-- src/qemu/qemu_driver.c | 21 ++++++++++++++++++++- src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++++++++----------- src/qemu/qemu_process.c | 11 +++++++++-- 4 files changed, 65 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea513693f7..35085f293c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8610,8 +8610,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportsBandwidth(actualType)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, - !virDomainNetTypeSharesHostView(net)) < 0) + if (qemuDomainDefIsOvsport(net, actualType)) { + if (virNetDevOpenvswitchInterfaceSetQos(net->ifname, actualBandwidth, + def->uuid, + !virDomainNetTypeSharesHostView(net)) < 0) + goto cleanup; + } + else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 235f575901..eefb67b404 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10231,6 +10231,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, bool inboundSpecified = false, outboundSpecified = false; int actualType; bool qosSupported = true; + bool ovsType = false;

virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -10277,6 +10278,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (net) { actualType = virDomainNetGetActualType(net); qosSupported = virNetDevSupportsBandwidth(actualType); + ovsType = qemuDomainDefIsOvsport(net, actualType); }

if (qosSupported && persistentNet) { @@ -10366,7 +10368,24 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } }

- if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, + if (ovsType){ + if (virNetDevOpenvswitchInterfaceSetQos(net->ifname, newBandwidth, + vm->def->uuid, + !virDomainNetTypeSharesHostView(net)) < 0){ + virErrorPtr orig_err; + virErrorPreserveLast(&orig_err);

Please separate variable declaration block and code block with an empty line.

+ ignore_value(virNetDevOpenvswitchInterfaceSetQos(net->ifname, newBandwidth, + vm->def->uuid, + !virDomainNetTypeSharesHostView(net))); + if (net->bandwidth) { + ignore_value(virDomainNetBandwidthUpdate(net, + net->bandwidth)); + } + virErrorRestore(&orig_err); + goto endjob; + } + } + else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0) { virErrorPtr orig_err;

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d2a354d026..4cf74072bc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1409,9 +1409,15 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportsBandwidth(actualType)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, - !virDomainNetTypeSharesHostView(net)) < 0) - goto cleanup; + if (qemuDomainDefIsOvsport(net, actualType)) { + if (virNetDevOpenvswitchInterfaceSetQos(net->ifname, actualBandwidth, + vm->def->uuid, + !virDomainNetTypeSharesHostView(net)) < 0) + goto cleanup; + } + else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) + goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " "type '%s' is not implemented yet", @@ -3914,9 +3920,15 @@ qemuDomainChangeNet(virQEMUDriver *driver, const virNetDevBandwidth *newb = virDomainNetGetActualBandwidth(newdev);

if (newb) { - if (virNetDevBandwidthSet(newdev->ifname, newb, false, - !virDomainNetTypeSharesHostView(newdev)) < 0) - goto cleanup; + if (qemuDomainDefIsOvsport(newdev, newType)) { + if (virNetDevOpenvswitchInterfaceSetQos(newdev->ifname, newb, + vm->def->uuid, + !virDomainNetTypeSharesHostView(newdev)) < 0) + goto cleanup; + } + else if (virNetDevBandwidthSet(newdev->ifname, newb, false, + !virDomainNetTypeSharesHostView(newdev)) < 0) + goto cleanup; } else { /* * virNetDevBandwidthSet() doesn't clear any existing @@ -4665,11 +4677,16 @@ qemuDomainRemoveNetDevice(virQEMUDriver *driver, if (!(charDevAlias = qemuAliasChardevFromDevAlias(net->info.alias))) return -1;

- if (virDomainNetGetActualBandwidth(net) && - virNetDevSupportsBandwidth(virDomainNetGetActualType(net)) && - virNetDevBandwidthClear(net->ifname) < 0) - VIR_WARN("cannot clear bandwidth setting for device : %s", - net->ifname); + if (virNetDevSupportsBandwidth(virDomainNetGetActualType(net))){ + if (qemuDomainDefIsOvsport(net, actualType)) { + if (virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0) + VIR_WARN("cannot clear bandwidth setting for ovs device : %s", + net->ifname); + } + else if (virNetDevBandwidthClear(net->ifname) < 0) + VIR_WARN("cannot clear bandwidth setting for device : %s", + net->ifname); + }

/* deactivate the tap/macvtap device on the host, which could also * affect the parent device (e.g. macvtap passthrough mode sets diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 2b03b0ab98..3499f4548c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7818,6 +7818,7 @@ void qemuProcessStop(virQEMUDriver *driver, g_autofree char *timestamp = NULL; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); g_autoptr(virConnect) conn = NULL; + int actualType;

VIR_DEBUG("Shutting down vm=%p name=%s id=%d pid=%lld, " "reason=%s, asyncJob=%s, flags=0x%x", @@ -7966,8 +7967,8 @@ void qemuProcessStop(virQEMUDriver *driver, for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i]; vport = virDomainNetGetActualVirtPortProfile(net); - - switch (virDomainNetGetActualType(net)) { + actualType = virDomainNetGetActualType(net); + switch (actualType) { case VIR_DOMAIN_NET_TYPE_DIRECT: ignore_value(virNetDevMacVLanDeleteWithVPortProfile( net->ifname, &net->mac, @@ -8023,6 +8024,12 @@ void qemuProcessStop(virQEMUDriver *driver, else VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); } + if ((actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) && vport && + vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) { + if (virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0)

This function can never return anything other than 0.

+ VIR_WARN("cannot clear bandwidth setting for ovs device : %s", + net->ifname); + } }

retry:

Michal