From: Jinsheng Zhang <zhangjl02(a)inspur.com&gt; Instead of cleaning all qos rules each time new qos is set, tx and rx's qos can be set or clean respectively. Replace virReportError with VIR_WARN to let the cleaning process continue when error occurs. Add ifname into ovs querying statements, which will reduce failure of removing qos on the other interfaces of the same vm. Additionally, two problems is fixed. 1. Ingress rules is not clean on previous version of virNetDevOpenvswitchInterfaceClearQos. 2. If errors occurs when removing inbound qos on multi interfaces vm, some rules may not be delete as aspected. Signed-off-by: zhangjl02 <zhangjl02(a)inspur.com&gt; --- src/libvirt_private.syms | 2 + src/util/virnetdevopenvswitch.c | 69 +++++++++++++++++++++++++++------ src/util/virnetdevopenvswitch.h | 7 ++++ 3 files changed, 67 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6fc8239d2e..7366919d6c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2816,6 +2816,8 @@ virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchGetVhostuserIfname; virNetDevOpenvswitchInterfaceClearQos; +virNetDevOpenvswitchInterfaceClearRxQos; +virNetDevOpenvswitchInterfaceClearTxQos; virNetDevOpenvswitchInterfaceGetMaster; virNetDevOpenvswitchInterfaceParseStats; virNetDevOpenvswitchInterfaceSetQos; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 7a64a8dbe6..af9c611905 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -778,6 +778,10 @@ virNetDevOpenvswitchInterfaceSetQos(const char *ifname, return -1; } } + } else { + if (virNetDevOpenvswitchInterfaceClearTxQos(ifname, vmid) < 0) { + VIR_WARN("Clean tx qos for interface %s failed", ifname); + } } if (rx) { @@ -794,17 +798,23 @@ virNetDevOpenvswitchInterfaceSetQos(const char *ifname, _("Unable to set vlan configuration on port %s"), ifname); return -1; } + } else { + if (virNetDevOpenvswitchInterfaceClearRxQos(ifname) < 0) { + VIR_WARN("Clean rx qos for interface %s failed", ifname); + } } return 0; } int -virNetDevOpenvswitchInterfaceClearQos(const char *ifname, - const unsigned char *vmid) +virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, + const unsigned char *vmid) { + int ret = 0; char vmuuidstr[VIR_UUID_STRING_BUFLEN]; g_autoptr(virCommand) cmd = NULL; + g_autofree char *ifname_ex_id = NULL; g_autofree char *vmid_ex_id = NULL; g_autofree char *qos_uuid = NULL; g_autofree char *queue_uuid = NULL; @@ -815,7 +825,8 @@ virNetDevOpenvswitchInterfaceClearQos(const char *ifname, 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); + ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); + 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); @@ -825,7 +836,7 @@ virNetDevOpenvswitchInterfaceClearQos(const char *ifname, 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); + 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); @@ -843,7 +854,7 @@ virNetDevOpenvswitchInterfaceClearQos(const char *ifname, virCommandFree(cmd); cmd = virNetDevOpenvswitchCreateCmd(); virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "--if-exists", - "list", "port", ifname, "qos", NULL); + "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); @@ -860,9 +871,8 @@ virNetDevOpenvswitchInterfaceClearQos(const char *ifname, 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; + VIR_WARN("Unable to destroy qos on port %s", ifname); + ret = -1; } } } @@ -879,12 +889,49 @@ virNetDevOpenvswitchInterfaceClearQos(const char *ifname, 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; + VIR_WARN("Unable to destroy queue on port %s", ifname); + ret = -1; } } } + return ret; +} + +int +virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) +{ + g_autoptr(virCommand) cmd = NULL; + + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); + virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", 0llu); + virCommandAddArgFormat(cmd, "ingress_policing_burst=%llu", 0llu); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set vlan configuration on port %s"), ifname); + return -1; + } + return 0; } + +int +virNetDevOpenvswitchInterfaceClearQos(const char *ifname, + const unsigned char *vmid) +{ + int ret = 0; + + if (virNetDevOpenvswitchInterfaceClearTxQos(ifname, vmid) < 0) { + VIR_WARN("Clean tx qos for interface %s failed", ifname); + ret = -1; + } + + if (virNetDevOpenvswitchInterfaceClearRxQos(ifname) < 0) { + VIR_WARN("Clean rx qos for interface %s failed", ifname); + ret = -1; + } + + return ret; +} diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index 2dcd1aec6b..d44072cc8c 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -80,3 +80,10 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, const unsigned char *vmid) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + +int virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; + +int virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, + const unsigned char *vmid) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; -- 2.30.2.windows.1

