[libvirt] [PATCH 3/3] all: replacing virGetLastError with virGetLastErrorCode where we can

Erik Skultety eskultet at redhat.com
Wed May 9 11:57:31 UTC 2018


On Sat, May 05, 2018 at 01:04:21PM +0100, ramyelkest wrote:
> Replaced instances where we called virGetLastError just to
> either get the code or to check if an error exists with
> virGetLastErrorCode to avoid a validity pre-check.
>
> Signed-off-by: Ramy Elkest <ramyelkest at gmail.com>
> ---
>  src/locking/lock_driver_lockd.c |  3 +--
>  src/lxc/lxc_controller.c        |  4 +---
>  src/qemu/qemu_agent.c           |  3 +--
>  src/qemu/qemu_conf.c            |  3 +--
>  src/qemu/qemu_domain.c          |  2 +-
>  src/qemu/qemu_driver.c          | 12 ++++++------
>  src/qemu/qemu_hotplug.c         |  2 +-
>  src/qemu/qemu_migration.c       |  4 ++--
>  src/qemu/qemu_monitor.c         |  5 ++---
>  src/qemu/qemu_monitor_json.c    |  2 +-
>  src/qemu/qemu_process.c         |  4 ++--
>  src/remote/remote_driver.c      |  3 +--
>  src/rpc/virnetclient.c          |  2 +-
>  src/rpc/virnetlibsshsession.c   |  4 +---
>  src/util/virmodule.c            |  3 +--
>  src/util/virxml.c               |  4 ++--
>  tests/commandtest.c             |  2 +-
>  tests/testutils.c               |  6 ++----
>  tests/virhostcputest.c          |  2 +-
>  tests/virstoragetest.c          |  8 ++++----
>  tools/virsh-domain-monitor.c    |  7 +++----
>  tools/virsh-domain.c            |  4 +---
>  tools/virsh-util.c              |  3 +--
>  tools/vsh.c                     |  2 +-
>  24 files changed, 39 insertions(+), 55 deletions(-)
...

> -    err = virGetLastError();
> -    if (!err || err->code == VIR_ERR_OK)
> +    if (!virGetLastErrorCode())
>          rc = wantReboot ? 1 : 0;

Again, just a tiny detail, the convention we use in libvirt with unary '!' is
that we only use it with pointers, but explicitly compare against an int
otherwise, so ^this would become == VIR_ERR_OK (yes, we don't mention this in
our hacking guidelines...). So hopefully I didn't miss anything in the
attached diff which I'd squash in before pushing, feel free to check again.

The replacement itself is fine, so
Reviewed-by: Erik Skultety <eskultet at redhat.com>

-------------- next part --------------
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index e1ee864fe7..6ed5101602 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1348,7 +1348,7 @@ static int virLXCControllerMain(virLXCControllerPtr ctrl)

     virNetDaemonRun(ctrl->daemon);

