10:46 a.m.
On 7/1/21 10:42 AM, zhangjl02 wrote:
From: zhangjl02 <zhangjl02(a)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