Recently we moved to returning errp. Why to keep int return value?
Generally it doesn't help: you can't use in a logic of handling
an error, as you are never sure, that in future the logic in
the stack will not change: it may start to return another error
code in the same case, or return same error code in another case.
Actually, we can only rely on concrete errno code when get it
_directly_ from documented library function or syscall. This way we
handle for example EINTR. But later in a stack, we can only add
this errno to the textual error by strerror().
Let this new, recently added API be simpler and clearer, let it
return simple boolean value, so we shouldn't think:
- should we handle different error codes differently
(if yes - how exactly, if no - why do we need this information?)
- should we add it to errp, or it was already added earlier in
the stack
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
backends/tpm/tpm_emulator.c | 10 ++++------
docs/devel/migration/main.rst | 6 +++---
include/migration/vmstate.h | 6 +++---
migration/vmstate.c | 31 +++++++++++++------------------
4 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index dacfca5ab7..3937ac5572 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -947,24 +947,22 @@ static void tpm_emulator_vm_state_change(void *opaque,
bool running,
/*
* Load the TPM state blobs into the TPM.
- *
- * Returns negative errno codes in case of error.
*/
-static int tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
+static bool tpm_emulator_post_load(void *opaque, int version_id, Error **errp)
{
TPMBackend *tb = opaque;
int ret;
ret = tpm_emulator_set_state_blobs(tb, errp);
if (ret < 0) {
- return ret;
+ return false;
}
if (tpm_emulator_startup_tpm_resume(tb, 0, true) < 0) {
- return -EIO;
+ return false;
}
- return 0;
+ return true;
}
static const VMStateDescription vmstate_tpm_emulator = {
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 1afe7b9689..234d280249 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -446,15 +446,15 @@ The functions to do that are inside a vmstate definition,
and are called:
Following are the errp variants of these functions.
-- ``int (*pre_load_errp)(void *opaque, Error **errp);``
+- ``bool (*pre_load_errp)(void *opaque, Error **errp);``
This function is called before we load the state of one device.
-- ``int (*post_load_errp)(void *opaque, int version_id, Error **errp);``
+- ``bool (*post_load_errp)(void *opaque, int version_id, Error **errp);``
This function is called after we load the state of one device.
-- ``int (*pre_save_errp)(void *opaque, Error **errp);``
+- ``bool (*pre_save_errp)(void *opaque, Error **errp);``
This function is called before we save the state of one device.
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 63ccaee07a..dbe330dd5f 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -218,11 +218,11 @@ struct VMStateDescription {
int minimum_version_id;
MigrationPriority priority;
int (*pre_load)(void *opaque);
- int (*pre_load_errp)(void *opaque, Error **errp);
+ bool (*pre_load_errp)(void *opaque, Error **errp);
int (*post_load)(void *opaque, int version_id);
- int (*post_load_errp)(void *opaque, int version_id, Error **errp);
+ bool (*post_load_errp)(void *opaque, int version_id, Error **errp);
int (*pre_save)(void *opaque);
- int (*pre_save_errp)(void *opaque, Error **errp);
+ bool (*pre_save_errp)(void *opaque, Error **errp);
int (*post_save)(void *opaque);
bool (*needed)(void *opaque);
bool (*dev_unplug_pending)(void *opaque);
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 81eadde553..026fd6f1a4 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -153,15 +153,12 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
return -EINVAL;
}
- if (vmsd->pre_load_errp) {
- ret = vmsd->pre_load_errp(opaque, errp);
- if (ret < 0) {
- error_prepend(errp, "pre load hook failed for: '%s', "
- "version_id: %d, minimum version_id: %d, "
- "ret: %d: ", vmsd->name, vmsd->version_id,
- vmsd->minimum_version_id, ret);
- return ret;
- }
+ if (vmsd->pre_load_errp && !vmsd->pre_load_errp(opaque, errp)) {
+ error_prepend(errp, "pre load hook failed for: '%s', "
+ "version_id: %d, minimum version_id: %d, "
+ "ret: %d: ", vmsd->name, vmsd->version_id,
+ vmsd->minimum_version_id, ret);
+ return -EINVAL;
} else if (vmsd->pre_load) {
ret = vmsd->pre_load(opaque);
if (ret) {
@@ -255,13 +252,12 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
qemu_file_set_error(f, ret);
return ret;
}
- if (vmsd->post_load_errp) {
- ret = vmsd->post_load_errp(opaque, version_id, errp);
- if (ret < 0) {
- error_prepend(errp, "post load hook failed for: %s, version_id: "
- "%d, minimum_version: %d, ret: %d: ", vmsd->name,
- vmsd->version_id, vmsd->minimum_version_id, ret);
- }
+ if (vmsd->post_load_errp && !vmsd->post_load_errp(opaque, version_id,
+ errp)) {
+ error_prepend(errp, "post load hook failed for: %s, version_id: "
+ "%d, minimum_version: %d, ret: %d: ", vmsd->name,
+ vmsd->version_id, vmsd->minimum_version_id, ret);
+ ret = -EINVAL;
} else if (vmsd->post_load) {
ret = vmsd->post_load(opaque, version_id);
if (ret < 0) {
@@ -438,9 +434,8 @@ int vmstate_save_state_v(QEMUFile *f, const
VMStateDescription *vmsd,
trace_vmstate_save_state_top(vmsd->name);
if (vmsd->pre_save_errp) {
- ret = vmsd->pre_save_errp(opaque, errp);
trace_vmstate_save_state_pre_save_res(vmsd->name, ret);
- if (ret < 0) {
+ if (!vmsd->pre_save_errp(opaque, errp)) {
error_prepend(errp, "pre-save for %s failed, ret: %d: ",
vmsd->name, ret);
}
--
2.48.1
