[PATCH] virnetdevopenvswitch: Propagate OVS error messages

Michal Privoznik posted 1 patch 7 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/f5e1b830016a6bd06c1e1cfb1c356e93b7c2cd06.1694163878.git.mprivozn@redhat.com
src/util/virnetdevopenvswitch.c | 93 ++++++++++++++++++++-------------
1 file changed, 58 insertions(+), 35 deletions(-)
[PATCH] virnetdevopenvswitch: Propagate OVS error messages
Posted by Michal Privoznik 7 months, 3 weeks ago
When configuring OVS interfaces/bridges we spawn 'ovs-vsctl' with
appropriate arguments and if it exited with a non-zero status we
report a generic error message, like "Unable to add port vnet0 to
OVS bridge ovsbr0". This is all cool, but the real reason why
operation failed is hidden in (debug) logs because that's where
virCommandRun() reports it unless caller requested otherwise.

This is a bit clumsy because then we have to ask users to turn on
debug logs and reproduce the problem again, e.g. [1].

Therefore, in cases where an error is reported to the user - just
read ovs-vsctl's stderr and include it in the error message. For
other cases (like VIR_DEBUG/VIR_WARN) - well they are meant to
end up in (debug) logs anyway.

1: https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052640.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/virnetdevopenvswitch.c | 93 ++++++++++++++++++++-------------
 1 file changed, 58 insertions(+), 35 deletions(-)

diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 8dad6ed2bd..d836d05845 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -54,10 +54,14 @@ virNetDevOpenvswitchSetTimeout(unsigned int timeout)
 }
 
 static virCommand *
-virNetDevOpenvswitchCreateCmd(void)
+virNetDevOpenvswitchCreateCmd(char **errbuf)
 {
     virCommand *cmd = virCommandNew(OVS_VSCTL);
+
     virCommandAddArgFormat(cmd, "--timeout=%u", virNetDevOpenvswitchTimeout);
+    if (errbuf)
+        virCommandSetErrorBuffer(cmd, errbuf);
+
     return cmd;
 }
 
@@ -137,6 +141,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
     char ifuuidstr[VIR_UUID_STRING_BUFLEN];
     char vmuuidstr[VIR_UUID_STRING_BUFLEN];
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errbuf = NULL;
     g_autofree char *attachedmac_ex_id = NULL;
     g_autofree char *ifaceid_ex_id = NULL;
     g_autofree char *profile_ex_id = NULL;
@@ -157,7 +162,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
                                         ovsport->profileID);
     }
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     virCommandAddArgList(cmd, "--", "--may-exist",
                          "add-port", brname, ifname, NULL);
 