-    if (!virGetLastErrorCode())
+    if (virGetLastErrorCode() == VIR_ERR_OK)
         rc = wantReboot ? 1 : 0;

  cleanup:
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 1fb31a7834..7c9b5eee1b 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -686,7 +686,7 @@ qemuAgentIO(int watch, int fd, int events, void *opaque)
             /* Already have an error, so clear any new error */
             virResetLastError();
         } else {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Error while processing monitor IO"));
             virCopyLastError(&mon->lastError);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0a551e67f3..e7b6a29aa7 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6100,7 +6100,7 @@ int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
 {
     qemuDomainObjExitMonitorInternal(driver, obj);
     if (!virDomainObjIsActive(obj)) {
-        if (!virGetLastErrorCode())
+        if (virGetLastErrorCode() == VIR_ERR_OK)
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("domain is no longer running"));
         return -1;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0c75b59ab8..f96ac493d9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1929,7 +1929,7 @@ static int qemuDomainResume(virDomainPtr dom)
         if (qemuProcessStartCPUs(driver, vm,
                                  VIR_DOMAIN_RUNNING_UNPAUSED,
                                  QEMU_ASYNC_JOB_NONE) < 0) {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_OPERATION_FAILED,
                                "%s", _("resume operation failed"));
             goto endjob;
@@ -3209,7 +3209,7 @@ qemuFileWrapperFDClose(virDomainObjPtr vm,
     ret = virFileWrapperFdClose(fd);
     virObjectLock(vm);
     if (!virDomainObjIsActive(vm)) {
-        if (!virGetLastErrorCode())
+        if (virGetLastErrorCode() == VIR_ERR_OK)
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("domain is no longer running"));
         ret = -1;
@@ -3991,7 +3991,7 @@ qemuDomainCoreDumpWithFormat(virDomainPtr dom,
                 event = virDomainEventLifecycleNewFromObj(vm,
                                                           VIR_DOMAIN_EVENT_SUSPENDED,
                                                           VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
-                if (!virGetLastErrorCode())
+                if (virGetLastErrorCode() == VIR_ERR_OK)
                     virReportError(VIR_ERR_OPERATION_FAILED,
                                    "%s", _("resuming after dump failed"));
             }
@@ -6638,7 +6638,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
         if (qemuProcessStartCPUs(driver, vm,
                                  VIR_DOMAIN_RUNNING_RESTORED,
                                  asyncJob) < 0) {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_OPERATION_FAILED,
                                "%s", _("failed to resume domain"));
             goto cleanup;
@@ -14084,7 +14084,7 @@ qemuDomainSnapshotCreateActiveInternal(virQEMUDriverPtr driver,
         event = virDomainEventLifecycleNewFromObj(vm,
                                          VIR_DOMAIN_EVENT_SUSPENDED,
                                          VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("resuming after snapshot failed"));
         }
@@ -15051,7 +15051,7 @@ qemuDomainSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
                                          VIR_DOMAIN_EVENT_SUSPENDED,
                                          VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR);
         qemuDomainEventQueue(driver, event);
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                            _("resuming after snapshot failed"));
         }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index c755aab57f..fef03f508a 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -212,7 +212,7 @@ qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver,
         if (rc > 0) {
             /* the caller called qemuMonitorEjectMedia which usually reports an
              * error. Report the failure in an off-chance that it didn't. */
-            if (!virGetLastErrorCode()) {
+            if (virGetLastErrorCode() == VIR_ERR_OK) {
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("timed out waiting for disk tray status update"));
             }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index bf4a144382..aa06f44a64 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4944,7 +4944,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
                                  inPostCopy ? VIR_DOMAIN_RUNNING_POSTCOPY
                                             : VIR_DOMAIN_RUNNING_MIGRATED,
                                  QEMU_ASYNC_JOB_MIGRATION_IN) < 0) {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_INTERNAL_ERROR,
                                "%s", _("resume operation failed"));
             /* Need to save the current error, in case shutting
@@ -5082,7 +5082,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
     /* Set a special error if Finish is expected to return NULL as a result of
      * successful call with retcode != 0
      */
-    if (retcode != 0 && !dom && !virGetLastErrorCode())
+    if (retcode != 0 && !dom && virGetLastErrorCode() == VIR_ERR_OK)
         virReportError(VIR_ERR_MIGRATE_FINISH_OK, NULL);
     return dom;
 }
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f68fd47401..944c46442b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -752,7 +752,7 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque)
             /* Already have an error, so clear any new error */
             virResetLastError();
         } else {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                                _("Error while processing monitor IO"));
             virCopyLastError(&mon->lastError);
@@ -1031,7 +1031,7 @@ qemuMonitorClose(qemuMonitorPtr mon)
     /* Propagate existing monitor error in case the current thread has no
      * error set.
      */
-    if (mon->lastError.code != VIR_ERR_OK && !virGetLastErrorCode())
+    if (mon->lastError.code != VIR_ERR_OK && virGetLastErrorCode() == VIR_ERR_OK)
         virSetError(&mon->lastError);

     virObjectUnlock(mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 306067b5cb..39cfabd683 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4338,7 +4338,7 @@ qemuMonitorJSONDiskNameLookup(qemuMonitorPtr mon,
     }
     /* Guarantee an error when returning NULL, but don't override a
      * more specific error if one was already generated.  */
-    if (!ret && !virGetLastErrorCode())
+    if (!ret && virGetLastErrorCode() == VIR_ERR_OK)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unable to find backing name for device %s"),
                        device);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b8f195ce00..e4fdde7aa2 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -558,7 +558,7 @@ qemuProcessFakeReboot(void *opaque)
     if (qemuProcessStartCPUs(driver, vm,
                              reason,
                              QEMU_ASYNC_JOB_NONE) < 0) {
-        if (!virGetLastErrorCode())
+        if (virGetLastErrorCode() == VIR_ERR_OK)
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            "%s", _("resume operation failed"));
         goto endjob;
