On Thu, Jul 17, 2025 at 06:07:45AM +0530, Arun Menon wrote:
> - We need to have good error reporting in the callbacks in
> VMStateDescription struct. Specifically pre_save, post_save,
> pre_load and post_load callbacks.
> - It is not possible to change these functions everywhere in one
> patch, therefore, we introduce a duplicate set of callbacks
> with Error object passed to them.
> - So, in this commit, we implement 'errp' variants of these callbacks,
> introducing an explicit Error object parameter.
> - This is a functional step towards transitioning the entire codebase
> to the new error-parameterized functions.
> - Deliberately called in mutual exclusion from their counterparts,
> to prevent conflicts during the transition.
> - New impls should preferentally use 'errp' variants of
> these methods, and existing impls incrementally converted.
> The variants without 'errp' are intended to be removed
> once all usage is converted.
>
> Signed-off-by: Arun Menon <arme...@redhat.com>
> ---
> include/migration/vmstate.h | 11 +++++++++++
> migration/vmstate.c | 36 +++++++++++++++++++++++++++++++-----
> 2 files changed, 42 insertions(+), 5 deletions(-)
> @@ -524,7 +542,9 @@ int vmstate_save_state_v(QEMUFile *f, const
> VMStateDescription *vmsd,
> if (ret) {
> error_setg(errp, "Save of field %s/%s failed",
> vmsd->name, field->name);
> - if (vmsd->post_save) {
> + if (vmsd->post_save_errp) {
> + vmsd->post_save_errp(opaque, errp);
We are already in an error path here, and we set 'errp' just a few lines
above. If post_save_errp tries to set it too, then we're doomed.
You'll need to pass an &Local_err in here, and then append it to the
current error.
> + } else if (vmsd->post_save) {
> vmsd->post_save(opaque);
> }
> return ret;
> @@ -552,7 +572,13 @@ int vmstate_save_state_v(QEMUFile *f, const
> VMStateDescription *vmsd,
>
> ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
Potentially set 'errp'
> - if (vmsd->post_save) {
> + if (vmsd->post_save_errp) {
> + int ps_ret = vmsd->post_save_errp(opaque, errp);
so must not be passing 'errp' here.
> + if (!ret && ps_ret) {
> + ret = ps_ret;
> + error_prepend(errp, "post-save failed: %s ", vmsd->name);
> + }
> + } else if (vmsd->post_save) {
> int ps_ret = vmsd->post_save(opaque);
> if (!ret && ps_ret) {
> ret = ps_ret;
>
> --
> 2.50.0
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
