[PATCHv2 0/4] Add qemu support setting qos via ovs on ovs interface
--- diff to v1: - move qemuDomainDefIsOvsport from src/qemu/qemu_domain.c to src/conf/domain_conf.c - call virCommandFree(cmd)free cmd before reusing it. - add g_autofree to variables. - reduce usage of virReportError(), and coupled it with return -1. - fix remove port qos error. - optimise code structure. Thanks to Michal Privoznik for helping reviewing these patches. Now libvirt use tc rules to manage interface's qos. But when an interface is created by ovs, there is no qos setting result in ovs database. Therefore, qos of ovs port should be set via ovs management command. We add a function to tell whether a port definition is an ovs managed virtual port. Change default qdisc rules, which return 0 directly if the port is ovs managed(When the ovs port is set noqueue, qos config on this port will not work). Add ovs management function of setting and cleaning qos. Then check if the port is an ovs managed port during its life cycle, and call the ovs management function to set or clean qos settings. zhangjl02 (4): virDomain: interface: add virDomainNetDefIsOvsport virDomain: interface: add virNetDevOpenvswitchInterfaceSetQos and virNetDevOpenvswitchInterfaceClearQos qemu: interface: remove setting noqueue for ovs port qemu: interface: check and use ovs command to set qos of ovs managed port src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 10 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 22 ++- src/qemu/qemu_hotplug.c | 39 +++-- src/qemu/qemu_process.c | 10 +- src/util/virnetdevopenvswitch.c | 271 ++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 11 ++ 10 files changed, 362 insertions(+), 17 deletions(-) -- 2.30.2.windows.1
From: zhangjl02 <zhangjl02@inspur.com> Tell whether a port definition is an ovs managed virtual port --- src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 11 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d78f846a52..7fe72fb3f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29126,6 +29126,14 @@ virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) } } +/* Check whether the port is an ovs managed port */ +bool +virDomainNetDefIsOvsport(virDomainNetDef *net, int actualType) { + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); + return (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && vport && + vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH; +} + const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface) { diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index f706c498ff..a024e4394f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3608,6 +3608,8 @@ int virDomainNetGetActualDirectMode(const virDomainNetDef *iface); virDomainHostdevDef *virDomainNetGetActualHostdev(virDomainNetDef *iface); const virNetDevVPortProfile * virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); +bool +virDomainNetDefIsOvsport(virDomainNetDef *net, int actualType); const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface); const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 68e4b6aab8..2bcbce7288 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -517,6 +517,7 @@ virDomainNetDefActualFromNetworkPort; virDomainNetDefActualToNetworkPort; virDomainNetDefFormat; virDomainNetDefFree; +virDomainNetDefIsOvsport; virDomainNetDefNew; virDomainNetDefToNetworkPort; virDomainNetDHCPInterfaces; -- 2.30.2.windows.1
On 7/1/21 10:42 AM, zhangjl02 wrote:
From: zhangjl02 <zhangjl02@inspur.com>
Tell whether a port definition is an ovs managed virtual port --- src/conf/domain_conf.c | 8 ++++++++ src/conf/domain_conf.h | 2 ++ src/libvirt_private.syms | 1 + 3 files changed, 11 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d78f846a52..7fe72fb3f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29126,6 +29126,14 @@ virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) } }
+/* Check whether the port is an ovs managed port */ +bool +virDomainNetDefIsOvsport(virDomainNetDef *net, int actualType) { + const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); + return (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && vport && + vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH; +} +
Couple of notes: 1) We like to separate variable declaration block and code with an empty line. 2) The opening curly brace should go onto a new line. I wonder why 'ninja check' did not catch this. Or did you not try? 3) I think that actualType argument is redundant. Why hot just query it inside the function, e.g. like this: virDomainNetType actualType = virDomainNetGetActualType(net); So this is what I think should be squashed in: diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 20c60659cd..5a27cd9d7d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29128,9 +29128,12 @@ virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface) /* Check whether the port is an ovs managed port */ bool -virDomainNetDefIsOvsport(virDomainNetDef *net, int actualType) { +virDomainNetDefIsOvsport(const virDomainNetDef *net) +{ const virNetDevVPortProfile *vport = virDomainNetGetActualVirtPortProfile(net); - return (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && vport && + virDomainNetType actualType = virDomainNetGetActualType(net); + + return (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 12ad98ec8c..2a36c5acf1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3611,7 +3611,7 @@ virDomainHostdevDef *virDomainNetGetActualHostdev(virDomainNetDef *iface); const virNetDevVPortProfile * virDomainNetGetActualVirtPortProfile(const virDomainNetDef *iface); bool -virDomainNetDefIsOvsport(virDomainNetDef *net, int actualType); +virDomainNetDefIsOvsport(const virDomainNetDef *net); const virNetDevBandwidth * virDomainNetGetActualBandwidth(const virDomainNetDef *iface); const virNetDevVlan *virDomainNetGetActualVlan(const virDomainNetDef *iface); Michal
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 | 271 ++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 11 ++ 3 files changed, 284 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2bcbce7288..e60bfa62c5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2806,8 +2806,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..83acc869cd 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,270 @@ 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) +{ + char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + virNetDevBandwidthRate *rx = NULL; /* From domain POV */ + virNetDevBandwidthRate *tx = NULL; /* From domain POV */ + g_autoptr(virCommand) cmd = NULL; + g_autofree char *vmid_ex_id = NULL; + g_autofree char *ifname_ex_id = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL; + g_autofree char *qos_uuid = NULL; + g_autofree char *queue_uuid = NULL; + g_autofree char **lines = NULL; + + if (!bandwidth) { + /* nothing to be enabled */ + return 0; + } + + 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) { + if(virNetDevOpenvswitchInterfaceClearQos(ifname, vmid) < 0){ + VIR_WARN("Clean qos for interface %s failed", ifname); + } + } + 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); + + /* 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) { + VIR_WARN("Unable to find queue on port %s", ifname); + } + + /* find qos */ + virCommandFree(cmd); + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "qos", + vmid_ex_id, ifname_ex_id,NULL); + virCommandSetOutputBuffer(cmd, &qos_uuid); + if (virCommandRun(cmd, NULL) < 0) { + VIR_WARN("Unable to find qos on port %s", ifname); + } + + /* create qos and set */ + virCommandFree(cmd); + 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); + return -1; + } + else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to create and set qos configuration on port %s"), ifname); + return -1; + } + } + + if(qos_uuid && *qos_uuid){ + virCommandFree(cmd); + 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); + return -1; + } + } + } + + if (rx) { + average = g_strdup_printf("%llu", rx->average * 8); + if (rx->burst) + burst = g_strdup_printf("%llu", rx->burst * 8); + virCommandFree(cmd); + 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); + return -1; + } + virCommandFree(cmd); + } + + return 0; +} + +int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, + const unsigned char *vmid){ + char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *vmid_ex_id = NULL; + g_autofree char *qos_uuid = NULL; + g_autofree char *queue_uuid = NULL; + g_autofree char *port_qos = NULL; + g_autofree 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) { + VIR_WARN("Unable to find qos on port %s", ifname); + } + + /* find queue */ + virCommandFree(cmd); + 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) { + VIR_WARN("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; + } + virCommandFree(cmd); + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "--if-exists", + "list", "port", ifname, "qos", NULL); + virCommandSetOutputBuffer(cmd, &port_qos); + if (virCommandRun(cmd, NULL) < 0) { + VIR_WARN("Unable to remove port qos on port %s", ifname); + } + if (port_qos && *port_qos) { + virCommandFree(cmd); + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "remove", "port", ifname, "qos", line, NULL); + if (virCommandRun(cmd, NULL) < 0) { + VIR_WARN("Unable to remove port qos on port %s", ifname); + } + } + virCommandFree(cmd); + 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); + return -1; + } + } + } + /* 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; + } + virCommandFree(cmd); + 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); + return -1; + } + } + } + + 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; -- 2.30.2.windows.1
On 7/1/21 10:42 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 | 271 ++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 11 ++ 3 files changed, 284 insertions(+)
Again, this breaks syntax-check. And it looks like you did not really incorporate all my review comments from v1. Let me see how much work it is to fix or whether I'll ask you to send v3. [After some time] I won't point out every little bit as in v1, because those points are still valid. I just think this should be squashed in: diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 83acc869cd..7a64a8dbe6 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -637,23 +637,14 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname, * * Return 0 on success, -1 otherwise. */ -int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, - const virNetDevBandwidth *bandwidth, - const unsigned char *vmid, - bool swapped) +int +virNetDevOpenvswitchInterfaceSetQos(const char *ifname, + const virNetDevBandwidth *bandwidth, + const unsigned char *vmid, + bool swapped) { - char vmuuidstr[VIR_UUID_STRING_BUFLEN]; virNetDevBandwidthRate *rx = NULL; /* From domain POV */ virNetDevBandwidthRate *tx = NULL; /* From domain POV */ - g_autoptr(virCommand) cmd = NULL; - g_autofree char *vmid_ex_id = NULL; - g_autofree char *ifname_ex_id = NULL; - g_autofree char *average = NULL; - g_autofree char *peak = NULL; - g_autofree char *burst = NULL; - g_autofree char *qos_uuid = NULL; - g_autofree char *queue_uuid = NULL; - g_autofree char **lines = NULL; if (!bandwidth) { /* nothing to be enabled */ @@ -681,12 +672,25 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, rx = bandwidth->in; tx = bandwidth->out; } - if(!bandwidth->out && !bandwidth->in) { - if(virNetDevOpenvswitchInterfaceClearQos(ifname, vmid) < 0){ + + if (!bandwidth->out && !bandwidth->in) { + if (virNetDevOpenvswitchInterfaceClearQos(ifname, vmid) < 0) { VIR_WARN("Clean qos for interface %s failed", ifname); } + return 0; } + if (tx && tx->average) { + char vmuuidstr[VIR_UUID_STRING_BUFLEN]; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *vmid_ex_id = NULL; + g_autofree char *ifname_ex_id = NULL; + g_autofree char *average = NULL; + g_autofree char *peak = NULL; + g_autofree char *burst = NULL; + g_autofree char *qos_uuid = NULL; + g_autofree char *queue_uuid = NULL; + average = g_strdup_printf("%llu", tx->average * 8192); if (tx->burst) burst = g_strdup_printf("%llu", tx->burst * 8192); @@ -699,7 +703,7 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, 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); + vmid_ex_id, ifname_ex_id, NULL); virCommandSetOutputBuffer(cmd, &queue_uuid); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to find queue on port %s", ifname); @@ -709,7 +713,7 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, virCommandFree(cmd); cmd = virNetDevOpenvswitchCreateCmd(); virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", "qos", - vmid_ex_id, ifname_ex_id,NULL); + vmid_ex_id, ifname_ex_id, NULL); virCommandSetOutputBuffer(cmd, &qos_uuid); if (virCommandRun(cmd, NULL) < 0) { VIR_WARN("Unable to find qos on port %s", ifname); @@ -719,53 +723,52 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, virCommandFree(cmd); cmd = virNetDevOpenvswitchCreateCmd(); if (queue_uuid && *queue_uuid) { - lines = g_strsplit(queue_uuid, "\n", 0); + g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0); virCommandAddArgList(cmd, "set", "queue", lines[0], NULL); - }else{ + } 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){ + if (burst) { virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); } - if(peak){ + 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){ + if (burst) { virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); } - if(peak){ + 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){ + if (*queue_uuid) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to set queue configuration on port %s"), ifname); - return -1; - } - else { + } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create and set qos configuration on port %s"), ifname); - return -1; } + return -1; } - if(qos_uuid && *qos_uuid){ + if (qos_uuid && *qos_uuid) { + g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); + virCommandFree(cmd); 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){ + if (burst) { virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); } - if(peak){ + if (peak) { virCommandAddArgFormat(cmd, "other_config:max-rate=%s", peak); } virCommandAddArgList(cmd, vmid_ex_id, ifname_ex_id, NULL); @@ -778,36 +781,34 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, } if (rx) { - average = g_strdup_printf("%llu", rx->average * 8); - if (rx->burst) - burst = g_strdup_printf("%llu", rx->burst * 8); - virCommandFree(cmd); + g_autoptr(virCommand) cmd = NULL; + cmd = virNetDevOpenvswitchCreateCmd(); virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); - virCommandAddArgFormat(cmd, "ingress_policing_rate=%s", average); - if (burst) - virCommandAddArgFormat(cmd, "ingress_policing_burst=%s", burst); + virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", rx->average * 8); + if (rx->burst) + virCommandAddArgFormat(cmd, "ingress_policing_burst=%llu", rx->burst * 8); if (virCommandRun(cmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to set vlan configuration on port %s"), ifname); return -1; } - virCommandFree(cmd); } return 0; } -int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, - const unsigned char *vmid){ +int +virNetDevOpenvswitchInterfaceClearQos(const char *ifname, + const unsigned char *vmid) +{ char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd = NULL; g_autofree char *vmid_ex_id = NULL; g_autofree char *qos_uuid = NULL; g_autofree char *queue_uuid = NULL; g_autofree char *port_qos = NULL; - g_autofree char **lines = NULL; size_t i; /* find qos */ @@ -831,7 +832,8 @@ int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, } if (qos_uuid && *qos_uuid) { - lines = g_strsplit(qos_uuid, "\n", 0); + g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); + /* destroy qos */ for (i = 0; lines[i] != NULL; i++) { const char *line = lines[i]; @@ -866,7 +868,8 @@ int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, } /* destroy queue */ if (queue_uuid && *queue_uuid) { - lines = g_strsplit(queue_uuid, "\n", 0); + g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0); + for (i = 0; lines[i] != NULL; i++) { const char *line = lines[i]; if (!*line) { @@ -884,4 +887,4 @@ int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, } return 0; -} \ No newline at end of file +} Michal
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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..b9485e4fe6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11526,7 +11526,8 @@ qemuDomainInterfaceSetDefaultQDisc(virQEMUDriver *driver, actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) + if (!virDomainNetDefIsOvsport(net, actualType) && + virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) return -1; } -- 2.30.2.windows.1
On 7/1/21 10:42 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 | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fc60e15eea..b9485e4fe6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11526,7 +11526,8 @@ qemuDomainInterfaceSetDefaultQDisc(virQEMUDriver *driver, actualType == VIR_DOMAIN_NET_TYPE_NETWORK || actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) + if (!virDomainNetDefIsOvsport(net, actualType) &&
This "actualType" argument won't be needed after my suggestion from 1/4.
+ virNetDevBandwidthSetRootQDisc(net->ifname, "noqueue") < 0) return -1; }
Michal
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 | 22 +++++++++++++++++++++- src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++++++++----------- src/qemu/qemu_process.c | 10 ++++++++-- 4 files changed, 65 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea513693f7..1e9ed592e1 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 (virDomainNetDefIsOvsport(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..a6212fed6a 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 = virDomainNetDefIsOvsport(net, actualType); } if (qosSupported && persistentNet) { @@ -10366,7 +10368,25 @@ 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); + 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..e900834e3e 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 (virDomainNetDefIsOvsport(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 (virDomainNetDefIsOvsport(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 (virDomainNetDefIsOvsport(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..95cdf232ef 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,11 @@ void qemuProcessStop(virQEMUDriver *driver, else VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); } + if (virDomainNetDefIsOvsport(net, actualType)) { + if (virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0) + VIR_WARN("cannot clear bandwidth setting for ovs device : %s", + net->ifname); + } } retry: -- 2.30.2.windows.1
On 7/1/21 10:42 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 | 22 +++++++++++++++++++++- src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++++++++----------- src/qemu/qemu_process.c | 10 ++++++++-- 4 files changed, 65 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index ea513693f7..1e9ed592e1 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 (virDomainNetDefIsOvsport(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;
Again, we use slightly different style for if-else and curly braces. I'll just paste what I think should be squashed in: diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e9ed592e1..522394bb74 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8610,15 +8610,15 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportsBandwidth(actualType)) { - if (virDomainNetDefIsOvsport(net, actualType)) { + if (virDomainNetDefIsOvsport(net)) { if (virNetDevOpenvswitchInterfaceSetQos(net->ifname, actualBandwidth, def->uuid, !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; - } - else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, - !virDomainNetTypeSharesHostView(net)) < 0) + } 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", diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a6212fed6a..72f550bf8d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10278,7 +10278,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (net) { actualType = virDomainNetGetActualType(net); qosSupported = virNetDevSupportsBandwidth(actualType); - ovsType = virDomainNetDefIsOvsport(net, actualType); + ovsType = virDomainNetDefIsOvsport(net); } if (qosSupported && persistentNet) { @@ -10368,10 +10368,10 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } } - if (ovsType){ + if (ovsType) { if (virNetDevOpenvswitchInterfaceSetQos(net->ifname, newBandwidth, vm->def->uuid, - !virDomainNetTypeSharesHostView(net)) < 0){ + !virDomainNetTypeSharesHostView(net)) < 0) { virErrorPtr orig_err; virErrorPreserveLast(&orig_err); @@ -10385,9 +10385,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virErrorRestore(&orig_err); goto endjob; } - } - else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, - !virDomainNetTypeSharesHostView(net)) < 0) { + } else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) { virErrorPtr orig_err; virErrorPreserveLast(&orig_err); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e900834e3e..cb6a4e4ea5 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1409,15 +1409,15 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportsBandwidth(actualType)) { - if (virDomainNetDefIsOvsport(net, actualType)) { + if (virDomainNetDefIsOvsport(net)) { 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 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", @@ -3920,15 +3920,15 @@ qemuDomainChangeNet(virQEMUDriver *driver, const virNetDevBandwidth *newb = virDomainNetGetActualBandwidth(newdev); if (newb) { - if (virDomainNetDefIsOvsport(newdev, newType)) { + if (virDomainNetDefIsOvsport(newdev)) { 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 if (virNetDevBandwidthSet(newdev->ifname, newb, false, - !virDomainNetTypeSharesHostView(newdev)) < 0) - goto cleanup; } else { /* * virNetDevBandwidthSet() doesn't clear any existing @@ -4677,15 +4677,15 @@ qemuDomainRemoveNetDevice(virQEMUDriver *driver, if (!(charDevAlias = qemuAliasChardevFromDevAlias(net->info.alias))) return -1; - if (virNetDevSupportsBandwidth(virDomainNetGetActualType(net))){ - if (virDomainNetDefIsOvsport(net, actualType)) { + if (virNetDevSupportsBandwidth(virDomainNetGetActualType(net))) { + if (virDomainNetDefIsOvsport(net)) { 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) + } 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 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 95cdf232ef..3693796b06 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7818,7 +7818,6 @@ 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", @@ -7967,8 +7966,7 @@ void qemuProcessStop(virQEMUDriver *driver, for (i = 0; i < def->nnets; i++) { virDomainNetDef *net = def->nets[i]; vport = virDomainNetGetActualVirtPortProfile(net); - actualType = virDomainNetGetActualType(net); - switch (actualType) { + switch (virDomainNetGetActualType(net)) { case VIR_DOMAIN_NET_TYPE_DIRECT: ignore_value(virNetDevMacVLanDeleteWithVPortProfile( net->ifname, &net->mac, @@ -8024,10 +8022,11 @@ void qemuProcessStop(virQEMUDriver *driver, else VIR_WARN("Unable to release network device '%s'", NULLSTR(net->ifname)); } - if (virDomainNetDefIsOvsport(net, actualType)) { - if (virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0) - VIR_WARN("cannot clear bandwidth setting for ovs device : %s", - net->ifname); + + if (virDomainNetDefIsOvsport(net) && + virNetDevOpenvswitchInterfaceClearQos(net->ifname, vm->def->uuid) < 0) { + VIR_WARN("cannot clear bandwidth setting for ovs device : %s", + net->ifname); } } Michal
On 7/1/21 10:42 AM, zhangjl02 wrote:
---
diff to v1: - move qemuDomainDefIsOvsport from src/qemu/qemu_domain.c to src/conf/domain_conf.c
You can also post a follow up patch that replaces check from the function with the function call :-) I mean, we have couple of places that do look exactly as the function body.
- call virCommandFree(cmd)free cmd before reusing it. - add g_autofree to variables. - reduce usage of virReportError(), and coupled it with return -1. - fix remove port qos error. - optimise code structure.
Thanks to Michal Privoznik for helping reviewing these patches.
Now libvirt use tc rules to manage interface's qos. But when an interface is created by ovs, there is no qos setting result in ovs database. Therefore, qos of ovs port should be set via ovs management command. We add a function to tell whether a port definition is an ovs managed virtual port. Change default qdisc rules, which return 0 directly if the port is ovs managed(When the ovs port is set noqueue, qos config on this port will not work). Add ovs management function of setting and cleaning qos. Then check if the port is an ovs managed port during its life cycle, and call the ovs management function to set or clean qos settings.
zhangjl02 (4): virDomain: interface: add virDomainNetDefIsOvsport virDomain: interface: add virNetDevOpenvswitchInterfaceSetQos and virNetDevOpenvswitchInterfaceClearQos qemu: interface: remove setting noqueue for ovs port qemu: interface: check and use ovs command to set qos of ovs managed port
src/conf/domain_conf.c | 8 + src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 3 + src/qemu/qemu_command.c | 10 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 22 ++- src/qemu/qemu_hotplug.c | 39 +++-- src/qemu/qemu_process.c | 10 +- src/util/virnetdevopenvswitch.c | 271 ++++++++++++++++++++++++++++++++ src/util/virnetdevopenvswitch.h | 11 ++ 10 files changed, 362 insertions(+), 17 deletions(-)
I've suggested what should be squashed in for each patch. Please let me know if you agree and if so I'll squash it and merge. Michal
participants (2)
-
Michal Prívozník -
zhangjl02