Vladimir Sementsov-Ogievskiy <[email protected]> writes: > We duplicate some of errors by trace points. That doesn't make > real sense: we report the error through errp, and stop migration, > no reason to duplicate the reported error by trace points, making > error path more complicated. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> > --- > migration/trace-events | 5 ++--- > migration/vmstate.c | 14 ++++++-------- > 2 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/migration/trace-events b/migration/trace-events > index 90629f828f..e864d19c55 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -55,15 +55,14 @@ postcopy_pause_incoming_continued(void) "" > postcopy_page_req_sync(void *host_addr) "sync page req %p" > > # vmstate.c > -vmstate_load_field_error(const char *field, int ret) "field \"%s\" load > failed, ret = %d" > vmstate_load_state(const char *name, int version_id) "%s v%d" > -vmstate_load_state_end(const char *name, const char *reason, int val) "%s > %s/%d" > +vmstate_load_state_end(const char *name) "%s" > vmstate_load_state_field(const char *name, const char *field, bool exists) > "%s:%s exists=%d" > vmstate_n_elems(const char *name, int n_elems) "%s: %d" > vmstate_subsection_load(const char *parent) "%s" > vmstate_subsection_load_bad(const char *parent, const char *sub, const char > *sub2) "%s: %s/%s" > vmstate_subsection_load_good(const char *parent) "%s" > -vmstate_save_state_pre_save_res(const char *name, int res) "%s/%d" > +vmstate_save_state_pre_save_done(const char *name) "%s" > vmstate_save_state_loop(const char *name, const char *field, int n_elems) > "%s/%s[%d]" > vmstate_save_state_top(const char *idstr) "%s" > vmstate_subsection_save_loop(const char *name, const char *sub) "%s/%s" > diff --git a/migration/vmstate.c b/migration/vmstate.c > index dd7cd27993..b07bbdd366 100644 > --- a/migration/vmstate.c > +++ b/migration/vmstate.c > @@ -144,7 +144,6 @@ int vmstate_load_state(QEMUFile *f, const > VMStateDescription *vmsd, > error_setg(errp, "%s: incoming version_id %d is too new " > "for local version_id %d", > vmsd->name, version_id, vmsd->version_id); > - trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL); > return -EINVAL; > } > > @@ -152,7 +151,6 @@ int vmstate_load_state(QEMUFile *f, const > VMStateDescription *vmsd, > error_setg(errp, "%s: incoming version_id %d is too old " > "for local minimum version_id %d", > vmsd->name, version_id, vmsd->minimum_version_id); > - trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL); > return -EINVAL; > } > > @@ -245,7 +243,6 @@ int vmstate_load_state(QEMUFile *f, const > VMStateDescription *vmsd, > > if (ret < 0) { > qemu_file_set_error(f, ret); > - trace_vmstate_load_field_error(field->name, ret); > return ret; > } > } > @@ -269,7 +266,7 @@ int vmstate_load_state(QEMUFile *f, const > VMStateDescription *vmsd, > error_prepend(errp, "post load hook failed for: %s, version_id: " > "%d, minimum_version: %d: ", vmsd->name, > vmsd->version_id, vmsd->minimum_version_id); > - ret = -EINVAL; > + return -EINVAL; > } > } else if (vmsd->post_load) { > ret = vmsd->post_load(opaque, version_id); > @@ -279,12 +276,13 @@ int vmstate_load_state(QEMUFile *f, const > VMStateDescription *vmsd, > "minimum_version: %d, ret: %d", > vmsd->name, vmsd->version_id, > vmsd->minimum_version_id, > ret); > + return ret; > } > } > > - trace_vmstate_load_state_end(vmsd->name, "end", ret); > + trace_vmstate_load_state_end(vmsd->name); > > - return ret; > + return 0; > } > > static int vmfield_name_num(const VMStateField *start, > @@ -444,20 +442,20 @@ static int vmstate_save_state_v(QEMUFile *f, const > VMStateDescription *vmsd, > > if (vmsd->pre_save_errp) { > ret = vmsd->pre_save_errp(opaque, errp) ? 0 : -EINVAL; > - trace_vmstate_save_state_pre_save_res(vmsd->name, ret); > if (ret < 0) { > error_prepend(errp, "pre-save for %s failed: ", vmsd->name); > return ret; > } > } else if (vmsd->pre_save) { > ret = vmsd->pre_save(opaque); > - trace_vmstate_save_state_pre_save_res(vmsd->name, ret); > if (ret) { > error_setg(errp, "pre-save failed: %s", vmsd->name); > return ret; > } > } > > + trace_vmstate_save_state_pre_save_done(vmsd->name); > + > if (vmdesc) { > json_writer_str(vmdesc, "vmsd_name", vmsd->name); > json_writer_int64(vmdesc, "version", version_id);
One benefit of the traces is to be able to match with source code when debugging a QEMU instance that cannot be rebuilt. Not so much about the trace itself, but to be able to know _where_ the code exited. However, I have no preference. Acked-by: Fabiano Rosas <[email protected]>
