| Message ID | 20220330123603.107120-2-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dump: Cleanup and consolidation | expand |
Hi On Wed, Mar 30, 2022 at 4:42 PM Janosch Frank <frankja@linux.ibm.com> wrote: > Let's move to the new way of handling errors before changing the dump > code. This patch has mostly been generated by the coccinelle script > scripts/coccinelle/errp-guard.cocci. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > nice Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> While you are at it, would you add a patch (at the end of this series, to avoid the churn) to return a bool for success/failure (as recommended in qapi/error.h)? thanks > --- > dump/dump.c | 144 ++++++++++++++++++++++------------------------------ > 1 file changed, 61 insertions(+), 83 deletions(-) > > diff --git a/dump/dump.c b/dump/dump.c > index f57ed76fa7..58c4923fce 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -390,23 +390,21 @@ static void write_data(DumpState *s, void *buf, int > length, Error **errp) > static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t > start, > int64_t size, Error **errp) > { > + ERRP_GUARD(); > int64_t i; > - Error *local_err = NULL; > > for (i = 0; i < size / s->dump_info.page_size; i++) { > write_data(s, block->host_addr + start + i * > s->dump_info.page_size, > - s->dump_info.page_size, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + s->dump_info.page_size, errp); > + if (*errp) { > return; > } > } > > if ((size % s->dump_info.page_size) != 0) { > write_data(s, block->host_addr + start + i * > s->dump_info.page_size, > - size % s->dump_info.page_size, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + size % s->dump_info.page_size, errp); > + if (*errp) { > return; > } > } > @@ -476,11 +474,11 @@ static void get_offset_range(hwaddr phys_addr, > > static void write_elf_loads(DumpState *s, Error **errp) > { > + ERRP_GUARD(); > hwaddr offset, filesz; > MemoryMapping *memory_mapping; > uint32_t phdr_index = 1; > uint32_t max_index; > - Error *local_err = NULL; > > if (s->have_section) { > max_index = s->sh_info; > @@ -494,14 +492,13 @@ static void write_elf_loads(DumpState *s, Error > **errp) > s, &offset, &filesz); > if (s->dump_info.d_class == ELFCLASS64) { > write_elf64_load(s, memory_mapping, phdr_index++, offset, > - filesz, &local_err); > + filesz, errp); > } else { > write_elf32_load(s, memory_mapping, phdr_index++, offset, > - filesz, &local_err); > + filesz, errp); > } > > - if (local_err) { > - error_propagate(errp, local_err); > + if (*errp) { > return; > } > > @@ -514,7 +511,7 @@ static void write_elf_loads(DumpState *s, Error **errp) > /* write elf header, PT_NOTE and elf note to vmcore. */ > static void dump_begin(DumpState *s, Error **errp) > { > - Error *local_err = NULL; > + ERRP_GUARD(); > > /* > * the vmcore's format is: > @@ -542,73 +539,64 @@ static void dump_begin(DumpState *s, Error **errp) > > /* write elf header to vmcore */ > if (s->dump_info.d_class == ELFCLASS64) { > - write_elf64_header(s, &local_err); > + write_elf64_header(s, errp); > } else { > - write_elf32_header(s, &local_err); > + write_elf32_header(s, errp); > } > - if (local_err) { > - error_propagate(errp, local_err); > + if (*errp) { > return; > } > > if (s->dump_info.d_class == ELFCLASS64) { > /* write PT_NOTE to vmcore */ > - write_elf64_note(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf64_note(s, errp); > + if (*errp) { > return; > } > > /* write all PT_LOAD to vmcore */ > - write_elf_loads(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf_loads(s, errp); > + if (*errp) { > return; > } > > /* write section to vmcore */ > if (s->have_section) { > - write_elf_section(s, 1, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf_section(s, 1, errp); > + if (*errp) { > return; > } > } > > /* write notes to vmcore */ > - write_elf64_notes(fd_write_vmcore, s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf64_notes(fd_write_vmcore, s, errp); > + if (*errp) { > return; > } > } else { > /* write PT_NOTE to vmcore */ > - write_elf32_note(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf32_note(s, errp); > + if (*errp) { > return; > } > > /* write all PT_LOAD to vmcore */ > - write_elf_loads(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf_loads(s, errp); > + if (*errp) { > return; > } > > /* write section to vmcore */ > if (s->have_section) { > - write_elf_section(s, 0, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf_section(s, 0, errp); > + if (*errp) { > return; > } > } > > /* write notes to vmcore */ > - write_elf32_notes(fd_write_vmcore, s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf32_notes(fd_write_vmcore, s, errp); > + if (*errp) { > return; > } > } > @@ -644,9 +632,9 @@ static int get_next_block(DumpState *s, GuestPhysBlock > *block) > /* write all memory to vmcore */ > static void dump_iterate(DumpState *s, Error **errp) > { > + ERRP_GUARD(); > GuestPhysBlock *block; > int64_t size; > - Error *local_err = NULL; > > do { > block = s->next_block; > @@ -658,9 +646,8 @@ static void dump_iterate(DumpState *s, Error **errp) > size -= block->target_end - (s->begin + s->length); > } > } > - write_memory(s, block, s->start, size, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_memory(s, block, s->start, size, errp); > + if (*errp) { > return; > } > > @@ -669,11 +656,10 @@ static void dump_iterate(DumpState *s, Error **errp) > > static void create_vmcore(DumpState *s, Error **errp) > { > - Error *local_err = NULL; > + ERRP_GUARD(); > > - dump_begin(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + dump_begin(s, errp); > + if (*errp) { > return; > } > > @@ -810,6 +796,7 @@ static bool note_name_equal(DumpState *s, > /* write common header, sub header and elf note to vmcore */ > static void create_header32(DumpState *s, Error **errp) > { > + ERRP_GUARD(); > DiskDumpHeader32 *dh = NULL; > KdumpSubHeader32 *kh = NULL; > size_t size; > @@ -818,7 +805,6 @@ static void create_header32(DumpState *s, Error **errp) > uint32_t bitmap_blocks; > uint32_t status = 0; > uint64_t offset_note; > - Error *local_err = NULL; > > /* write common header, the version of kdump-compressed format is 6th > */ > size = sizeof(DiskDumpHeader32); > @@ -894,9 +880,8 @@ static void create_header32(DumpState *s, Error **errp) > s->note_buf_offset = 0; > > /* use s->note_buf to store notes temporarily */ > - write_elf32_notes(buf_write_note, s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf32_notes(buf_write_note, s, errp); > + if (*errp) { > goto out; > } > if (write_buffer(s->fd, offset_note, s->note_buf, > @@ -922,6 +907,7 @@ out: > /* write common header, sub header and elf note to vmcore */ > static void create_header64(DumpState *s, Error **errp) > { > + ERRP_GUARD(); > DiskDumpHeader64 *dh = NULL; > KdumpSubHeader64 *kh = NULL; > size_t size; > @@ -930,7 +916,6 @@ static void create_header64(DumpState *s, Error **errp) > uint32_t bitmap_blocks; > uint32_t status = 0; > uint64_t offset_note; > - Error *local_err = NULL; > > /* write common header, the version of kdump-compressed format is 6th > */ > size = sizeof(DiskDumpHeader64); > @@ -1006,9 +991,8 @@ static void create_header64(DumpState *s, Error > **errp) > s->note_buf_offset = 0; > > /* use s->note_buf to store notes temporarily */ > - write_elf64_notes(buf_write_note, s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_elf64_notes(buf_write_note, s, errp); > + if (*errp) { > goto out; > } > > @@ -1464,8 +1448,8 @@ out: > > static void create_kdump_vmcore(DumpState *s, Error **errp) > { > + ERRP_GUARD(); > int ret; > - Error *local_err = NULL; > > /* > * the kdump-compressed format is: > @@ -1495,21 +1479,18 @@ static void create_kdump_vmcore(DumpState *s, > Error **errp) > return; > } > > - write_dump_header(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_dump_header(s, errp); > + if (*errp) { > return; > } > > - write_dump_bitmap(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_dump_bitmap(s, errp); > + if (*errp) { > return; > } > > - write_dump_pages(s, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + write_dump_pages(s, errp); > + if (*errp) { > return; > } > > @@ -1639,10 +1620,10 @@ static void dump_init(DumpState *s, int fd, bool > has_format, > DumpGuestMemoryFormat format, bool paging, bool > has_filter, > int64_t begin, int64_t length, Error **errp) > { > + ERRP_GUARD(); > VMCoreInfoState *vmci = vmcoreinfo_find(); > CPUState *cpu; > int nr_cpus; > - Error *err = NULL; > int ret; > > s->has_format = has_format; > @@ -1761,9 +1742,8 @@ static void dump_init(DumpState *s, int fd, bool > has_format, > > /* get memory mapping */ > if (paging) { > - qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, > &err); > - if (err != NULL) { > - error_propagate(errp, err); > + qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, > errp); > + if (*errp) { > goto cleanup; > } > } else { > @@ -1862,33 +1842,32 @@ cleanup: > /* this operation might be time consuming. */ > static void dump_process(DumpState *s, Error **errp) > { > - Error *local_err = NULL; > + ERRP_GUARD(); > DumpQueryResult *result = NULL; > > if (s->has_format && s->format == DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) { > #ifdef TARGET_X86_64 > - create_win_dump(s, &local_err); > + create_win_dump(s, errp); > #endif > } else if (s->has_format && s->format != > DUMP_GUEST_MEMORY_FORMAT_ELF) { > - create_kdump_vmcore(s, &local_err); > + create_kdump_vmcore(s, errp); > } else { > - create_vmcore(s, &local_err); > + create_vmcore(s, errp); > } > > /* make sure status is written after written_size updates */ > smp_wmb(); > qatomic_set(&s->status, > - (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); > + (*errp ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); > > /* send DUMP_COMPLETED message (unconditionally) */ > result = qmp_query_dump(NULL); > /* should never fail */ > assert(result); > - qapi_event_send_dump_completed(result, !!local_err, (local_err ? > - error_get_pretty(local_err) : NULL)); > + qapi_event_send_dump_completed(result, !!*errp, (*errp ? > + > error_get_pretty(*errp) : NULL)); > qapi_free_DumpQueryResult(result); > > - error_propagate(errp, local_err); > dump_cleanup(s); > } > > @@ -1917,10 +1896,10 @@ void qmp_dump_guest_memory(bool paging, const char > *file, > int64_t length, bool has_format, > DumpGuestMemoryFormat format, Error **errp) > { > + ERRP_GUARD(); > const char *p; > int fd = -1; > DumpState *s; > - Error *local_err = NULL; > bool detach_p = false; > > if (runstate_check(RUN_STATE_INMIGRATE)) { > @@ -2020,9 +1999,8 @@ void qmp_dump_guest_memory(bool paging, const char > *file, > dump_state_prepare(s); > > dump_init(s, fd, has_format, format, paging, has_begin, > - begin, length, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > + begin, length, errp); > + if (*errp) { > qatomic_set(&s->status, DUMP_STATUS_FAILED); > return; > } > -- > 2.32.0 > > >
On 3/31/22 10:59, Marc-André Lureau wrote: > Hi > > On Wed, Mar 30, 2022 at 4:42 PM Janosch Frank <frankja@linux.ibm.com> wrote: > >> Let's move to the new way of handling errors before changing the dump >> code. This patch has mostly been generated by the coccinelle script >> scripts/coccinelle/errp-guard.cocci. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> > > nice > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Thanks! > > While you are at it, would you add a patch (at the end of this series, to > avoid the churn) to return a bool for success/failure (as recommended in > qapi/error.h)? I knew it was a mistake to touch this file :) Sure, it will be churn anyway since I have two more patch sets that follow on this one. > > thanks > > >> --- >> dump/dump.c | 144 ++++++++++++++++++++++------------------------------ >> 1 file changed, 61 insertions(+), 83 deletions(-) >> >> diff --git a/dump/dump.c b/dump/dump.c >> index f57ed76fa7..58c4923fce 100644 >> --- a/dump/dump.c >> +++ b/dump/dump.c >> @@ -390,23 +390,21 @@ static void write_data(DumpState *s, void *buf, int >> length, Error **errp) >> static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t >> start, >> int64_t size, Error **errp) >> { >> + ERRP_GUARD(); >> int64_t i; >> - Error *local_err = NULL; >> >> for (i = 0; i < size / s->dump_info.page_size; i++) { >> write_data(s, block->host_addr + start + i * >> s->dump_info.page_size, >> - s->dump_info.page_size, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + s->dump_info.page_size, errp); >> + if (*errp) { >> return; >> } >> } >> >> if ((size % s->dump_info.page_size) != 0) { >> write_data(s, block->host_addr + start + i * >> s->dump_info.page_size, >> - size % s->dump_info.page_size, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + size % s->dump_info.page_size, errp); >> + if (*errp) { >> return; >> } >> } >> @@ -476,11 +474,11 @@ static void get_offset_range(hwaddr phys_addr, >> >> static void write_elf_loads(DumpState *s, Error **errp) >> { >> + ERRP_GUARD(); >> hwaddr offset, filesz; >> MemoryMapping *memory_mapping; >> uint32_t phdr_index = 1; >> uint32_t max_index; >> - Error *local_err = NULL; >> >> if (s->have_section) { >> max_index = s->sh_info; >> @@ -494,14 +492,13 @@ static void write_elf_loads(DumpState *s, Error >> **errp) >> s, &offset, &filesz); >> if (s->dump_info.d_class == ELFCLASS64) { >> write_elf64_load(s, memory_mapping, phdr_index++, offset, >> - filesz, &local_err); >> + filesz, errp); >> } else { >> write_elf32_load(s, memory_mapping, phdr_index++, offset, >> - filesz, &local_err); >> + filesz, errp); >> } >> >> - if (local_err) { >> - error_propagate(errp, local_err); >> + if (*errp) { >> return; >> } >> >> @@ -514,7 +511,7 @@ static void write_elf_loads(DumpState *s, Error **errp) >> /* write elf header, PT_NOTE and elf note to vmcore. */ >> static void dump_begin(DumpState *s, Error **errp) >> { >> - Error *local_err = NULL; >> + ERRP_GUARD(); >> >> /* >> * the vmcore's format is: >> @@ -542,73 +539,64 @@ static void dump_begin(DumpState *s, Error **errp) >> >> /* write elf header to vmcore */ >> if (s->dump_info.d_class == ELFCLASS64) { >> - write_elf64_header(s, &local_err); >> + write_elf64_header(s, errp); >> } else { >> - write_elf32_header(s, &local_err); >> + write_elf32_header(s, errp); >> } >> - if (local_err) { >> - error_propagate(errp, local_err); >> + if (*errp) { >> return; >> } >> >> if (s->dump_info.d_class == ELFCLASS64) { >> /* write PT_NOTE to vmcore */ >> - write_elf64_note(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf64_note(s, errp); >> + if (*errp) { >> return; >> } >> >> /* write all PT_LOAD to vmcore */ >> - write_elf_loads(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf_loads(s, errp); >> + if (*errp) { >> return; >> } >> >> /* write section to vmcore */ >> if (s->have_section) { >> - write_elf_section(s, 1, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf_section(s, 1, errp); >> + if (*errp) { >> return; >> } >> } >> >> /* write notes to vmcore */ >> - write_elf64_notes(fd_write_vmcore, s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf64_notes(fd_write_vmcore, s, errp); >> + if (*errp) { >> return; >> } >> } else { >> /* write PT_NOTE to vmcore */ >> - write_elf32_note(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf32_note(s, errp); >> + if (*errp) { >> return; >> } >> >> /* write all PT_LOAD to vmcore */ >> - write_elf_loads(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf_loads(s, errp); >> + if (*errp) { >> return; >> } >> >> /* write section to vmcore */ >> if (s->have_section) { >> - write_elf_section(s, 0, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf_section(s, 0, errp); >> + if (*errp) { >> return; >> } >> } >> >> /* write notes to vmcore */ >> - write_elf32_notes(fd_write_vmcore, s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf32_notes(fd_write_vmcore, s, errp); >> + if (*errp) { >> return; >> } >> } >> @@ -644,9 +632,9 @@ static int get_next_block(DumpState *s, GuestPhysBlock >> *block) >> /* write all memory to vmcore */ >> static void dump_iterate(DumpState *s, Error **errp) >> { >> + ERRP_GUARD(); >> GuestPhysBlock *block; >> int64_t size; >> - Error *local_err = NULL; >> >> do { >> block = s->next_block; >> @@ -658,9 +646,8 @@ static void dump_iterate(DumpState *s, Error **errp) >> size -= block->target_end - (s->begin + s->length); >> } >> } >> - write_memory(s, block, s->start, size, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_memory(s, block, s->start, size, errp); >> + if (*errp) { >> return; >> } >> >> @@ -669,11 +656,10 @@ static void dump_iterate(DumpState *s, Error **errp) >> >> static void create_vmcore(DumpState *s, Error **errp) >> { >> - Error *local_err = NULL; >> + ERRP_GUARD(); >> >> - dump_begin(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + dump_begin(s, errp); >> + if (*errp) { >> return; >> } >> >> @@ -810,6 +796,7 @@ static bool note_name_equal(DumpState *s, >> /* write common header, sub header and elf note to vmcore */ >> static void create_header32(DumpState *s, Error **errp) >> { >> + ERRP_GUARD(); >> DiskDumpHeader32 *dh = NULL; >> KdumpSubHeader32 *kh = NULL; >> size_t size; >> @@ -818,7 +805,6 @@ static void create_header32(DumpState *s, Error **errp) >> uint32_t bitmap_blocks; >> uint32_t status = 0; >> uint64_t offset_note; >> - Error *local_err = NULL; >> >> /* write common header, the version of kdump-compressed format is 6th >> */ >> size = sizeof(DiskDumpHeader32); >> @@ -894,9 +880,8 @@ static void create_header32(DumpState *s, Error **errp) >> s->note_buf_offset = 0; >> >> /* use s->note_buf to store notes temporarily */ >> - write_elf32_notes(buf_write_note, s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf32_notes(buf_write_note, s, errp); >> + if (*errp) { >> goto out; >> } >> if (write_buffer(s->fd, offset_note, s->note_buf, >> @@ -922,6 +907,7 @@ out: >> /* write common header, sub header and elf note to vmcore */ >> static void create_header64(DumpState *s, Error **errp) >> { >> + ERRP_GUARD(); >> DiskDumpHeader64 *dh = NULL; >> KdumpSubHeader64 *kh = NULL; >> size_t size; >> @@ -930,7 +916,6 @@ static void create_header64(DumpState *s, Error **errp) >> uint32_t bitmap_blocks; >> uint32_t status = 0; >> uint64_t offset_note; >> - Error *local_err = NULL; >> >> /* write common header, the version of kdump-compressed format is 6th >> */ >> size = sizeof(DiskDumpHeader64); >> @@ -1006,9 +991,8 @@ static void create_header64(DumpState *s, Error >> **errp) >> s->note_buf_offset = 0; >> >> /* use s->note_buf to store notes temporarily */ >> - write_elf64_notes(buf_write_note, s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_elf64_notes(buf_write_note, s, errp); >> + if (*errp) { >> goto out; >> } >> >> @@ -1464,8 +1448,8 @@ out: >> >> static void create_kdump_vmcore(DumpState *s, Error **errp) >> { >> + ERRP_GUARD(); >> int ret; >> - Error *local_err = NULL; >> >> /* >> * the kdump-compressed format is: >> @@ -1495,21 +1479,18 @@ static void create_kdump_vmcore(DumpState *s, >> Error **errp) >> return; >> } >> >> - write_dump_header(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_dump_header(s, errp); >> + if (*errp) { >> return; >> } >> >> - write_dump_bitmap(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_dump_bitmap(s, errp); >> + if (*errp) { >> return; >> } >> >> - write_dump_pages(s, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + write_dump_pages(s, errp); >> + if (*errp) { >> return; >> } >> >> @@ -1639,10 +1620,10 @@ static void dump_init(DumpState *s, int fd, bool >> has_format, >> DumpGuestMemoryFormat format, bool paging, bool >> has_filter, >> int64_t begin, int64_t length, Error **errp) >> { >> + ERRP_GUARD(); >> VMCoreInfoState *vmci = vmcoreinfo_find(); >> CPUState *cpu; >> int nr_cpus; >> - Error *err = NULL; >> int ret; >> >> s->has_format = has_format; >> @@ -1761,9 +1742,8 @@ static void dump_init(DumpState *s, int fd, bool >> has_format, >> >> /* get memory mapping */ >> if (paging) { >> - qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, >> &err); >> - if (err != NULL) { >> - error_propagate(errp, err); >> + qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, >> errp); >> + if (*errp) { >> goto cleanup; >> } >> } else { >> @@ -1862,33 +1842,32 @@ cleanup: >> /* this operation might be time consuming. */ >> static void dump_process(DumpState *s, Error **errp) >> { >> - Error *local_err = NULL; >> + ERRP_GUARD(); >> DumpQueryResult *result = NULL; >> >> if (s->has_format && s->format == DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) { >> #ifdef TARGET_X86_64 >> - create_win_dump(s, &local_err); >> + create_win_dump(s, errp); >> #endif >> } else if (s->has_format && s->format != >> DUMP_GUEST_MEMORY_FORMAT_ELF) { >> - create_kdump_vmcore(s, &local_err); >> + create_kdump_vmcore(s, errp); >> } else { >> - create_vmcore(s, &local_err); >> + create_vmcore(s, errp); >> } >> >> /* make sure status is written after written_size updates */ >> smp_wmb(); >> qatomic_set(&s->status, >> - (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); >> + (*errp ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); >> >> /* send DUMP_COMPLETED message (unconditionally) */ >> result = qmp_query_dump(NULL); >> /* should never fail */ >> assert(result); >> - qapi_event_send_dump_completed(result, !!local_err, (local_err ? >> - error_get_pretty(local_err) : NULL)); >> + qapi_event_send_dump_completed(result, !!*errp, (*errp ? >> + >> error_get_pretty(*errp) : NULL)); >> qapi_free_DumpQueryResult(result); >> >> - error_propagate(errp, local_err); >> dump_cleanup(s); >> } >> >> @@ -1917,10 +1896,10 @@ void qmp_dump_guest_memory(bool paging, const char >> *file, >> int64_t length, bool has_format, >> DumpGuestMemoryFormat format, Error **errp) >> { >> + ERRP_GUARD(); >> const char *p; >> int fd = -1; >> DumpState *s; >> - Error *local_err = NULL; >> bool detach_p = false; >> >> if (runstate_check(RUN_STATE_INMIGRATE)) { >> @@ -2020,9 +1999,8 @@ void qmp_dump_guest_memory(bool paging, const char >> *file, >> dump_state_prepare(s); >> >> dump_init(s, fd, has_format, format, paging, has_begin, >> - begin, length, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> + begin, length, errp); >> + if (*errp) { >> qatomic_set(&s->status, DUMP_STATUS_FAILED); >> return; >> } >> -- >> 2.32.0 >> >> >> >
On Thu, Mar 31, 2022 at 1:48 PM Janosch Frank <frankja@linux.ibm.com> wrote: > On 3/31/22 10:59, Marc-André Lureau wrote: > > Hi > > > > On Wed, Mar 30, 2022 at 4:42 PM Janosch Frank <frankja@linux.ibm.com> > wrote: > > > >> Let's move to the new way of handling errors before changing the dump > >> code. This patch has mostly been generated by the coccinelle script > >> scripts/coccinelle/errp-guard.cocci. > >> > >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > >> > > > > nice > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Thanks! > > > > > While you are at it, would you add a patch (at the end of this series, to > > avoid the churn) to return a bool for success/failure (as recommended in > > qapi/error.h)? > > I knew it was a mistake to touch this file :) > > Sure, it will be churn anyway since I have two more patch sets that > follow on this one. > > Feel free to leave that cleanup for now thanks > > > > thanks > > > > > >> --- > >> dump/dump.c | 144 ++++++++++++++++++++++------------------------------ > >> 1 file changed, 61 insertions(+), 83 deletions(-) > >> > >> diff --git a/dump/dump.c b/dump/dump.c > >> index f57ed76fa7..58c4923fce 100644 > >> --- a/dump/dump.c > >> +++ b/dump/dump.c > >> @@ -390,23 +390,21 @@ static void write_data(DumpState *s, void *buf, > int > >> length, Error **errp) > >> static void write_memory(DumpState *s, GuestPhysBlock *block, > ram_addr_t > >> start, > >> int64_t size, Error **errp) > >> { > >> + ERRP_GUARD(); > >> int64_t i; > >> - Error *local_err = NULL; > >> > >> for (i = 0; i < size / s->dump_info.page_size; i++) { > >> write_data(s, block->host_addr + start + i * > >> s->dump_info.page_size, > >> - s->dump_info.page_size, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + s->dump_info.page_size, errp); > >> + if (*errp) { > >> return; > >> } > >> } > >> > >> if ((size % s->dump_info.page_size) != 0) { > >> write_data(s, block->host_addr + start + i * > >> s->dump_info.page_size, > >> - size % s->dump_info.page_size, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + size % s->dump_info.page_size, errp); > >> + if (*errp) { > >> return; > >> } > >> } > >> @@ -476,11 +474,11 @@ static void get_offset_range(hwaddr phys_addr, > >> > >> static void write_elf_loads(DumpState *s, Error **errp) > >> { > >> + ERRP_GUARD(); > >> hwaddr offset, filesz; > >> MemoryMapping *memory_mapping; > >> uint32_t phdr_index = 1; > >> uint32_t max_index; > >> - Error *local_err = NULL; > >> > >> if (s->have_section) { > >> max_index = s->sh_info; > >> @@ -494,14 +492,13 @@ static void write_elf_loads(DumpState *s, Error > >> **errp) > >> s, &offset, &filesz); > >> if (s->dump_info.d_class == ELFCLASS64) { > >> write_elf64_load(s, memory_mapping, phdr_index++, offset, > >> - filesz, &local_err); > >> + filesz, errp); > >> } else { > >> write_elf32_load(s, memory_mapping, phdr_index++, offset, > >> - filesz, &local_err); > >> + filesz, errp); > >> } > >> > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + if (*errp) { > >> return; > >> } > >> > >> @@ -514,7 +511,7 @@ static void write_elf_loads(DumpState *s, Error > **errp) > >> /* write elf header, PT_NOTE and elf note to vmcore. */ > >> static void dump_begin(DumpState *s, Error **errp) > >> { > >> - Error *local_err = NULL; > >> + ERRP_GUARD(); > >> > >> /* > >> * the vmcore's format is: > >> @@ -542,73 +539,64 @@ static void dump_begin(DumpState *s, Error **errp) > >> > >> /* write elf header to vmcore */ > >> if (s->dump_info.d_class == ELFCLASS64) { > >> - write_elf64_header(s, &local_err); > >> + write_elf64_header(s, errp); > >> } else { > >> - write_elf32_header(s, &local_err); > >> + write_elf32_header(s, errp); > >> } > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + if (*errp) { > >> return; > >> } > >> > >> if (s->dump_info.d_class == ELFCLASS64) { > >> /* write PT_NOTE to vmcore */ > >> - write_elf64_note(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf64_note(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> /* write all PT_LOAD to vmcore */ > >> - write_elf_loads(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf_loads(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> /* write section to vmcore */ > >> if (s->have_section) { > >> - write_elf_section(s, 1, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf_section(s, 1, errp); > >> + if (*errp) { > >> return; > >> } > >> } > >> > >> /* write notes to vmcore */ > >> - write_elf64_notes(fd_write_vmcore, s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf64_notes(fd_write_vmcore, s, errp); > >> + if (*errp) { > >> return; > >> } > >> } else { > >> /* write PT_NOTE to vmcore */ > >> - write_elf32_note(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf32_note(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> /* write all PT_LOAD to vmcore */ > >> - write_elf_loads(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf_loads(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> /* write section to vmcore */ > >> if (s->have_section) { > >> - write_elf_section(s, 0, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf_section(s, 0, errp); > >> + if (*errp) { > >> return; > >> } > >> } > >> > >> /* write notes to vmcore */ > >> - write_elf32_notes(fd_write_vmcore, s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf32_notes(fd_write_vmcore, s, errp); > >> + if (*errp) { > >> return; > >> } > >> } > >> @@ -644,9 +632,9 @@ static int get_next_block(DumpState *s, > GuestPhysBlock > >> *block) > >> /* write all memory to vmcore */ > >> static void dump_iterate(DumpState *s, Error **errp) > >> { > >> + ERRP_GUARD(); > >> GuestPhysBlock *block; > >> int64_t size; > >> - Error *local_err = NULL; > >> > >> do { > >> block = s->next_block; > >> @@ -658,9 +646,8 @@ static void dump_iterate(DumpState *s, Error **errp) > >> size -= block->target_end - (s->begin + s->length); > >> } > >> } > >> - write_memory(s, block, s->start, size, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_memory(s, block, s->start, size, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> @@ -669,11 +656,10 @@ static void dump_iterate(DumpState *s, Error > **errp) > >> > >> static void create_vmcore(DumpState *s, Error **errp) > >> { > >> - Error *local_err = NULL; > >> + ERRP_GUARD(); > >> > >> - dump_begin(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + dump_begin(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> @@ -810,6 +796,7 @@ static bool note_name_equal(DumpState *s, > >> /* write common header, sub header and elf note to vmcore */ > >> static void create_header32(DumpState *s, Error **errp) > >> { > >> + ERRP_GUARD(); > >> DiskDumpHeader32 *dh = NULL; > >> KdumpSubHeader32 *kh = NULL; > >> size_t size; > >> @@ -818,7 +805,6 @@ static void create_header32(DumpState *s, Error > **errp) > >> uint32_t bitmap_blocks; > >> uint32_t status = 0; > >> uint64_t offset_note; > >> - Error *local_err = NULL; > >> > >> /* write common header, the version of kdump-compressed format is > 6th > >> */ > >> size = sizeof(DiskDumpHeader32); > >> @@ -894,9 +880,8 @@ static void create_header32(DumpState *s, Error > **errp) > >> s->note_buf_offset = 0; > >> > >> /* use s->note_buf to store notes temporarily */ > >> - write_elf32_notes(buf_write_note, s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf32_notes(buf_write_note, s, errp); > >> + if (*errp) { > >> goto out; > >> } > >> if (write_buffer(s->fd, offset_note, s->note_buf, > >> @@ -922,6 +907,7 @@ out: > >> /* write common header, sub header and elf note to vmcore */ > >> static void create_header64(DumpState *s, Error **errp) > >> { > >> + ERRP_GUARD(); > >> DiskDumpHeader64 *dh = NULL; > >> KdumpSubHeader64 *kh = NULL; > >> size_t size; > >> @@ -930,7 +916,6 @@ static void create_header64(DumpState *s, Error > **errp) > >> uint32_t bitmap_blocks; > >> uint32_t status = 0; > >> uint64_t offset_note; > >> - Error *local_err = NULL; > >> > >> /* write common header, the version of kdump-compressed format is > 6th > >> */ > >> size = sizeof(DiskDumpHeader64); > >> @@ -1006,9 +991,8 @@ static void create_header64(DumpState *s, Error > >> **errp) > >> s->note_buf_offset = 0; > >> > >> /* use s->note_buf to store notes temporarily */ > >> - write_elf64_notes(buf_write_note, s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_elf64_notes(buf_write_note, s, errp); > >> + if (*errp) { > >> goto out; > >> } > >> > >> @@ -1464,8 +1448,8 @@ out: > >> > >> static void create_kdump_vmcore(DumpState *s, Error **errp) > >> { > >> + ERRP_GUARD(); > >> int ret; > >> - Error *local_err = NULL; > >> > >> /* > >> * the kdump-compressed format is: > >> @@ -1495,21 +1479,18 @@ static void create_kdump_vmcore(DumpState *s, > >> Error **errp) > >> return; > >> } > >> > >> - write_dump_header(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_dump_header(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> - write_dump_bitmap(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_dump_bitmap(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> - write_dump_pages(s, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + write_dump_pages(s, errp); > >> + if (*errp) { > >> return; > >> } > >> > >> @@ -1639,10 +1620,10 @@ static void dump_init(DumpState *s, int fd, bool > >> has_format, > >> DumpGuestMemoryFormat format, bool paging, bool > >> has_filter, > >> int64_t begin, int64_t length, Error **errp) > >> { > >> + ERRP_GUARD(); > >> VMCoreInfoState *vmci = vmcoreinfo_find(); > >> CPUState *cpu; > >> int nr_cpus; > >> - Error *err = NULL; > >> int ret; > >> > >> s->has_format = has_format; > >> @@ -1761,9 +1742,8 @@ static void dump_init(DumpState *s, int fd, bool > >> has_format, > >> > >> /* get memory mapping */ > >> if (paging) { > >> - qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, > >> &err); > >> - if (err != NULL) { > >> - error_propagate(errp, err); > >> + qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, > >> errp); > >> + if (*errp) { > >> goto cleanup; > >> } > >> } else { > >> @@ -1862,33 +1842,32 @@ cleanup: > >> /* this operation might be time consuming. */ > >> static void dump_process(DumpState *s, Error **errp) > >> { > >> - Error *local_err = NULL; > >> + ERRP_GUARD(); > >> DumpQueryResult *result = NULL; > >> > >> if (s->has_format && s->format == > DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) { > >> #ifdef TARGET_X86_64 > >> - create_win_dump(s, &local_err); > >> + create_win_dump(s, errp); > >> #endif > >> } else if (s->has_format && s->format != > >> DUMP_GUEST_MEMORY_FORMAT_ELF) { > >> - create_kdump_vmcore(s, &local_err); > >> + create_kdump_vmcore(s, errp); > >> } else { > >> - create_vmcore(s, &local_err); > >> + create_vmcore(s, errp); > >> } > >> > >> /* make sure status is written after written_size updates */ > >> smp_wmb(); > >> qatomic_set(&s->status, > >> - (local_err ? DUMP_STATUS_FAILED : > DUMP_STATUS_COMPLETED)); > >> + (*errp ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); > >> > >> /* send DUMP_COMPLETED message (unconditionally) */ > >> result = qmp_query_dump(NULL); > >> /* should never fail */ > >> assert(result); > >> - qapi_event_send_dump_completed(result, !!local_err, (local_err ? > >> - error_get_pretty(local_err) : > NULL)); > >> + qapi_event_send_dump_completed(result, !!*errp, (*errp ? > >> + > >> error_get_pretty(*errp) : NULL)); > >> qapi_free_DumpQueryResult(result); > >> > >> - error_propagate(errp, local_err); > >> dump_cleanup(s); > >> } > >> > >> @@ -1917,10 +1896,10 @@ void qmp_dump_guest_memory(bool paging, const > char > >> *file, > >> int64_t length, bool has_format, > >> DumpGuestMemoryFormat format, Error **errp) > >> { > >> + ERRP_GUARD(); > >> const char *p; > >> int fd = -1; > >> DumpState *s; > >> - Error *local_err = NULL; > >> bool detach_p = false; > >> > >> if (runstate_check(RUN_STATE_INMIGRATE)) { > >> @@ -2020,9 +1999,8 @@ void qmp_dump_guest_memory(bool paging, const char > >> *file, > >> dump_state_prepare(s); > >> > >> dump_init(s, fd, has_format, format, paging, has_begin, > >> - begin, length, &local_err); > >> - if (local_err) { > >> - error_propagate(errp, local_err); > >> + begin, length, errp); > >> + if (*errp) { > >> qatomic_set(&s->status, DUMP_STATUS_FAILED); > >> return; > >> } > >> -- > >> 2.32.0 > >> > >> > >> > > > >
diff --git a/dump/dump.c b/dump/dump.c index f57ed76fa7..58c4923fce 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -390,23 +390,21 @@ static void write_data(DumpState *s, void *buf, int length, Error **errp) static void write_memory(DumpState *s, GuestPhysBlock *block, ram_addr_t start, int64_t size, Error **errp) { + ERRP_GUARD(); int64_t i; - Error *local_err = NULL; for (i = 0; i < size / s->dump_info.page_size; i++) { write_data(s, block->host_addr + start + i * s->dump_info.page_size, - s->dump_info.page_size, &local_err); - if (local_err) { - error_propagate(errp, local_err); + s->dump_info.page_size, errp); + if (*errp) { return; } } if ((size % s->dump_info.page_size) != 0) { write_data(s, block->host_addr + start + i * s->dump_info.page_size, - size % s->dump_info.page_size, &local_err); - if (local_err) { - error_propagate(errp, local_err); + size % s->dump_info.page_size, errp); + if (*errp) { return; } } @@ -476,11 +474,11 @@ static void get_offset_range(hwaddr phys_addr, static void write_elf_loads(DumpState *s, Error **errp) { + ERRP_GUARD(); hwaddr offset, filesz; MemoryMapping *memory_mapping; uint32_t phdr_index = 1; uint32_t max_index; - Error *local_err = NULL; if (s->have_section) { max_index = s->sh_info; @@ -494,14 +492,13 @@ static void write_elf_loads(DumpState *s, Error **errp) s, &offset, &filesz); if (s->dump_info.d_class == ELFCLASS64) { write_elf64_load(s, memory_mapping, phdr_index++, offset, - filesz, &local_err); + filesz, errp); } else { write_elf32_load(s, memory_mapping, phdr_index++, offset, - filesz, &local_err); + filesz, errp); } - if (local_err) { - error_propagate(errp, local_err); + if (*errp) { return; } @@ -514,7 +511,7 @@ static void write_elf_loads(DumpState *s, Error **errp) /* write elf header, PT_NOTE and elf note to vmcore. */ static void dump_begin(DumpState *s, Error **errp) { - Error *local_err = NULL; + ERRP_GUARD(); /* * the vmcore's format is: @@ -542,73 +539,64 @@ static void dump_begin(DumpState *s, Error **errp) /* write elf header to vmcore */ if (s->dump_info.d_class == ELFCLASS64) { - write_elf64_header(s, &local_err); + write_elf64_header(s, errp); } else { - write_elf32_header(s, &local_err); + write_elf32_header(s, errp); } - if (local_err) { - error_propagate(errp, local_err); + if (*errp) { return; } if (s->dump_info.d_class == ELFCLASS64) { /* write PT_NOTE to vmcore */ - write_elf64_note(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf64_note(s, errp); + if (*errp) { return; } /* write all PT_LOAD to vmcore */ - write_elf_loads(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf_loads(s, errp); + if (*errp) { return; } /* write section to vmcore */ if (s->have_section) { - write_elf_section(s, 1, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf_section(s, 1, errp); + if (*errp) { return; } } /* write notes to vmcore */ - write_elf64_notes(fd_write_vmcore, s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf64_notes(fd_write_vmcore, s, errp); + if (*errp) { return; } } else { /* write PT_NOTE to vmcore */ - write_elf32_note(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf32_note(s, errp); + if (*errp) { return; } /* write all PT_LOAD to vmcore */ - write_elf_loads(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf_loads(s, errp); + if (*errp) { return; } /* write section to vmcore */ if (s->have_section) { - write_elf_section(s, 0, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf_section(s, 0, errp); + if (*errp) { return; } } /* write notes to vmcore */ - write_elf32_notes(fd_write_vmcore, s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf32_notes(fd_write_vmcore, s, errp); + if (*errp) { return; } } @@ -644,9 +632,9 @@ static int get_next_block(DumpState *s, GuestPhysBlock *block) /* write all memory to vmcore */ static void dump_iterate(DumpState *s, Error **errp) { + ERRP_GUARD(); GuestPhysBlock *block; int64_t size; - Error *local_err = NULL; do { block = s->next_block; @@ -658,9 +646,8 @@ static void dump_iterate(DumpState *s, Error **errp) size -= block->target_end - (s->begin + s->length); } } - write_memory(s, block, s->start, size, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_memory(s, block, s->start, size, errp); + if (*errp) { return; } @@ -669,11 +656,10 @@ static void dump_iterate(DumpState *s, Error **errp) static void create_vmcore(DumpState *s, Error **errp) { - Error *local_err = NULL; + ERRP_GUARD(); - dump_begin(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + dump_begin(s, errp); + if (*errp) { return; } @@ -810,6 +796,7 @@ static bool note_name_equal(DumpState *s, /* write common header, sub header and elf note to vmcore */ static void create_header32(DumpState *s, Error **errp) { + ERRP_GUARD(); DiskDumpHeader32 *dh = NULL; KdumpSubHeader32 *kh = NULL; size_t size; @@ -818,7 +805,6 @@ static void create_header32(DumpState *s, Error **errp) uint32_t bitmap_blocks; uint32_t status = 0; uint64_t offset_note; - Error *local_err = NULL; /* write common header, the version of kdump-compressed format is 6th */ size = sizeof(DiskDumpHeader32); @@ -894,9 +880,8 @@ static void create_header32(DumpState *s, Error **errp) s->note_buf_offset = 0; /* use s->note_buf to store notes temporarily */ - write_elf32_notes(buf_write_note, s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf32_notes(buf_write_note, s, errp); + if (*errp) { goto out; } if (write_buffer(s->fd, offset_note, s->note_buf, @@ -922,6 +907,7 @@ out: /* write common header, sub header and elf note to vmcore */ static void create_header64(DumpState *s, Error **errp) { + ERRP_GUARD(); DiskDumpHeader64 *dh = NULL; KdumpSubHeader64 *kh = NULL; size_t size; @@ -930,7 +916,6 @@ static void create_header64(DumpState *s, Error **errp) uint32_t bitmap_blocks; uint32_t status = 0; uint64_t offset_note; - Error *local_err = NULL; /* write common header, the version of kdump-compressed format is 6th */ size = sizeof(DiskDumpHeader64); @@ -1006,9 +991,8 @@ static void create_header64(DumpState *s, Error **errp) s->note_buf_offset = 0; /* use s->note_buf to store notes temporarily */ - write_elf64_notes(buf_write_note, s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_elf64_notes(buf_write_note, s, errp); + if (*errp) { goto out; } @@ -1464,8 +1448,8 @@ out: static void create_kdump_vmcore(DumpState *s, Error **errp) { + ERRP_GUARD(); int ret; - Error *local_err = NULL; /* * the kdump-compressed format is: @@ -1495,21 +1479,18 @@ static void create_kdump_vmcore(DumpState *s, Error **errp) return; } - write_dump_header(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_dump_header(s, errp); + if (*errp) { return; } - write_dump_bitmap(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_dump_bitmap(s, errp); + if (*errp) { return; } - write_dump_pages(s, &local_err); - if (local_err) { - error_propagate(errp, local_err); + write_dump_pages(s, errp); + if (*errp) { return; } @@ -1639,10 +1620,10 @@ static void dump_init(DumpState *s, int fd, bool has_format, DumpGuestMemoryFormat format, bool paging, bool has_filter, int64_t begin, int64_t length, Error **errp) { + ERRP_GUARD(); VMCoreInfoState *vmci = vmcoreinfo_find(); CPUState *cpu; int nr_cpus; - Error *err = NULL; int ret; s->has_format = has_format; @@ -1761,9 +1742,8 @@ static void dump_init(DumpState *s, int fd, bool has_format, /* get memory mapping */ if (paging) { - qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, &err); - if (err != NULL) { - error_propagate(errp, err); + qemu_get_guest_memory_mapping(&s->list, &s->guest_phys_blocks, errp); + if (*errp) { goto cleanup; } } else { @@ -1862,33 +1842,32 @@ cleanup: /* this operation might be time consuming. */ static void dump_process(DumpState *s, Error **errp) { - Error *local_err = NULL; + ERRP_GUARD(); DumpQueryResult *result = NULL; if (s->has_format && s->format == DUMP_GUEST_MEMORY_FORMAT_WIN_DMP) { #ifdef TARGET_X86_64 - create_win_dump(s, &local_err); + create_win_dump(s, errp); #endif } else if (s->has_format && s->format != DUMP_GUEST_MEMORY_FORMAT_ELF) { - create_kdump_vmcore(s, &local_err); + create_kdump_vmcore(s, errp); } else { - create_vmcore(s, &local_err); + create_vmcore(s, errp); } /* make sure status is written after written_size updates */ smp_wmb(); qatomic_set(&s->status, - (local_err ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); + (*errp ? DUMP_STATUS_FAILED : DUMP_STATUS_COMPLETED)); /* send DUMP_COMPLETED message (unconditionally) */ result = qmp_query_dump(NULL); /* should never fail */ assert(result); - qapi_event_send_dump_completed(result, !!local_err, (local_err ? - error_get_pretty(local_err) : NULL)); + qapi_event_send_dump_completed(result, !!*errp, (*errp ? + error_get_pretty(*errp) : NULL)); qapi_free_DumpQueryResult(result); - error_propagate(errp, local_err); dump_cleanup(s); } @@ -1917,10 +1896,10 @@ void qmp_dump_guest_memory(bool paging, const char *file, int64_t length, bool has_format, DumpGuestMemoryFormat format, Error **errp) { + ERRP_GUARD(); const char *p; int fd = -1; DumpState *s; - Error *local_err = NULL; bool detach_p = false; if (runstate_check(RUN_STATE_INMIGRATE)) { @@ -2020,9 +1999,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, dump_state_prepare(s); dump_init(s, fd, has_format, format, paging, has_begin, - begin, length, &local_err); - if (local_err) { - error_propagate(errp, local_err); + begin, length, errp); + if (*errp) { qatomic_set(&s->status, DUMP_STATUS_FAILED); return; }