[libvirt] [PATCH 00/10] Clean up error preservation/restoration
I was going through some old git trees and found this one started before I 'altered' my Red Hat career path. Seeing as no one has picked it up from https://wiki.libvirt.org/page/BiteSizedTasks - figured I may as well post it rather than throw it away ;-) The change essentially alters virSaveLastError ... virSetError and virFreeError paths to use virErrorPreserveLast & virErrorRestore. John Ferlan (10): conf: Use consistent error preservation and restoration calls src: Use consistent error preservation and restoration calls libxl: Use consistent error preservation and restoration calls lxc: Use consistent error preservation and restoration calls qemu: Use consistent error preservation and restoration calls remote: Use consistent error preservation and restoration calls storage: Use consistent error preservation and restoration calls util: Use consistent error preservation and restoration calls vz: Use consistent error preservation and restoration calls tools: Use consistent error preservation and restoration calls src/conf/domain_conf.c | 7 +-- src/libvirt-stream.c | 36 +++++------- src/libxl/libxl_migration.c | 18 ++---- src/lxc/lxc_driver.c | 9 ++- src/lxc/lxc_process.c | 7 +-- src/qemu/qemu_cgroup.c | 14 ++--- src/qemu/qemu_command.c | 12 ++-- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 36 +++++------- src/qemu/qemu_migration.c | 85 +++++++++------------------ src/qemu/qemu_migration_params.c | 9 ++- src/qemu/qemu_monitor.c | 19 +++--- src/qemu/qemu_process.c | 7 +-- src/remote/remote_daemon_stream.c | 17 +++--- src/storage/storage_backend_disk.c | 5 +- src/storage/storage_backend_logical.c | 5 +- src/storage/storage_driver.c | 8 +-- src/util/vircgroup.c | 18 +++--- src/util/virfirewall.c | 6 +- src/vz/vz_driver.c | 9 +-- tools/virsh.c | 7 +-- tools/virt-admin.c | 7 +-- 22 files changed, 137 insertions(+), 211 deletions(-) -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 10d6bf0eea..188ea80c3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -23624,17 +23624,14 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src, return true; error: - err = virSaveLastError(); + virErrorPreserveLast(&err); strSrc = virDomainDefFormat(src, NULL, 0); strDst = virDomainDefFormat(dst, NULL, 0); VIR_DEBUG("XMLs that failed stability check were: src=\"%s\", dst=\"%s\"", NULLSTR(strSrc), NULLSTR(strDst)); - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); return false; } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt-stream.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/libvirt-stream.c b/src/libvirt-stream.c index ef83696bcd..fb646d0aef 100644 --- a/src/libvirt-stream.c +++ b/src/libvirt-stream.c @@ -622,12 +622,11 @@ virStreamSendAll(virStreamPtr stream, VIR_FREE(bytes); if (ret != 0) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); virStreamAbort(stream); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virDispatchError(stream->conn); } @@ -794,12 +793,11 @@ int virStreamSparseSendAll(virStreamPtr stream, VIR_FREE(bytes); if (ret != 0) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); virStreamAbort(stream); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virDispatchError(stream->conn); } @@ -900,12 +898,11 @@ virStreamRecvAll(virStreamPtr stream, VIR_FREE(bytes); if (ret != 0) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); virStreamAbort(stream); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virDispatchError(stream->conn); } @@ -1034,12 +1031,11 @@ virStreamSparseRecvAll(virStreamPtr stream, VIR_FREE(bytes); if (ret != 0) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + + virErrorPreserveLast(&orig_err); virStreamAbort(stream); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virDispatchError(stream->conn); } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 8a41e9374d..770a300316 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -1049,7 +1049,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver, if (uri_out) { if (virTypedParamsReplaceString(¶ms, &nparams, VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } } else { @@ -1067,7 +1067,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver, uri_out, NULL, flags); if (ret < 0) { notify_source = false; - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); } cancelled = (ret < 0); @@ -1094,7 +1094,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver, * one we need to preserve it in case confirm3 overwrites */ if (!orig_err) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); confirm: if (notify_source) { @@ -1119,10 +1119,7 @@ libxlDoMigrateSrcP2P(libxlDriverPrivatePtr driver, ret = -1; } - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(cookieout); VIR_FREE(dom_xml); @@ -1200,15 +1197,12 @@ libxlDomainMigrationSrcPerformP2P(libxlDriverPrivatePtr driver, } cleanup: - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); virObjectUnlock(vm); virObjectUnref(dconn); virObjectUnref(cfg); virObjectLock(vm); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/lxc/lxc_driver.c | 9 ++++----- src/lxc/lxc_process.c | 7 ++----- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 5c7a9140b2..204a3ed522 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -1857,12 +1857,11 @@ static int lxcSetVcpuBWLive(virCgroupPtr cgroup, unsigned long long period, error: if (period) { - virErrorPtr saved = virSaveLastError(); + virErrorPtr saved; + + virErrorPreserveLast(&saved); virCgroupSetCpuCfsPeriod(cgroup, old_period); - if (saved) { - virSetError(saved); - virFreeError(saved); - } + virErrorRestore(&saved); } return -1; diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index da0b055b85..d9939f102d 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1568,7 +1568,7 @@ int virLXCProcessStart(virConnectPtr conn, rc = -1; } if (rc != 0) { - err = virSaveLastError(); + virErrorPreserveLast(&err); virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); } virCommandFree(cmd); @@ -1582,10 +1582,7 @@ int virLXCProcessStart(virConnectPtr conn, virObjectUnref(cfg); virObjectUnref(caps); - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); return rc; } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_cgroup.c | 14 +++--- src/qemu/qemu_command.c | 12 ++--- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 36 ++++++-------- src/qemu/qemu_migration.c | 85 +++++++++++--------------------- src/qemu/qemu_migration_params.c | 9 ++-- src/qemu/qemu_monitor.c | 19 +++---- src/qemu/qemu_process.c | 7 +-- 8 files changed, 72 insertions(+), 117 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index f1776a7c0b..8b915d124c 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -1149,12 +1149,11 @@ qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, error: if (period) { - virErrorPtr saved = virSaveLastError(); + virErrorPtr saved; + + virErrorPreserveLast(&saved); ignore_value(virCgroupSetCpuCfsPeriod(cgroup, old_period)); - if (saved) { - virSetError(saved); - virFreeError(saved); - } + virErrorRestore(&saved); } return -1; @@ -1362,10 +1361,9 @@ qemuCgroupEmulatorAllNodesRestore(qemuCgroupEmulatorAllNodesDataPtr data) if (!data) return; - err = virSaveLastError(); + virErrorPreserveLast(&err); virCgroupSetCpusetMems(data->emulatorCgroup, data->emulatorMemMask); - virSetError(err); - virFreeError(err); + virErrorRestore(&err); qemuCgroupEmulatorAllNodesDataFree(data); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c2a19689c5..2e5e2a75bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8644,10 +8644,11 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, ret = 0; cleanup: if (ret < 0) { - virErrorPtr saved_err = virSaveLastError(); + virErrorPtr saved_err; + + virErrorPreserveLast(&saved_err); virDomainConfNWFilterTeardown(net); - virSetError(saved_err); - virFreeError(saved_err); + virErrorRestore(&saved_err); } for (i = 0; vhostfd && i < vhostfdSize; i++) { if (ret < 0) @@ -8730,11 +8731,10 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver, error: /* free up any resources in the network driver * but don't overwrite the original error */ - originalError = virSaveLastError(); + virErrorPreserveLast(&originalError); for (i = 0; last_good_net != -1 && i <= last_good_net; i++) virDomainConfNWFilterTeardown(def->nets[i]); - virSetError(originalError); - virFreeError(originalError); + virErrorRestore(&originalError); return -1; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c638077aa8..43aec2e3d7 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -9174,7 +9174,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, /* We don't care about errors logging taint info, so * preserve original error, and clear any error that * is raised */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); if (!(timestamp = virTimeStringNow())) goto cleanup; @@ -9198,10 +9198,7 @@ void qemuDomainObjTaint(virQEMUDriverPtr driver, cleanup: VIR_FREE(timestamp); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 6ce6348593..c662676be3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3436,7 +3436,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, endjob: if (ret < 0) { if (was_running && virDomainObjIsActive(vm)) { - virErrorPtr save_err = virSaveLastError(); + virErrorPtr save_err; + virErrorPreserveLast(&save_err); if (qemuProcessStartCPUs(driver, vm, VIR_DOMAIN_RUNNING_SAVE_CANCELED, QEMU_ASYNC_JOB_SAVE) < 0) { @@ -3446,8 +3447,7 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR)); } - virSetError(save_err); - virFreeError(save_err); + virErrorRestore(&save_err); } } qemuDomainObjEndAsyncJob(driver, vm); @@ -6729,7 +6729,9 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, goto cleanup; if (!virDomainDefCheckABIStability(def, newdef_migr, driver->xmlopt)) { - virErrorPtr err = virSaveLastError(); + virErrorPtr save_err; + + virErrorPreserveLast(&save_err); /* Due to a bug in older version of external snapshot creation * code, the XML saved in the save image was not a migratable @@ -6738,11 +6740,10 @@ qemuDomainSaveImageUpdateDef(virQEMUDriverPtr driver, * the user provided XML if the check against the migratable XML * fails. Snapshots created prior to v1.1.3 have this issue. */ if (!virDomainDefCheckABIStability(def, newdef, driver->xmlopt)) { - virSetError(err); - virFreeError(err); + virErrorRestore(&save_err); goto cleanup; } - virFreeError(err); + virFreeError(save_err); /* use the user provided XML */ VIR_STEAL_PTR(ret, newdef); @@ -6991,7 +6992,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, * must manually kill it and ignore any error related to * the process */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); VIR_FORCE_CLOSE(intermediatefd); VIR_FORCE_CLOSE(*fd); } @@ -7002,10 +7003,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } VIR_DEBUG("Decompression binary stderr: %s", NULLSTR(errbuf)); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } VIR_FORCE_CLOSE(intermediatefd); @@ -14739,13 +14737,11 @@ qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver G_GNUC_UNUSED, agent = qemuDomainObjEnterAgent(vm); if (!report) - err = virSaveLastError(); + virErrorPreserveLast(&err); thawed = qemuAgentFSThaw(agent); - if (!report) - virSetError(err); qemuDomainObjExitAgent(vm, agent); - virFreeError(err); + virErrorRestore(&err); return thawed; } @@ -18897,16 +18893,14 @@ qemuDomainBlockCommit(virDomainPtr dom, endjob: if (ret < 0 && clean_access) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + virErrorPreserveLast(&orig_err); /* Revert access to read-only, if possible. */ qemuDomainStorageSourceAccessAllow(driver, vm, baseSource, true, false); if (top_parent && top_parent != disk->src) qemuDomainStorageSourceAccessAllow(driver, vm, top_parent, true, false); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } qemuBlockJobStartupFinalize(vm, job); qemuDomainObjEndJob(driver, vm); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 52ef1874aa..8ad1e6fcaf 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -728,7 +728,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, if (rv != 0) { if (rv < 0) { if (!err) - err = virSaveLastError(); + virErrorPreserveLast(&err); failed = true; } qemuBlockJobSyncEnd(vm, job, asyncJob); @@ -753,7 +753,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, } if (failed && !err) - err = virSaveLastError(); + virErrorPreserveLast(&err); if (virDomainObjWait(vm) < 0) goto cleanup; @@ -775,10 +775,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver, ret = failed ? -1 : 0; cleanup: - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); return ret; } @@ -3001,15 +2998,16 @@ qemuMigrationSrcConfirmPhase(virQEMUDriverPtr driver, virObjectEventStateQueue(driver->domainEventState, event); qemuDomainEventEmitJobCompleted(driver, vm); } else { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; int reason; + virErrorPreserveLast(&orig_err); + /* cancel any outstanding NBD jobs */ qemuMigrationSrcNBDCopyCancel(driver, vm, false, QEMU_ASYNC_JOB_MIGRATION_OUT, NULL); - virSetError(orig_err); - virFreeError(orig_err); + virErrorRestore(&orig_err); if (virDomainObjGetState(vm, &reason) == VIR_DOMAIN_PAUSED && reason == VIR_DOMAIN_PAUSED_POSTCOPY) @@ -3213,16 +3211,13 @@ static void qemuMigrationSrcIOFunc(void *arg) return; abrt: - err = virSaveLastError(); + virErrorPreserveLast(&err); if (err && err->code == VIR_ERR_OK) { virFreeError(err); err = NULL; } virStreamAbort(data->st); - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); error: /* Let the source qemu know that the transfer cant continue anymore. @@ -3704,15 +3699,12 @@ qemuMigrationSrcRun(virQEMUDriverPtr driver, if (events) priv->signalIOError = false; - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; error: - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); if (virDomainObjIsActive(vm)) { if (cancel && @@ -3967,7 +3959,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare2 did not set uri")); cancelled = true; - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -3990,7 +3982,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* If Perform returns < 0, then we need to cancel the VM * startup on the destination @@ -4023,10 +4015,7 @@ qemuMigrationSrcPerformPeer2Peer2(virQEMUDriverPtr driver, virObjectUnref(st); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(uri_out); VIR_FREE(cookie); @@ -4200,13 +4189,13 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, if (useParams && virTypedParamsReplaceString(¶ms, &nparams, VIR_MIGRATE_PARAM_URI, uri_out) < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } } else if (!uri && !(flags & VIR_MIGRATE_TUNNELLED)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domainMigratePrepare3 did not set uri")); - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -4239,7 +4228,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, /* Perform failed. Make sure Finish doesn't overwrite the error */ if (ret < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); } else { qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE); @@ -4331,7 +4320,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, * one we need to preserve it in case confirm3 overwrites */ if (!orig_err) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* * If cancelled, then src VM will be restarted, else @@ -4363,10 +4352,7 @@ qemuMigrationSrcPerformPeer2Peer3(virQEMUDriverPtr driver, virObjectUnref(st); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(uri_out); VIR_FREE(cookiein); VIR_FREE(cookieout); @@ -4542,15 +4528,12 @@ qemuMigrationSrcPerformPeer2Peer(virQEMUDriverPtr driver, } cleanup: - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); qemuDomainObjEnterRemote(vm); virConnectUnregisterCloseCallback(dconn, qemuMigrationSrcConnectionClosed); virObjectUnref(dconn); ignore_value(qemuDomainObjExitRemote(vm, false)); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virObjectUnref(cfg); return ret; } @@ -4639,7 +4622,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, endjob: if (ret < 0) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* v2 proto has no confirm phase so we need to reset migration parameters * here @@ -4659,10 +4642,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, qemuDomainRemoveInactiveJob(driver, vm); } - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); cleanup: virObjectEventStateQueue(driver->domainEventState, event); @@ -5074,7 +5054,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, /* Need to save the current error, in case shutting * down the process overwrites it */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* * In v3 protocol, the source VM is still available to @@ -5203,10 +5183,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver, VIR_FREE(priv->origname); virDomainObjEndAPI(&vm); qemuMigrationCookieFree(mig); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virObjectUnref(cfg); /* Set a special error if Finish is expected to return NULL as a result of @@ -5311,7 +5288,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, if (rc < 0) { if (rc == -2) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); virCommandAbort(cmd); if (virDomainObjIsActive(vm) && qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { @@ -5330,7 +5307,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, cleanup: if (ret < 0 && !orig_err) - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); /* Restore max migration bandwidth */ if (virDomainObjIsActive(vm) && @@ -5348,10 +5325,7 @@ qemuMigrationSrcToFile(virQEMUDriverPtr driver, virDomainObjPtr vm, virCommandFree(cmd); } - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); return ret; } @@ -5539,8 +5513,7 @@ qemuMigrationDstErrorReport(virQEMUDriverPtr driver, VIR_DEBUG("Restoring saved incoming migration error for domain %s: %s", name, err->message); - virSetError(err); - virFreeError(err); + virErrorRestore(&err); } diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 85fa8f8de5..1acaebcfd0 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -1197,7 +1197,9 @@ qemuMigrationParamsReset(virQEMUDriverPtr driver, qemuMigrationParamsPtr origParams, unsigned long apiFlags) { - virErrorPtr err = virSaveLastError(); + virErrorPtr err; + + virErrorPreserveLast(&err); VIR_DEBUG("Resetting migration parameters %p, flags 0x%lx", origParams, apiFlags); @@ -1211,10 +1213,7 @@ qemuMigrationParamsReset(virQEMUDriverPtr driver, qemuMigrationParamsResetTLS(driver, vm, asyncJob, origParams, apiFlags); cleanup: - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); } diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 49f9159315..68a7cfc453 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -929,17 +929,17 @@ qemuMonitorClose(qemuMonitorPtr mon) */ if (mon->msg) { if (mon->lastError.code == VIR_ERR_OK) { - virErrorPtr err = virSaveLastError(); + virErrorPtr err; + + virErrorPreserveLast(&err); virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("QEMU monitor was closed")); virCopyLastError(&mon->lastError); - if (err) { - virSetError(err); - virFreeError(err); - } else { + if (err) + virErrorRestore(&err); + else virResetLastError(); - } } mon->msg->finished = 1; virCondSignal(&mon->notify); @@ -2671,17 +2671,14 @@ qemuMonitorCloseFileHandle(qemuMonitorPtr mon, VIR_DEBUG("fdname=%s", fdname); - error = virSaveLastError(); + virErrorPreserveLast(&error); QEMU_CHECK_MONITOR_GOTO(mon, cleanup); ret = qemuMonitorJSONCloseFileHandle(mon, fdname); cleanup: - if (error) { - virSetError(error); - virFreeError(error); - } + virErrorRestore(&error); return ret; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9c50c4a1d8..d82d5eb202 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7412,7 +7412,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* This method is routinely used in clean up paths. Disable error * reporting so we don't squash a legit error. */ - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); if (asyncJob != QEMU_ASYNC_JOB_NONE) { if (qemuDomainObjBeginNestedJob(driver, vm, asyncJob) < 0) @@ -7705,10 +7705,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm); cleanup: - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); virObjectUnref(conn); virObjectUnref(cfg); } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/remote/remote_daemon_stream.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c index d7fcb1bf42..c4f14a27ef 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -229,15 +229,18 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) int ret; virNetMessagePtr msg; virNetMessageError rerr; - virErrorPtr origErr = virSaveLastError(); + virErrorPtr origErr; + + virErrorPreserveLast(&origErr); memset(&rerr, 0, sizeof(rerr)); stream->closed = true; virStreamEventRemoveCallback(stream->st); virStreamAbort(stream->st); if (origErr && origErr->code != VIR_ERR_OK) { - virSetError(origErr); + virErrorRestore(&origErr); } else { + virFreeError(origErr); if (events & VIR_STREAM_EVENT_HANGUP) virReportError(VIR_ERR_RPC, "%s", _("stream had unexpected termination")); @@ -245,7 +248,6 @@ daemonStreamEvent(virStreamPtr st, int events, void *opaque) virReportError(VIR_ERR_RPC, "%s", _("stream had I/O failure")); } - virFreeError(origErr); msg = virNetMessageNew(false); if (!msg) { @@ -549,7 +551,9 @@ daemonStreamHandleWriteData(virNetServerClientPtr client, return 1; } else if (ret < 0) { virNetMessageError rerr; - virErrorPtr err = virSaveLastError(); + virErrorPtr err; + + virErrorPreserveLast(&err); memset(&rerr, 0, sizeof(rerr)); @@ -558,10 +562,7 @@ daemonStreamHandleWriteData(virNetServerClientPtr client, virStreamEventRemoveCallback(stream->st); virStreamAbort(stream->st); - if (err) { - virSetError(err); - virFreeError(err); - } + virErrorRestore(&err); return virNetServerProgramSendReplyError(stream->prog, client, -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_disk.c | 5 ++--- src/storage/storage_backend_logical.c | 5 ++--- src/storage/storage_driver.c | 8 +++----- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 5bf704bcc8..d2f7b94224 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -918,10 +918,9 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, /* Best effort to remove the partition. Ignore any errors * since we could be calling this with vol->target.path == NULL */ - save_err = virSaveLastError(); + virErrorPreserveLast(&save_err); ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); - virSetError(save_err); - virFreeError(save_err); + virErrorRestore(&save_err); return -1; } diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 6e468b3579..bfedb06a91 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -969,10 +969,9 @@ virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, return 0; error: - err = virSaveLastError(); + virErrorPreserveLast(&err); virStorageBackendLogicalDeleteVol(pool, vol, 0); - virSetError(err); - virFreeError(err); + virErrorRestore(&err); return -1; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 6516b0943d..e8551ba57e 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -81,18 +81,16 @@ storagePoolRefreshFailCleanup(virStorageBackendPtr backend, virStoragePoolObjPtr obj, const char *stateFile) { - virErrorPtr orig_err = virSaveLastError(); + virErrorPtr orig_err; + virErrorPreserveLast(&orig_err); virStoragePoolObjClearVols(obj); if (stateFile) unlink(stateFile); if (backend->stopPool) backend->stopPool(obj); - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/vircgroup.c | 18 ++++++++---------- src/util/virfirewall.c | 6 ++++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index c6e4cf2dde..4c83e37ec4 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -1168,13 +1168,12 @@ virCgroupNewMachineSystemd(const char *name, } if (virCgroupAddProcess(*group, pidleader) < 0) { - virErrorPtr saved = virSaveLastError(); + virErrorPtr saved; + + virErrorPreserveLast(&saved); virCgroupRemove(*group); virCgroupFree(group); - if (saved) { - virSetError(saved); - virFreeError(saved); - } + virErrorRestore(&saved); } return 0; @@ -1220,13 +1219,12 @@ virCgroupNewMachineManual(const char *name, goto cleanup; if (virCgroupAddProcess(*group, pidleader) < 0) { - virErrorPtr saved = virSaveLastError(); + virErrorPtr saved; + + virErrorPreserveLast(&saved); virCgroupRemove(*group); virCgroupFree(group); - if (saved) { - virSetError(saved); - virFreeError(saved); - } + virErrorRestore(&saved); } done: diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 0d4bfae8f8..62f404afd6 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -838,7 +838,9 @@ virFirewallApply(virFirewallPtr firewall) if (virFirewallApplyGroup(firewall, i) < 0) { VIR_DEBUG("Rolling back groups up to %zu for %p", i, firewall); size_t first = i; - VIR_AUTOPTR(virError) saved_error = virSaveLastError(); + virErrorPtr saved_error; + + virErrorPreserveLast(&saved_error); /* * Look at any inheritance markers to figure out @@ -858,7 +860,7 @@ virFirewallApply(virFirewallPtr firewall) virFirewallRollbackGroup(firewall, j); } - virSetError(saved_error); + virErrorRestore(&saved_error); VIR_DEBUG("Done rolling back groups for %p", firewall); goto cleanup; } -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/vz/vz_driver.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c index 917acdae57..b4049261ae 100644 --- a/src/vz/vz_driver.c +++ b/src/vz/vz_driver.c @@ -3206,7 +3206,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom, */ if (uri && virTypedParamsReplaceString(¶ms, &nparams, VIR_MIGRATE_PARAM_URI, uri) < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -3216,7 +3216,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom, cookieoutlen = 0; if (vzDomainMigratePerformStep(dom, driver, params, nparams, cookiein, cookieinlen, flags) < 0) { - orig_err = virSaveLastError(); + virErrorPreserveLast(&orig_err); goto finish; } @@ -3242,10 +3242,7 @@ vzDomainMigratePerformP2P(virDomainObjPtr dom, /* confirm step is NOOP thus no need to call it */ done: - if (orig_err) { - virSetError(orig_err); - virFreeError(orig_err); - } + virErrorRestore(&orig_err); VIR_FREE(dom_xml); VIR_FREE(uri); VIR_FREE(cookiein); -- 2.20.1
Provide some consistency over error message variable name and usage when saving error messages across possible other errors or possibility of resetting of the last error. Instead of virSaveLastError paired up with virSetError and virFreeError, we should use the newer virErrorPreserveLast and virRestoreError. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh.c | 7 ++----- tools/virt-admin.c | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 4aae877160..a3553ddd36 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -93,7 +93,7 @@ virshCatchDisconnect(virConnectPtr conn, virErrorPtr error; char *uri; - error = virSaveLastError(); + virErrorPreserveLast(&error); uri = virConnectGetURI(conn); switch ((virConnectCloseReason) reason) { @@ -114,10 +114,7 @@ virshCatchDisconnect(virConnectPtr conn, vshError(ctl, _(str), NULLSTR(uri)); VIR_FREE(uri); - if (error) { - virSetError(error); - virFreeError(error); - } + virErrorRestore(&error); disconnected++; vshEventDone(ctl); } diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c0cc0999cb..a820e7241d 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -124,7 +124,7 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn G_GNUC_UNUSED, if (reason == VIR_CONNECT_CLOSE_REASON_CLIENT) return; - error = virSaveLastError(); + virErrorPreserveLast(&error); uri = virAdmConnectGetURI(conn); switch ((virConnectCloseReason) reason) { @@ -146,10 +146,7 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn G_GNUC_UNUSED, vshError(ctl, _(str), NULLSTR(uri)); VIR_FREE(uri); - if (error) { - virSetError(error); - virFreeError(error); - } + virErrorRestore(&error); } static int -- 2.20.1
On 10/15/19 7:37 PM, John Ferlan wrote:
I was going through some old git trees and found this one started before I 'altered' my Red Hat career path. Seeing as no one has picked it up from https://wiki.libvirt.org/page/BiteSizedTasks - figured I may as well post it rather than throw it away ;-)
The change essentially alters virSaveLastError ... virSetError and virFreeError paths to use virErrorPreserveLast & virErrorRestore.
John Ferlan (10): conf: Use consistent error preservation and restoration calls src: Use consistent error preservation and restoration calls libxl: Use consistent error preservation and restoration calls lxc: Use consistent error preservation and restoration calls qemu: Use consistent error preservation and restoration calls remote: Use consistent error preservation and restoration calls storage: Use consistent error preservation and restoration calls util: Use consistent error preservation and restoration calls vz: Use consistent error preservation and restoration calls tools: Use consistent error preservation and restoration calls
src/conf/domain_conf.c | 7 +-- src/libvirt-stream.c | 36 +++++------- src/libxl/libxl_migration.c | 18 ++---- src/lxc/lxc_driver.c | 9 ++- src/lxc/lxc_process.c | 7 +-- src/qemu/qemu_cgroup.c | 14 ++--- src/qemu/qemu_command.c | 12 ++-- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 36 +++++------- src/qemu/qemu_migration.c | 85 +++++++++------------------ src/qemu/qemu_migration_params.c | 9 ++- src/qemu/qemu_monitor.c | 19 +++--- src/qemu/qemu_process.c | 7 +-- src/remote/remote_daemon_stream.c | 17 +++--- src/storage/storage_backend_disk.c | 5 +- src/storage/storage_backend_logical.c | 5 +- src/storage/storage_driver.c | 8 +-- src/util/vircgroup.c | 18 +++--- src/util/virfirewall.c | 6 +- src/vz/vz_driver.c | 9 +-- tools/virsh.c | 7 +-- tools/virt-admin.c | 7 +-- 22 files changed, 137 insertions(+), 211 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
John Ferlan -
Michal Privoznik