@@ -6259,7 +6259,7 @@ qemuProcessFinishStartup(virQEMUDriverPtr driver,
         if (qemuProcessStartCPUs(driver, vm,
                                  VIR_DOMAIN_RUNNING_BOOTED,
                                  asyncJob) < 0) {
-            if (!virGetLastErrorCode())
+            if (virGetLastErrorCode() == VIR_ERR_OK)
                 virReportError(VIR_ERR_OPERATION_FAILED, "%s",
                                _("resume operation failed"));
             goto cleanup;
diff --git a/src/rpc/virnetlibsshsession.c b/src/rpc/virnetlibsshsession.c
index b2cb5c1e02..87a8d63f08 100644
--- a/src/rpc/virnetlibsshsession.c
+++ b/src/rpc/virnetlibsshsession.c
@@ -499,7 +499,7 @@ virNetLibsshImportPrivkey(virNetLibsshSessionPtr sess,
         err = SSH_AUTH_ERROR;
         goto error;
     } else if (ret == SSH_ERROR) {
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             virReportError(VIR_ERR_AUTH_FAILED,
                            _("error while opening private key '%s', wrong "
                              "passphrase?"),
diff --git a/src/util/virmodule.c b/src/util/virmodule.c
index 9d7d249cb9..b19a787e4f 100644
--- a/src/util/virmodule.c
+++ b/src/util/virmodule.c
@@ -123,7 +123,7 @@ virModuleLoad(const char *path,
     if ((*regsym)() < 0) {
         /* regsym() should report an error itself, but lets
          * just make sure */
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to execute symbol '%s' in module '%s'"),
                            regfunc, path);
diff --git a/src/util/virxml.c b/src/util/virxml.c
index b5caee7394..a03a747e60 100644
--- a/src/util/virxml.c
+++ b/src/util/virxml.c
@@ -845,7 +845,7 @@ virXMLParseHelper(int domcode,
     xmlFreeDoc(xml);
     xml = NULL;

-    if (!virGetLastErrorCode()) {
+    if (virGetLastErrorCode() == VIR_ERR_OK) {
         virGenericReportError(domcode, VIR_ERR_XML_ERROR,
                               "%s", _("failed to parse xml document"));
     }
diff --git a/tests/commandtest.c b/tests/commandtest.c
index 3bb7220509..744a387aa0 100644
--- a/tests/commandtest.c
+++ b/tests/commandtest.c
@@ -127,7 +127,7 @@ static int test0(const void *unused ATTRIBUTE_UNUSED)
     if (virCommandRun(cmd, NULL) == 0)
         goto cleanup;

-    if (!virGetLastErrorCode())
+    if (virGetLastErrorCode() == VIR_ERR_OK)
         goto cleanup;

     virResetLastError();
diff --git a/tests/testutils.c b/tests/testutils.c
index f0719c88de..423f4bfdff 100644
--- a/tests/testutils.c
+++ b/tests/testutils.c
@@ -257,7 +257,7 @@ virTestRun(const char *title,
                 fprintf(stderr, " alloc %zu failed but no err status\n", i + 1);
 # endif
             } else {
-                if (!virGetLastErrorCode()) {
+                if (virGetLastErrorCode() == VIR_ERR_OK) {
 # if 0
                     fprintf(stderr, " alloc %zu failed but no error report\n", i + 1);
 # endif
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index de68ea07d4..5221e82ad2 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -333,7 +333,7 @@ testStorageChain(const void *args)
         goto cleanup;
     }
     if (data->flags & EXP_WARN) {
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             fprintf(stderr, "call should have warned\n");
             goto cleanup;
         }
@@ -449,7 +449,7 @@ testStorageLookup(const void *args)
                                        idx, NULL);

     if (!data->expResult) {
-        if (!virGetLastErrorCode()) {
+        if (virGetLastErrorCode() == VIR_ERR_OK) {
             fprintf(stderr, "call should have failed\n");
             ret = -1;
         }
diff --git a/tools/vsh.c b/tools/vsh.c
index 8f8ffb07c8..18f447ac1a 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -266,7 +266,7 @@ vshSaveLibvirtHelperError(void)
     if (last_error)
         return;

-    if (!virGetLastErrorCode())
+    if (virGetLastErrorCode() == VIR_ERR_OK)
         return;

     vshSaveLibvirtError();


More information about the libvir-list mailing list