@@ -185,8 +190,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
 
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to add port %1$s to OVS bridge %2$s"),
-                       ifname, brname);
+                       _("Unable to add port %1$s to OVS bridge %2$s: %3$s"),
+                       ifname, brname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -203,13 +208,15 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
  */
 int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char *ifname)
 {
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autofree char *errbuf = NULL;
+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
 
     virCommandAddArgList(cmd, "--", "--if-exists", "del-port", ifname, NULL);
 
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to delete port %1$s from OVS"), ifname);
+                       _("Unable to delete port %1$s from OVS: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -228,7 +235,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char
 int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 {
     size_t len;
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autofree char *errbuf = NULL;
+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
 
     virCommandAddArgList(cmd, "--if-exists", "get", "Interface",
                          ifname, "external_ids:PortData", NULL);
@@ -238,8 +246,8 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
     /* Run the command */
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to run command to get OVS port data for interface %1$s"),
-                       ifname);
+                       _("Unable to run command to get OVS port data for interface %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -263,21 +271,22 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
 int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname)
 {
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errbuf = NULL;
 
     if (!migrate) {
         VIR_DEBUG("No OVS port data for interface %s", ifname);
         return 0;
     }
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     virCommandAddArgList(cmd, "set", "Interface", ifname, NULL);
     virCommandAddArgFormat(cmd, "external_ids:PortData=%s", migrate);
 
     /* Run the command */
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to run command to set OVS port data for interface %1$s"),
-                       ifname);
+                       _("Unable to run command to set OVS port data for interface %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -371,7 +380,8 @@ int
 virNetDevOpenvswitchInterfaceStats(const char *ifname,
                                    virDomainInterfaceStatsPtr stats)
 {
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autofree char *errbuf = NULL;
+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     g_autofree char *output = NULL;
 
     virCommandAddArgList(cmd, "--if-exists", "--format=list", "--data=json",
@@ -390,8 +400,9 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
     if (virCommandRun(cmd, NULL) < 0 ||
         STREQ_NULLABLE(output, "")) {
         /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Interface not found"));
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Interface not found: %1$s"),
+                       NULLSTR(errbuf));
         return -1;
     }
 
@@ -433,7 +444,8 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
 int
 virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
 {
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autofree char *errbuf = NULL;
+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     int exitstatus;
 
     *master = NULL;
@@ -443,8 +455,8 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
 
     if (virCommandRun(cmd, &exitstatus) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to run command to get OVS master for interface %1$s"),
-                       ifname);
+                       _("Unable to run command to get OVS master for interface %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -546,7 +558,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
         return 0;
     }
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(NULL);
 
     if (server) {
         virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
@@ -600,7 +612,8 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
 int virNetDevOpenvswitchUpdateVlan(const char *ifname,
                                    const virNetDevVlan *virtVlan)
 {
-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+    g_autofree char *errbuf = NULL;
+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
 
     virCommandAddArgList(cmd,
                          "--", "--if-exists", "clear", "Port", ifname, "tag",
@@ -614,7 +627,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,
 
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to set vlan configuration on port %1$s"), ifname);
+                       _("Unable to set vlan configuration on port %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -629,7 +643,7 @@ virNetDevOpenvswitchFindUUID(const char *table,
     g_autoptr(virCommand) cmd = NULL;
     char *uuid = NULL;
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(NULL);
     virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", table,
                          vmid_ex_id, ifname_ex_id, NULL);
     virCommandSetOutputBuffer(cmd, &uuid);
@@ -673,7 +687,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
             if (!*line) {
                 continue;
             }
-            listcmd = virNetDevOpenvswitchCreateCmd();
+            listcmd = virNetDevOpenvswitchCreateCmd(NULL);
             virCommandAddArgList(listcmd, "--no-heading", "--columns=_uuid", "--if-exists",
                                  "list", "port", ifname, "qos", NULL);
             virCommandSetOutputBuffer(listcmd, &port_qos);
@@ -681,13 +695,13 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
                 VIR_WARN("Unable to remove port qos on port %s", ifname);
             }
             if (port_qos && *port_qos) {
-                g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
+                g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(NULL);
                 virCommandAddArgList(cmd, "remove", "port", ifname, "qos", line, NULL);
                 if (virCommandRun(cmd, NULL) < 0) {
                     VIR_WARN("Unable to remove port qos on port %s", ifname);
                 }
             }
-            destroycmd = virNetDevOpenvswitchCreateCmd();
+            destroycmd = virNetDevOpenvswitchCreateCmd(NULL);
             virCommandAddArgList(destroycmd, "destroy", "qos", line, NULL);
             if (virCommandRun(destroycmd, NULL) < 0) {
                 VIR_WARN("Unable to destroy qos on port %s", ifname);
@@ -706,7 +720,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
                 continue;
             }
 
-            cmd = virNetDevOpenvswitchCreateCmd();
+            cmd = virNetDevOpenvswitchCreateCmd(NULL);
             virCommandAddArgList(cmd, "destroy", "queue", line, NULL);
             if (virCommandRun(cmd, NULL) < 0) {
                 VIR_WARN("Unable to destroy queue on port %s", ifname);
@@ -722,15 +736,17 @@ static int
 virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname)
 {
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errbuf = NULL;
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     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 %1$s"), ifname);
+                       _("Unable to reset ingress on port %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
@@ -754,6 +770,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
 {
     char vmuuidstr[VIR_UUID_STRING_BUFLEN];
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errbuf = NULL;
     g_autofree char *vmid_ex_id = NULL;
     g_autofree char *ifname_ex_id = NULL;
     g_autofree char *average = NULL;
@@ -777,7 +794,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
     qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id);
 
     /* create qos and set */
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     if (queue_uuid && *queue_uuid) {
         g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0);
         virCommandAddArgList(cmd, "set", "queue", lines[0], NULL);
@@ -806,17 +823,20 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
     if (virCommandRun(cmd, NULL) < 0) {
         if (queue_uuid && *queue_uuid) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to set queue configuration on port %1$s"), ifname);
+                           _("Unable to set queue configuration on port %1$s: %2$s"),
+                           ifname, NULLSTR(errbuf));
         } else {
             virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Unable to create and set qos configuration on port %1$s"), ifname);
+                           _("Unable to create and set qos configuration on port %1$s: %2$s"),
+                           ifname, NULLSTR(errbuf));
         }
         return -1;
     }
 
     if (qos_uuid && *qos_uuid) {
         g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0);
-        g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd();
+        g_autofree char *qoserrbuf = NULL;
+        g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd(&qoserrbuf);
 
         virCommandAddArgList(qoscmd, "set", "qos", lines[0], NULL);
         virCommandAddArgFormat(qoscmd, "other_config:min-rate=%s", average);
@@ -829,7 +849,8 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
         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 %1$s"), ifname);
+                           _("Unable to set qos configuration on port %1$s: %2$s"),
+                           ifname, NULLSTR(qoserrbuf));
             return -1;
         }
     }
@@ -842,8 +863,9 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname,
                                       const virNetDevBandwidthRate *rx)
 {
     g_autoptr(virCommand) cmd = NULL;
+    g_autofree char *errbuf = NULL;
 
-    cmd = virNetDevOpenvswitchCreateCmd();
+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
     virCommandAddArgList(cmd, "set", "Interface", ifname, NULL);
     virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu",
                            rx->average * VIR_NETDEV_RX_TO_OVS);
@@ -853,7 +875,8 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname,
 
     if (virCommandRun(cmd, NULL) < 0) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unable to set vlan configuration on port %1$s"), ifname);
+                       _("Unable to set vlan configuration on port %1$s: %2$s"),
+                       ifname, NULLSTR(errbuf));
         return -1;
     }
 
