[libvirt PATCH 0/6] util: openvswitch: cleanup
Rearrange some code and remove 'cmd' variable reuse to get rid of manual 'virCommandFree' in functions using g_auto. Ján Tomko (6): util: openvswitch: move InterfaceClear{Rx,Tx}Qos util: openvswitch: unexport InterfaceClear{Rx,Tx}Qos util: openvswitch: split out virNetDevOpenvswitchInterfaceSetTxQos util: openvswitch: split out virNetDevOpenvswitchInterfaceSetRxQos util: openvswitch: do not reuse cmd in InterfaceSetTxQos util: openvswitch: do not reuse cmd in InterfaceClearTxQos src/libvirt_private.syms | 2 - src/util/virnetdevopenvswitch.c | 406 +++++++++++++++++--------------- src/util/virnetdevopenvswitch.h | 7 - 3 files changed, 213 insertions(+), 202 deletions(-) -- 2.31.1
These functions are called by virNetDevOpenvswitchInterfaceSetQos as well as virNetDevOpenvswitchInterfaceClearQos. Move them above both fuctions. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnetdevopenvswitch.c | 196 ++++++++++++++++---------------- 1 file changed, 98 insertions(+), 98 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index ba886dfb7d..5db4a397f4 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -637,6 +637,104 @@ virNetDevOpenvswitchFindUUID(const char *table, return uuid; } +int +virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, + const unsigned char *vmuuid) +{ + 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; + g_autofree char *port_qos = NULL; + size_t i; + + /* find qos */ + virUUIDFormat(vmuuid, vmuuidstr); + vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); + /* find queue */ + queue_uuid = virNetDevOpenvswitchFindUUID("queue", vmid_ex_id, ifname_ex_id); + /* find qos */ + qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id); + + if (qos_uuid && *qos_uuid) { + g_auto(GStrv) 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) { + VIR_WARN("Unable to destroy qos on port %s", ifname); + ret = -1; + } + } + } + /* destroy queue */ + if (queue_uuid && *queue_uuid) { + g_auto(GStrv) 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) { + 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 reset ingress on port %s"), ifname); + return -1; + } + + return 0; +} + /* * In virNetDevBandwidthRate, average, peak, floor are in kilobyes and burst in * kibibytes. However other_config in ovs qos is in bits and @@ -822,104 +920,6 @@ virNetDevOpenvswitchInterfaceSetQos(const char *ifname, return 0; } -int -virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, - const unsigned char *vmuuid) -{ - 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; - g_autofree char *port_qos = NULL; - size_t i; - - /* find qos */ - virUUIDFormat(vmuuid, vmuuidstr); - vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); - ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); - /* find queue */ - queue_uuid = virNetDevOpenvswitchFindUUID("queue", vmid_ex_id, ifname_ex_id); - /* find qos */ - qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id); - - if (qos_uuid && *qos_uuid) { - g_auto(GStrv) 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) { - VIR_WARN("Unable to destroy qos on port %s", ifname); - ret = -1; - } - } - } - /* destroy queue */ - if (queue_uuid && *queue_uuid) { - g_auto(GStrv) 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) { - 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 reset ingress on port %s"), ifname); - return -1; - } - - return 0; -} - int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, const unsigned char *vmuuid) -- 2.31.1
This also removes the indentation error. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/libvirt_private.syms | 2 -- src/util/virnetdevopenvswitch.c | 4 ++-- src/util/virnetdevopenvswitch.h | 7 ------- 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f75dea36c4..3f3357422e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2825,8 +2825,6 @@ virNetDevOpenvswitchAddPort; virNetDevOpenvswitchGetMigrateData; virNetDevOpenvswitchGetVhostuserIfname; virNetDevOpenvswitchInterfaceClearQos; -virNetDevOpenvswitchInterfaceClearRxQos; -virNetDevOpenvswitchInterfaceClearTxQos; virNetDevOpenvswitchInterfaceGetMaster; virNetDevOpenvswitchInterfaceParseStats; virNetDevOpenvswitchInterfaceSetQos; diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 5db4a397f4..b2cc2f36be 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -637,7 +637,7 @@ virNetDevOpenvswitchFindUUID(const char *table, return uuid; } -int +static int virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, const unsigned char *vmuuid) { @@ -716,7 +716,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, return ret; } -int +static int virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) { g_autoptr(virCommand) cmd = NULL; diff --git a/src/util/virnetdevopenvswitch.h b/src/util/virnetdevopenvswitch.h index e275af59df..b16c8fe318 100644 --- a/src/util/virnetdevopenvswitch.h +++ b/src/util/virnetdevopenvswitch.h @@ -80,10 +80,3 @@ int virNetDevOpenvswitchInterfaceSetQos(const char *ifname, int virNetDevOpenvswitchInterfaceClearQos(const char *ifname, const unsigned char *vmuuid) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; - -int virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) -ATTRIBUTE_NONNULL(1) 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.31.1
The virNetDevOpenvswitchInterfaceSetQos function is uneven because setting the Tx Qos is open-coded, while clearing it is sepearated in another function. Separate the setting too. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnetdevopenvswitch.c | 173 +++++++++++++++++--------------- 1 file changed, 92 insertions(+), 81 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index b2cc2f36be..71bd961b5a 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -745,6 +745,97 @@ virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname) #define KIBIBYTE_TO_BITS(x) (x * 8192ULL) #define VIR_NETDEV_RX_TO_OVS 8 +static int +virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, + const virNetDevBandwidthRate *tx, + const unsigned char *vmuuid) +{ + 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", KBYTE_TO_BITS(tx->average)); + if (tx->burst) + burst = g_strdup_printf("%llu", KIBIBYTE_TO_BITS(tx->burst)); + if (tx->peak) + peak = g_strdup_printf("%llu", KBYTE_TO_BITS(tx->peak)); + + virUUIDFormat(vmuuid, vmuuidstr); + vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); + ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); + /* find queue */ + queue_uuid = virNetDevOpenvswitchFindUUID("queue", vmid_ex_id, ifname_ex_id); + /* find qos */ + qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id); + + /* create qos and set */ + cmd = virNetDevOpenvswitchCreateCmd(); + if (queue_uuid && *queue_uuid) { + g_auto(GStrv) 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 && *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); + } + return -1; + } + + if (qos_uuid && *qos_uuid) { + g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); + + virCommandFree(cmd); + cmd = virNetDevOpenvswitchCreateCmd(); + 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; + } + } + + return 0; +} + /** * virNetDevOpenvswitchInterfaceSetQos: * @ifname: on which interface @@ -807,88 +898,8 @@ virNetDevOpenvswitchInterfaceSetQos(const char *ifname, } 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", KBYTE_TO_BITS(tx->average)); - if (tx->burst) - burst = g_strdup_printf("%llu", KIBIBYTE_TO_BITS(tx->burst)); - if (tx->peak) - peak = g_strdup_printf("%llu", KBYTE_TO_BITS(tx->peak)); - - virUUIDFormat(vmuuid, vmuuidstr); - vmid_ex_id = g_strdup_printf("external-ids:vm-id=\"%s\"", vmuuidstr); - ifname_ex_id = g_strdup_printf("external-ids:ifname=\"%s\"", ifname); - /* find queue */ - queue_uuid = virNetDevOpenvswitchFindUUID("queue", vmid_ex_id, ifname_ex_id); - /* find qos */ - qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id); - - /* create qos and set */ - cmd = virNetDevOpenvswitchCreateCmd(); - if (queue_uuid && *queue_uuid) { - g_auto(GStrv) 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 && *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 (virNetDevOpenvswitchInterfaceSetTxQos(ifname, tx, vmuuid) < 0) return -1; - } - - if (qos_uuid && *qos_uuid) { - g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); - - virCommandFree(cmd); - cmd = virNetDevOpenvswitchCreateCmd(); - 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; - } - } } else { if (virNetDevOpenvswitchInterfaceClearTxQos(ifname, vmuuid) < 0) { VIR_WARN("Clean tx qos for interface %s failed", ifname); -- 2.31.1
The virNetDevOpenvswitchInterfaceSetQos function is uneven because setting the Rx Qos is open-coded, while clearing it is sepearated in another function. Separate the setting too. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnetdevopenvswitch.c | 38 +++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 71bd961b5a..32f423ef04 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -836,6 +836,29 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, return 0; } +static int +virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname, + const virNetDevBandwidthRate *rx) +{ + g_autoptr(virCommand) cmd = NULL; + + cmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); + virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", + rx->average * VIR_NETDEV_RX_TO_OVS); + if (rx->burst) + virCommandAddArgFormat(cmd, "ingress_policing_burst=%llu", + rx->burst * VIR_NETDEV_RX_TO_OVS); + + if (virCommandRun(cmd, NULL) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to set vlan configuration on port %s"), ifname); + return -1; + } + + return 0; +} + /** * virNetDevOpenvswitchInterfaceSetQos: * @ifname: on which interface @@ -907,21 +930,8 @@ virNetDevOpenvswitchInterfaceSetQos(const char *ifname, } if (rx) { - g_autoptr(virCommand) cmd = NULL; - - cmd = virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "set", "Interface", ifname, NULL); - virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu", - rx->average * VIR_NETDEV_RX_TO_OVS); - if (rx->burst) - virCommandAddArgFormat(cmd, "ingress_policing_burst=%llu", - rx->burst * VIR_NETDEV_RX_TO_OVS); - - if (virCommandRun(cmd, NULL) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Unable to set vlan configuration on port %s"), ifname); + if (virNetDevOpenvswitchInterfaceSetRxQos(ifname, rx) < 0) return -1; - } } else { if (virNetDevOpenvswitchInterfaceClearRxQos(ifname) < 0) { VIR_WARN("Clean rx qos for interface %s failed", ifname); -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnetdevopenvswitch.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index 32f423ef04..a5180e5843 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -814,19 +814,18 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname, if (qos_uuid && *qos_uuid) { g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0); + g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd(); - virCommandFree(cmd); - cmd = virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "set", "qos", lines[0], NULL); - virCommandAddArgFormat(cmd, "other_config:min-rate=%s", average); + virCommandAddArgList(qoscmd, "set", "qos", lines[0], NULL); + virCommandAddArgFormat(qoscmd, "other_config:min-rate=%s", average); if (burst) { - virCommandAddArgFormat(cmd, "other_config:burst=%s", burst); + virCommandAddArgFormat(qoscmd, "other_config:burst=%s", burst); } if (peak) { - virCommandAddArgFormat(cmd, "other_config:max-rate=%s", peak); + virCommandAddArgFormat(qoscmd, "other_config:max-rate=%s", peak); } - virCommandAddArgList(cmd, vmid_ex_id, ifname_ex_id, NULL); - if (virCommandRun(cmd, NULL) < 0) { + virCommandAddArgList(qoscmd, vmid_ex_id, ifname_ex_id, NULL); + if (virCommandRun(qoscmd, NULL) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to set qos configuration on port %s"), ifname); return -1; -- 2.31.1
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/util/virnetdevopenvswitch.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c index a5180e5843..bcdb7c4180 100644 --- a/src/util/virnetdevopenvswitch.c +++ b/src/util/virnetdevopenvswitch.c @@ -643,7 +643,6 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, { 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; @@ -666,29 +665,29 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, /* destroy qos */ for (i = 0; lines[i] != NULL; i++) { const char *line = lines[i]; + g_autoptr(virCommand) listcmd = NULL; + g_autoptr(virCommand) destroycmd = NULL; + if (!*line) { continue; } - virCommandFree(cmd); - cmd = virNetDevOpenvswitchCreateCmd(); - virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "--if-exists", + listcmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(listcmd, "--no-heading", "--columns=_uuid", "--if-exists", "list", "port", ifname, "qos", NULL); - virCommandSetOutputBuffer(cmd, &port_qos); - if (virCommandRun(cmd, NULL) < 0) { + virCommandSetOutputBuffer(listcmd, &port_qos); + if (virCommandRun(listcmd, NULL) < 0) { VIR_WARN("Unable to remove port qos on port %s", ifname); } if (port_qos && *port_qos) { - virCommandFree(cmd); - cmd = virNetDevOpenvswitchCreateCmd(); + g_autoptr(virCommand) 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) { + destroycmd = virNetDevOpenvswitchCreateCmd(); + virCommandAddArgList(destroycmd, "destroy", "qos", line, NULL); + if (virCommandRun(destroycmd, NULL) < 0) { VIR_WARN("Unable to destroy qos on port %s", ifname); ret = -1; } @@ -699,11 +698,12 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname, g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0); for (i = 0; lines[i] != NULL; i++) { + g_autoptr(virCommand) cmd = NULL; const char *line = lines[i]; if (!*line) { continue; } - virCommandFree(cmd); + cmd = virNetDevOpenvswitchCreateCmd(); virCommandAddArgList(cmd, "destroy", "queue", line, NULL); if (virCommandRun(cmd, NULL) < 0) { -- 2.31.1
On 1/17/22 17:40, Ján Tomko wrote:
Rearrange some code and remove 'cmd' variable reuse to get rid of manual 'virCommandFree' in functions using g_auto.
Ján Tomko (6): util: openvswitch: move InterfaceClear{Rx,Tx}Qos util: openvswitch: unexport InterfaceClear{Rx,Tx}Qos util: openvswitch: split out virNetDevOpenvswitchInterfaceSetTxQos util: openvswitch: split out virNetDevOpenvswitchInterfaceSetRxQos util: openvswitch: do not reuse cmd in InterfaceSetTxQos util: openvswitch: do not reuse cmd in InterfaceClearTxQos
src/libvirt_private.syms | 2 - src/util/virnetdevopenvswitch.c | 406 +++++++++++++++++--------------- src/util/virnetdevopenvswitch.h | 7 - 3 files changed, 213 insertions(+), 202 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko -
Michal Prívozník