Firstly, virCommandRun() does report an error on failure (which
in most cases is more accurate than what we overwrite it with).
Secondly, usually errno is not set (or gets overwritten in the
cleanup code) which makes virReportSystemError() report useless
error messages. Drop all virReportSystemError() calls in cases
like this (I've found three occurrences).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---

I was under impression that I've sent this one earlier. Well, maybe I
did not.

 src/storage/storage_backend_fs.c |  7 +------
 src/util/virdnsmasq.c            | 10 ++--------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 97148acebe..ae4e9a03a3 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -425,13 +425,8 @@ virStorageBackendExecuteMKFS(const char *device,
 
     virCommandAddArg(cmd, device);
 
-    if (virCommandRun(cmd, NULL) < 0) {
-        virReportSystemError(errno,
-                             _("Failed to make filesystem of "
-                               "type '%s' on device '%s'"),
-                             format, device);
+    if (virCommandRun(cmd, NULL) < 0)
         return -1;
-    }
 
     return 0;
 }
diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c
index 550f3179ae..42f62682c4 100644
--- a/src/util/virdnsmasq.c
+++ b/src/util/virdnsmasq.c
@@ -751,22 +751,16 @@ dnsmasqCapsRefreshInternal(dnsmasqCapsPtr caps, bool force)
     virCommandSetOutputBuffer(cmd, &version);
     virCommandAddEnvPassCommon(cmd);
     virCommandClearCaps(cmd);
-    if (virCommandRun(cmd, NULL) < 0) {
-        virReportSystemError(errno, _("failed to run '%s --version': %s"),
-                             caps->binaryPath, version);
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
-    }
     virCommandFree(cmd);
 
     cmd = virCommandNewArgList(caps->binaryPath, "--help", NULL);
     virCommandSetOutputBuffer(cmd, &help);
     virCommandAddEnvPassCommon(cmd);
     virCommandClearCaps(cmd);
-    if (virCommandRun(cmd, NULL) < 0) {
-        virReportSystemError(errno, _("failed to run '%s --help': %s"),
-                             caps->binaryPath, help);
+    if (virCommandRun(cmd, NULL) < 0)
         goto cleanup;
-    }
 
     if (virAsprintf(&complete, "%s\n%s", version, help) < 0)
         goto cleanup;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Fri, Apr 12, 2019 at 11:25:36AM +0200, Michal Privoznik wrote:
> Firstly, virCommandRun() does report an error on failure (which
> in most cases is more accurate than what we overwrite it with).
> Secondly, usually errno is not set (or gets overwritten in the
> cleanup code) which makes virReportSystemError() report useless
> error messages. Drop all virReportSystemError() calls in cases
> like this (I've found three occurrences).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
> 
> I was under impression that I've sent this one earlier. Well, maybe I
> did not.
> 
>  src/storage/storage_backend_fs.c |  7 +------
>  src/util/virdnsmasq.c            | 10 ++--------
>  2 files changed, 3 insertions(+), 14 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list