-- 
2.41.0
Re: [PATCH] virnetdevopenvswitch: Propagate OVS error messages
Posted by Martin Kletzander 7 months, 3 weeks ago
On Fri, Sep 08, 2023 at 11:04:38AM +0200, Michal Privoznik wrote:
>When configuring OVS interfaces/bridges we spawn 'ovs-vsctl' with
>appropriate arguments and if it exited with a non-zero status we
>report a generic error message, like "Unable to add port vnet0 to
>OVS bridge ovsbr0". This is all cool, but the real reason why
>operation failed is hidden in (debug) logs because that's where
>virCommandRun() reports it unless caller requested otherwise.
>
>This is a bit clumsy because then we have to ask users to turn on
>debug logs and reproduce the problem again, e.g. [1].
>
>Therefore, in cases where an error is reported to the user - just
>read ovs-vsctl's stderr and include it in the error message. For
>other cases (like VIR_DEBUG/VIR_WARN) - well they are meant to
>end up in (debug) logs anyway.
>
>1: https://mail.openvswitch.org/pipermail/ovs-discuss/2023-September/052640.html
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

>---
> src/util/virnetdevopenvswitch.c | 93 ++++++++++++++++++++-------------
> 1 file changed, 58 insertions(+), 35 deletions(-)
>
>diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
>index 8dad6ed2bd..d836d05845 100644
>--- a/src/util/virnetdevopenvswitch.c
>+++ b/src/util/virnetdevopenvswitch.c
>@@ -54,10 +54,14 @@ virNetDevOpenvswitchSetTimeout(unsigned int timeout)
> }
>
> static virCommand *
>-virNetDevOpenvswitchCreateCmd(void)
>+virNetDevOpenvswitchCreateCmd(char **errbuf)
> {
>     virCommand *cmd = virCommandNew(OVS_VSCTL);
>+
>     virCommandAddArgFormat(cmd, "--timeout=%u", virNetDevOpenvswitchTimeout);
>+    if (errbuf)
>+        virCommandSetErrorBuffer(cmd, errbuf);
>+
>     return cmd;
> }
>
>@@ -137,6 +141,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>     char ifuuidstr[VIR_UUID_STRING_BUFLEN];
>     char vmuuidstr[VIR_UUID_STRING_BUFLEN];
>     g_autoptr(virCommand) cmd = NULL;
>+    g_autofree char *errbuf = NULL;
>     g_autofree char *attachedmac_ex_id = NULL;
>     g_autofree char *ifaceid_ex_id = NULL;
>     g_autofree char *profile_ex_id = NULL;
>@@ -157,7 +162,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>                                         ovsport->profileID);
>     }
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     virCommandAddArgList(cmd, "--", "--may-exist",
>                          "add-port", brname, ifname, NULL);
>
>@@ -185,8 +190,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to add port %1$s to OVS bridge %2$s"),
>-                       ifname, brname);
>+                       _("Unable to add port %1$s to OVS bridge %2$s: %3$s"),
>+                       ifname, brname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -203,13 +208,15 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>  */
> int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char *ifname)
> {
>-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+    g_autofree char *errbuf = NULL;
>+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>
>     virCommandAddArgList(cmd, "--", "--if-exists", "del-port", ifname, NULL);
>
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to delete port %1$s from OVS"), ifname);
>+                       _("Unable to delete port %1$s from OVS: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -228,7 +235,8 @@ int virNetDevOpenvswitchRemovePort(const char *brname G_GNUC_UNUSED, const char
> int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
> {
>     size_t len;
>-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+    g_autofree char *errbuf = NULL;
>+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>
>     virCommandAddArgList(cmd, "--if-exists", "get", "Interface",
>                          ifname, "external_ids:PortData", NULL);
>@@ -238,8 +246,8 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
>     /* Run the command */
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to run command to get OVS port data for interface %1$s"),
>-                       ifname);
>+                       _("Unable to run command to get OVS port data for interface %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -263,21 +271,22 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char *ifname)
> int virNetDevOpenvswitchSetMigrateData(char *migrate, const char *ifname)
> {
>     g_autoptr(virCommand) cmd = NULL;
>+    g_autofree char *errbuf = NULL;
>
>     if (!migrate) {
>         VIR_DEBUG("No OVS port data for interface %s", ifname);
>         return 0;
>     }
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     virCommandAddArgList(cmd, "set", "Interface", ifname, NULL);
>     virCommandAddArgFormat(cmd, "external_ids:PortData=%s", migrate);
>
>     /* Run the command */
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to run command to set OVS port data for interface %1$s"),
>-                       ifname);
>+                       _("Unable to run command to set OVS port data for interface %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -371,7 +380,8 @@ int
> virNetDevOpenvswitchInterfaceStats(const char *ifname,
>                                    virDomainInterfaceStatsPtr stats)
> {
>-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+    g_autofree char *errbuf = NULL;
>+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     g_autofree char *output = NULL;
>
>     virCommandAddArgList(cmd, "--if-exists", "--format=list", "--data=json",
>@@ -390,8 +400,9 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
>     if (virCommandRun(cmd, NULL) < 0 ||
>         STREQ_NULLABLE(output, "")) {
>         /* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */
>-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>-                       _("Interface not found"));
>+        virReportError(VIR_ERR_INTERNAL_ERROR,
>+                       _("Interface not found: %1$s"),
>+                       NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -433,7 +444,8 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
> int
> virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
> {
>-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+    g_autofree char *errbuf = NULL;
>+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     int exitstatus;
>
>     *master = NULL;
>@@ -443,8 +455,8 @@ virNetDevOpenvswitchInterfaceGetMaster(const char *ifname, char **master)
>
>     if (virCommandRun(cmd, &exitstatus) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to run command to get OVS master for interface %1$s"),
>-                       ifname);
>+                       _("Unable to run command to get OVS master for interface %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -546,7 +558,7 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
>         return 0;
>     }
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(NULL);
>
>     if (server) {
>         virCommandAddArgList(cmd, "--no-headings", "--columns=name", "find",
>@@ -600,7 +612,8 @@ virNetDevOpenvswitchGetVhostuserIfname(const char *path,
> int virNetDevOpenvswitchUpdateVlan(const char *ifname,
>                                    const virNetDevVlan *virtVlan)
> {
>-    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+    g_autofree char *errbuf = NULL;
>+    g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>
>     virCommandAddArgList(cmd,
>                          "--", "--if-exists", "clear", "Port", ifname, "tag",
>@@ -614,7 +627,8 @@ int virNetDevOpenvswitchUpdateVlan(const char *ifname,
>
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to set vlan configuration on port %1$s"), ifname);
>+                       _("Unable to set vlan configuration on port %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -629,7 +643,7 @@ virNetDevOpenvswitchFindUUID(const char *table,
>     g_autoptr(virCommand) cmd = NULL;
>     char *uuid = NULL;
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(NULL);
>     virCommandAddArgList(cmd, "--no-heading", "--columns=_uuid", "find", table,
>                          vmid_ex_id, ifname_ex_id, NULL);
>     virCommandSetOutputBuffer(cmd, &uuid);
>@@ -673,7 +687,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
>             if (!*line) {
>                 continue;
>             }
>-            listcmd = virNetDevOpenvswitchCreateCmd();
>+            listcmd = virNetDevOpenvswitchCreateCmd(NULL);
>             virCommandAddArgList(listcmd, "--no-heading", "--columns=_uuid", "--if-exists",
>                                  "list", "port", ifname, "qos", NULL);
>             virCommandSetOutputBuffer(listcmd, &port_qos);
>@@ -681,13 +695,13 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
>                 VIR_WARN("Unable to remove port qos on port %s", ifname);
>             }
>             if (port_qos && *port_qos) {
>-                g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd();
>+                g_autoptr(virCommand) cmd = virNetDevOpenvswitchCreateCmd(NULL);
>                 virCommandAddArgList(cmd, "remove", "port", ifname, "qos", line, NULL);
>                 if (virCommandRun(cmd, NULL) < 0) {
>                     VIR_WARN("Unable to remove port qos on port %s", ifname);
>                 }
>             }
>-            destroycmd = virNetDevOpenvswitchCreateCmd();
>+            destroycmd = virNetDevOpenvswitchCreateCmd(NULL);
>             virCommandAddArgList(destroycmd, "destroy", "qos", line, NULL);
>             if (virCommandRun(destroycmd, NULL) < 0) {
>                 VIR_WARN("Unable to destroy qos on port %s", ifname);
>@@ -706,7 +720,7 @@ virNetDevOpenvswitchInterfaceClearTxQos(const char *ifname,
>                 continue;
>             }
>
>-            cmd = virNetDevOpenvswitchCreateCmd();
>+            cmd = virNetDevOpenvswitchCreateCmd(NULL);
>             virCommandAddArgList(cmd, "destroy", "queue", line, NULL);
>             if (virCommandRun(cmd, NULL) < 0) {
>                 VIR_WARN("Unable to destroy queue on port %s", ifname);
>@@ -722,15 +736,17 @@ static int
> virNetDevOpenvswitchInterfaceClearRxQos(const char *ifname)
> {
>     g_autoptr(virCommand) cmd = NULL;
>+    g_autofree char *errbuf = NULL;
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     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 %1$s"), ifname);
>+                       _("Unable to reset ingress on port %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>@@ -754,6 +770,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
> {
>     char vmuuidstr[VIR_UUID_STRING_BUFLEN];
>     g_autoptr(virCommand) cmd = NULL;
>+    g_autofree char *errbuf = NULL;
>     g_autofree char *vmid_ex_id = NULL;
>     g_autofree char *ifname_ex_id = NULL;
>     g_autofree char *average = NULL;
>@@ -777,7 +794,7 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
>     qos_uuid = virNetDevOpenvswitchFindUUID("qos", vmid_ex_id, ifname_ex_id);
>
>     /* create qos and set */
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     if (queue_uuid && *queue_uuid) {
>         g_auto(GStrv) lines = g_strsplit(queue_uuid, "\n", 0);
>         virCommandAddArgList(cmd, "set", "queue", lines[0], NULL);
>@@ -806,17 +823,20 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
>     if (virCommandRun(cmd, NULL) < 0) {
>         if (queue_uuid && *queue_uuid) {
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>-                           _("Unable to set queue configuration on port %1$s"), ifname);
>+                           _("Unable to set queue configuration on port %1$s: %2$s"),
>+                           ifname, NULLSTR(errbuf));
>         } else {
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>-                           _("Unable to create and set qos configuration on port %1$s"), ifname);
>+                           _("Unable to create and set qos configuration on port %1$s: %2$s"),
>+                           ifname, NULLSTR(errbuf));
>         }
>         return -1;
>     }
>
>     if (qos_uuid && *qos_uuid) {
>         g_auto(GStrv) lines = g_strsplit(qos_uuid, "\n", 0);
>-        g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd();
>+        g_autofree char *qoserrbuf = NULL;
>+        g_autoptr(virCommand) qoscmd = virNetDevOpenvswitchCreateCmd(&qoserrbuf);
>
>         virCommandAddArgList(qoscmd, "set", "qos", lines[0], NULL);
>         virCommandAddArgFormat(qoscmd, "other_config:min-rate=%s", average);
>@@ -829,7 +849,8 @@ virNetDevOpenvswitchInterfaceSetTxQos(const char *ifname,
>         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 %1$s"), ifname);
>+                           _("Unable to set qos configuration on port %1$s: %2$s"),
>+                           ifname, NULLSTR(qoserrbuf));
>             return -1;
>         }
>     }
>@@ -842,8 +863,9 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname,
>                                       const virNetDevBandwidthRate *rx)
> {
>     g_autoptr(virCommand) cmd = NULL;
>+    g_autofree char *errbuf = NULL;
>
>-    cmd = virNetDevOpenvswitchCreateCmd();
>+    cmd = virNetDevOpenvswitchCreateCmd(&errbuf);
>     virCommandAddArgList(cmd, "set", "Interface", ifname, NULL);
>     virCommandAddArgFormat(cmd, "ingress_policing_rate=%llu",
>                            rx->average * VIR_NETDEV_RX_TO_OVS);
>@@ -853,7 +875,8 @@ virNetDevOpenvswitchInterfaceSetRxQos(const char *ifname,
>
>     if (virCommandRun(cmd, NULL) < 0) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>-                       _("Unable to set vlan configuration on port %1$s"), ifname);
>+                       _("Unable to set vlan configuration on port %1$s: %2$s"),
>+                       ifname, NULLSTR(errbuf));
>         return -1;
>     }
>
>-- 
>2.41.0
>