Re: [PATCH v2 3/3] target/s390x/arch_dump: Add arch cleanup function for PV dumps

[Thomas Huth]

On 09/11/2023 13.04, Janosch Frank wrote:
> PV dumps block vcpu runs until dump end is reached. If there's an
> error between PV dump init and PV dump end the vm will never be able
> to run again. One example of such an error is insufficient disk space
> for the dump file.
>
> Let's add a cleanup function that tries to do a dump end. The dump
> completion data is discarded but there's no point in writing it to a
> file anyway if there's a possibility that other PV dump data is
> missing.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>   target/s390x/arch_dump.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
>
> diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
> index bdb0bfa0e7..7e8a1b4fc0 100644
> --- a/target/s390x/arch_dump.c
> +++ b/target/s390x/arch_dump.c
> @@ -433,6 +433,22 @@ static int arch_sections_write(DumpState *s, uint8_t *buff)
>       return 0;
>   }
>   
> +static void arch_cleanup(DumpState *s)
> +{
> +    g_autofree uint8_t *buff = NULL;
> +    int rc;
> +
> +    if (!pv_dump_initialized) {
> +        return;
> +    }

Maybe add a comment here why we still try the completion (e.g. something 
like the first paragraph of the commit description)

> +    buff = g_malloc(kvm_s390_pv_dmp_get_size_completion_data());
> +    rc = kvm_s390_dump_completion_data(buff);
> +    if (!rc) {
> +            pv_dump_initialized = false;
> +    }
> +}
> +
>   int cpu_get_dump_info(ArchDumpInfo *info,
>                         const struct GuestPhysBlockList *guest_phys_blocks)
>   {
> @@ -448,6 +464,7 @@ int cpu_get_dump_info(ArchDumpInfo *info,
>           info->arch_sections_add_fn = *arch_sections_add;
>           info->arch_sections_write_hdr_fn = *arch_sections_write_hdr;
>           info->arch_sections_write_fn = *arch_sections_write;
> +        info->arch_cleanup_fn = *arch_cleanup;
>       }
>       return 0;
>   }

Reviewed-by: Thomas Huth <thuth@redhat.com>