On 2025/07/31 22:21, Arun Menon wrote:
> The original vmstate_save_state_v() function combined multiple
> responsibilities like calling pre-save hooks, saving the state of
> the device, handling subsection saves and invoking post-save hooks.
> This led to a lengthy and less readable function.
>
> To address this, introduce wrapper functions for pre-save,
> post-save and the device-state save functionalities.
>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
> migration/vmstate.c | 89 ++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 64 insertions(+), 25 deletions(-)
>
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 25a819da069b982d4043f287b4562ea402d9eb0e..cec1ee2f30d6f0270ee1fd30d29f6f0f5d20bdb0 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -414,23 +414,43 @@ int vmstate_save_state_with_err(QEMUFile *f, const VMStateDescription *vmsd,
> return vmstate_save_state_v(f, vmsd, opaque, vmdesc_id, vmsd->version_id, errp);
> }
>
> -int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> - void *opaque, JSONWriter *vmdesc, int version_id, Error **errp)
> +static int pre_save_dispatch(const VMStateDescription *vmsd, void *opaque,
> + Error **errp)
> {
> int ret = 0;
> - int ps_ret = 0;
> - const VMStateField *field = vmsd->fields;
> -
> - trace_vmstate_save_state_top(vmsd->name);
> -
> 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;
> + error_setg(errp, "pre-save for %s failed, ret: '%d'",
> + vmsd->name, ret);
> }
> }
> + return ret;
> +}
> +
> +static int post_save_dispatch(const VMStateDescription *vmsd, void *opaque,
> + Error **errp)
> +{
> + int ret = 0;
> + if (vmsd->post_save) {
> + ret = vmsd->post_save(opaque);
> + error_setg(errp, "post-save for %s failed, ret: '%d'",
> + vmsd->name, ret);
> + }
> + return ret;
> +}
> +
> +static int vmstate_save_dispatch(QEMUFile *f,
> + const VMStateDescription *vmsd,
> + void *opaque, JSONWriter *vmdesc,
> + int version_id, Error **errp)
> +{
> + ERRP_GUARD();
> + int ret = 0;
> + int ps_ret = 0;
> + Error *local_err = NULL;
> + const VMStateField *field = vmsd->fields;
>
> if (vmdesc) {
> json_writer_str(vmdesc, "vmsd_name", vmsd->name);
> @@ -533,15 +553,11 @@ 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) {
> - ps_ret = vmsd->post_save(opaque);
> - if (ps_ret) {
> - ret = ps_ret;
> - error_free_or_abort(errp);
> - error_setg(errp,
> - "post-save for %s failed, ret: '%d'",
> - vmsd->name, ret);
Putting this patch before the last one will make the addition and
removal of this error_setg() unnecessary and make the patches smaller.
> - }
> + ps_ret = post_save_dispatch(vmsd, opaque, &local_err);
> + if (ps_ret) {
> + ret = ps_ret;
> + error_free_or_abort(errp);
> + error_propagate(errp, local_err);
> }
> return ret;
> }
> @@ -565,17 +581,40 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> if (vmdesc) {
> json_writer_end_array(vmdesc);
> }
> + return ret;
> +}
>
> - ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
>
> - if (vmsd->post_save) {
> - ps_ret = vmsd->post_save(opaque);
> - if (ps_ret) {
> - ret = ps_ret;
> +int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
> + void *opaque, JSONWriter *vmdesc, int version_id,
> + Error **errp)
> +{
> + ERRP_GUARD();
> + int ret = 0;
> + Error *local_err = NULL;
> + int ps_ret = 0;
> +
> + trace_vmstate_save_state_top(vmsd->name);
> +
> + ret = pre_save_dispatch(vmsd, opaque, errp);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = vmstate_save_dispatch(f, vmsd, opaque, vmdesc,
> + version_id, errp);
> + if (ret) {
> + return ret;
> + }
> +
> + ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
> + ps_ret = post_save_dispatch(vmsd, opaque, &local_err);
> + if (ps_ret) {
> + if (ret) {
> error_free_or_abort(errp);
> - error_setg(errp, "post-save for %s failed, ret: '%d'",
> - vmsd->name, ret);
> }
> + ret = ps_ret;
> + error_propagate(errp, local_err);
> }
> return ret;
> }
>