From 7fb2d1339ae55e5081dccd768a61435d4b453c24 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Thu, 6 Dec 2018 12:33:04 -0500 Subject: [PATCH] qemu: Use consistent error preservation and restoration calls 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 Reviewed-by: Michal Privoznik --- 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 dacbb603b0..5a58e77fa4 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 7b6cc725f4..e98195b1d7 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 fb008fcbb0..09b6c9a570 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 2e422b5882..cee3257a8a 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 */ ret = g_steal_pointer(&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 882c6810a0..c65491f9d6 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 159205527e..65f533e451 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 333acbe9b9..86d3800108 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 724e950101..aaca2fc7d6 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.39.5