diff mbox series

[4/4] target/s390x/arch_dump: Add arch cleanup function for PV dumps

Message ID 20231107142048.22422-5-frankja@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series dump: Arch info function pointer addition and cleanup | expand

Commit Message

Janosch Frank Nov. 7, 2023, 2:20 p.m. UTC
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(+)

Comments

Marc-André Lureau Nov. 8, 2023, 8:17 a.m. UTC | #1
Hi

On Tue, Nov 7, 2023 at 6:22 PM Janosch Frank <frankja@linux.ibm.com> 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..70146d7e84 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)
> +{
> +    uint8_t *buff = g_malloc(kvm_s390_pv_dmp_get_size_completion_data());
> +    int rc;
> +
> +    if (!pv_dump_initialized || !buff) {

this may leak if bluff != NULL && !pv_dump_initialized

Better use g_autofree.

No need to check bluff != NULL. (g_malloc abort() if it failed)

> +        return;
> +    }
> +
> +    rc = kvm_s390_dump_completion_data(buff);
> +    if (!rc) {
> +            pv_dump_initialized = false;
> +    }
> +    g_free(buff);
> +}
> +
>  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;
>  }
> --
> 2.34.1
>
>

otherwise, seems ok
diff mbox series

Patch

diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index bdb0bfa0e7..70146d7e84 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)
+{
+    uint8_t *buff = g_malloc(kvm_s390_pv_dmp_get_size_completion_data());
+    int rc;
+
+    if (!pv_dump_initialized || !buff) {
+        return;
+    }
+
+    rc = kvm_s390_dump_completion_data(buff);
+    if (!rc) {
+            pv_dump_initialized = false;
+    }
+    g_free(buff);
+}
+
 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;
 }