From: Jinsheng Zhang <zhangjl02(a)inspur.com&gt; Test virNetDevOpenvswitchInterfaceSetQos and virNetDevOpenvswitchInterfaceClearQos with dryrun method. Signed-off-by: zhangjl02 <zhangjl02(a)inspur.com&gt; --- tests/virnetdevopenvswitchtest.c | 196 ++++++++++++++++++++++++++++++- 1 file changed, 195 insertions(+), 1 deletion(-) diff --git a/tests/virnetdevopenvswitchtest.c b/tests/virnetdevopenvswitchtest.c index 46172dae90..d1bcca637a 100644 --- a/tests/virnetdevopenvswitchtest.c +++ b/tests/virnetdevopenvswitchtest.c @@ -19,7 +19,11 @@ #include <config.h> #include "testutils.h" +#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW +#include "vircommandpriv.h" +#include "virnetdevbandwidth.h" #include "virnetdevopenvswitch.h" +#include "netdev_bandwidth_conf.c" #define VIR_FROM_THIS VIR_FROM_NONE @@ -29,6 +33,43 @@ struct _InterfaceParseStatsData { const virDomainInterfaceStatsStruct stats; }; +struct testSetQosStruct { + const char *band; + const char *exp_cmd; + const char *iface; +}; + +struct testClearQosStruct { + const char *exp_cmd; + const char *iface; + const unsigned char *vmid; +}; + +#define PARSE(xml, var) \ + do { \ + int rc; \ + xmlDocPtr doc; \ + xmlXPathContextPtr ctxt = NULL; \ + \ + if (!xml) \ + break; \ + \ + if (!(doc = virXMLParseStringCtxt((xml), \ + "bandwidth definition", \ + &ctxt))) \ + goto cleanup; \ + \ + rc = virNetDevBandwidthParse(&(var), \ + NULL, \ + ctxt->node, \ + true); \ + xmlFreeDoc(doc); \ + xmlXPathFreeContext(ctxt); \ + if (rc < 0) \ + goto cleanup; \ + } while (0) + +static const unsigned char vm_id[VIR_UUID_BUFLEN] = "fakeuuid"; static int testInterfaceParseStats(const void *opaque) @@ -111,6 +152,80 @@ testNameEscape(const void *opaque) } +static int +testVirNetDevOpenvswitchInterfaceSetQos(const void *data) +{ + int ret = -1; + const struct testSetQosStruct *info = data; + const char *iface = info->iface; + g_autoptr(virNetDevBandwidth) band = NULL; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + char *actual_cmd = NULL; + g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); + + PARSE(info->band, band); + + if (!iface) + iface = "tap-fake"; + + virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL); + + if (virNetDevOpenvswitchInterfaceSetQos(iface, band, vm_id, true) < 0) + goto cleanup; + + if (!(actual_cmd = virBufferContentAndReset(&buf))) { + /* This is interesting, no command has been executed. + * Maybe that's expected, actually. */ + } + + if (STRNEQ_NULLABLE(info->exp_cmd, actual_cmd)) { + virTestDifference(stderr, + NULLSTR(info->exp_cmd), + NULLSTR(actual_cmd)); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(actual_cmd); + return ret; +} + + +static int +testVirNetDevOpenvswitchInterfaceClearQos(const void *data) +{ + int ret = -1; + const struct testClearQosStruct *info = data; + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + char *actual_cmd = NULL; + const char *iface = info->iface; + const unsigned char *vmid = info->vmid; + g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); + + virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL); + + if (virNetDevOpenvswitchInterfaceClearQos(iface, vmid) < 0) + goto cleanup; + + if (!(actual_cmd = virBufferContentAndReset(&buf))) { + /* This is interesting, no command has been executed. + * Maybe that's expected, actually. */ + } + + if (STRNEQ_NULLABLE(info->exp_cmd, actual_cmd)) { + virTestDifference(stderr, + NULLSTR(info->exp_cmd), + NULLSTR(actual_cmd)); + goto cleanup; + } + + ret = 0; + cleanup: + VIR_FREE(actual_cmd); + return ret; +} + static int mymain(void) { @@ -146,7 +261,86 @@ mymain(void) TEST_NAME_ESCAPE("\"vhost\"user1\"", NULL); TEST_NAME_ESCAPE("\"\\\\", NULL); +#define DO_TEST_SET(Band, Exp_cmd, ...) \ + do { \ + struct testSetQosStruct data = {.band = Band, \ + .exp_cmd = Exp_cmd, \ + __VA_ARGS__}; \ + if (virTestRun("virNetDevOpenvswitchInterfaceSetQos", \ + testVirNetDevOpenvswitchInterfaceSetQos, \ + &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_SET(("<bandwidth>" + " <inbound average='20000'/>" + "</bandwidth>"), + (OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 set port tap-fake qos=@qos1" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'" + " -- --id=@qos1 create qos type=linux-htb other_config:min-rate=163840000" + " queues:0=@queue0 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'" + " -- --id=@queue0 create queue other_config:min-rate=163840000 " + "'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 set Interface tap-fake ingress_policing_rate=0 ingress_policing_burst=0\n")); + + DO_TEST_SET(NULL, NULL); + + DO_TEST_SET("<bandwidth/>", NULL); + + DO_TEST_SET(("<bandwidth>" + " <inbound average='0' />" + "</bandwidth>"), + (OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 set Interface tap-fake ingress_policing_rate=0 ingress_policing_burst=0\n")); + + DO_TEST_SET(("<bandwidth>" + " <inbound average='0' />" + " <outbound average='5000' />" + "</bandwidth>"), + (OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"tap-fake\"'\n" + OVS_VSCTL " --timeout=5 set Interface tap-fake ingress_policing_rate=40000\n")); + +#define DO_TEST_CLEAR_QOS(Iface, Vmid, Exp_cmd, ...) \ + do { \ + struct testClearQosStruct data = {.iface = Iface, \ + .vmid = Vmid, \ + .exp_cmd = Exp_cmd, \ + __VA_ARGS__}; \ + if (virTestRun("virNetDevOpenvswitchInterfaceClearQos", \ + testVirNetDevOpenvswitchInterfaceClearQos, \ + &data) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST_CLEAR_QOS(("fake-iface"), vm_id, + (OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"fake-iface\"'\n" + OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue" + " 'external-ids:vm-id=\"66616b65-7575-6964-0000-000000000000\"'" + " 'external-ids:ifname=\"fake-iface\"'\n" + OVS_VSCTL " --timeout=5 set Interface fake-iface ingress_policing_rate=0 ingress_policing_burst=0\n")); + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -VIR_TEST_MAIN(mymain); +VIR_TEST_MAIN_PRELOAD(mymain, VIR_TEST_MOCK("virnetdevbandwidth")) -- 2.30.2.windows.1