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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
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();
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
