| Message ID | 20230918233233.1431858-3-stephen.s.brennan@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Allow dump-guest-memory to output standard kdump format | expand |
Hi Stephen On Tue, Sep 19, 2023 at 3:32 AM Stephen Brennan <stephen.s.brennan@oracle.com> wrote: > > The flattened format (currently output by QEMU) is used by makedumpfile > only when it is outputting a vmcore to a file which is not seekable. The > flattened format functions essentially as a set of instructions of the > form "seek to the given offset, then write the given bytes out". > > The flattened format can be reconstructed using makedumpfile -R, or > makedumpfile-R.pl, but it is a slow process because it requires copying > the entire vmcore. The flattened format can also be directly read by > crash, but still, it requires a lengthy reassembly phase. > > To sum up, the flattened format is not an ideal one: it should only be > used on files which are actually not seekable. This is the exact > strategy which makedumpfile uses, as seen in the implementation of > "write_buffer()" in makedumpfile [1]. However, QEMU has always used the > flattened format. For compatibility it is best not to change the default > output format without warning. So, add a flag to DumpState which changes > the output to use the normal (i.e. raw) format. This flag will be added > to the QMP and HMP commands in the next change. > > [1]: https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040 > > Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> > --- > dump/dump.c | 32 +++++++++++++++++++++++++------- > include/sysemu/dump.h | 1 + > 2 files changed, 26 insertions(+), 7 deletions(-) > > diff --git a/dump/dump.c b/dump/dump.c > index 74071a1565..10aa2c79e0 100644 > --- a/dump/dump.c > +++ b/dump/dump.c > @@ -814,6 +814,10 @@ static int write_start_flat_header(DumpState *s) > MakedumpfileHeader *mh; > int ret = 0; > > + if (s->kdump_raw) { > + return 0; > + } > + > QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER); > mh = g_malloc0(MAX_SIZE_MDF_HEADER); > > @@ -837,6 +841,10 @@ static int write_end_flat_header(DumpState *s) > { > MakedumpfileDataHeader mdh; > > + if (s->kdump_raw) { > + return 0; > + } > + > mdh.offset = END_FLAG_FLAT_HEADER; > mdh.buf_size = END_FLAG_FLAT_HEADER; > > @@ -853,13 +861,21 @@ static int write_buffer(DumpState *s, off_t offset, const void *buf, size_t size > { > size_t written_size; > MakedumpfileDataHeader mdh; > + loff_t seek_loc; Any reason to use loff_t over off_t here? It fails to compile on win32 for ex. I can touch on PR commit otherwise. > > - mdh.offset = cpu_to_be64(offset); > - mdh.buf_size = cpu_to_be64(size); > + if (s->kdump_raw) { > + seek_loc = lseek(s->fd, offset, SEEK_SET); > + if (seek_loc == (off_t) -1) { > + return -1; > + } > + } else { > + mdh.offset = cpu_to_be64(offset); > + mdh.buf_size = cpu_to_be64(size); > > - written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); > - if (written_size != sizeof(mdh)) { > - return -1; > + written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); > + if (written_size != sizeof(mdh)) { > + return -1; > + } > } > > written_size = qemu_write_full(s->fd, buf, size); > @@ -1775,7 +1791,8 @@ static void vmcoreinfo_update_phys_base(DumpState *s) > > 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) > + int64_t begin, int64_t length, bool kdump_raw, > + Error **errp) > { > ERRP_GUARD(); > VMCoreInfoState *vmci = vmcoreinfo_find(); > @@ -1786,6 +1803,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, > s->has_format = has_format; > s->format = format; > s->written_size = 0; > + s->kdump_raw = kdump_raw; > > /* kdump-compressed is conflict with paging and filter */ > if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { > @@ -2168,7 +2186,7 @@ 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, errp); > + begin, length, false, errp); > if (*errp) { > qatomic_set(&s->status, DUMP_STATUS_FAILED); > return; > diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h > index e27af8fb34..d702854853 100644 > --- a/include/sysemu/dump.h > +++ b/include/sysemu/dump.h > @@ -157,6 +157,7 @@ typedef struct DumpState { > MemoryMappingList list; > bool resume; > bool detached; > + bool kdump_raw; > hwaddr memory_offset; > int fd; > > -- > 2.39.3 >
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi Stephen > > On Tue, Sep 19, 2023 at 3:32 AM Stephen Brennan > <stephen.s.brennan@oracle.com> wrote: >> >> The flattened format (currently output by QEMU) is used by makedumpfile >> only when it is outputting a vmcore to a file which is not seekable. The >> flattened format functions essentially as a set of instructions of the >> form "seek to the given offset, then write the given bytes out". >> >> The flattened format can be reconstructed using makedumpfile -R, or >> makedumpfile-R.pl, but it is a slow process because it requires copying >> the entire vmcore. The flattened format can also be directly read by >> crash, but still, it requires a lengthy reassembly phase. >> >> To sum up, the flattened format is not an ideal one: it should only be >> used on files which are actually not seekable. This is the exact >> strategy which makedumpfile uses, as seen in the implementation of >> "write_buffer()" in makedumpfile [1]. However, QEMU has always used the >> flattened format. For compatibility it is best not to change the default >> output format without warning. So, add a flag to DumpState which changes >> the output to use the normal (i.e. raw) format. This flag will be added >> to the QMP and HMP commands in the next change. >> >> [1]: https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040 >> >> Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> >> --- >> dump/dump.c | 32 +++++++++++++++++++++++++------- >> include/sysemu/dump.h | 1 + >> 2 files changed, 26 insertions(+), 7 deletions(-) >> >> diff --git a/dump/dump.c b/dump/dump.c >> index 74071a1565..10aa2c79e0 100644 >> --- a/dump/dump.c >> +++ b/dump/dump.c >> @@ -814,6 +814,10 @@ static int write_start_flat_header(DumpState *s) >> MakedumpfileHeader *mh; >> int ret = 0; >> >> + if (s->kdump_raw) { >> + return 0; >> + } >> + >> QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER); >> mh = g_malloc0(MAX_SIZE_MDF_HEADER); >> >> @@ -837,6 +841,10 @@ static int write_end_flat_header(DumpState *s) >> { >> MakedumpfileDataHeader mdh; >> >> + if (s->kdump_raw) { >> + return 0; >> + } >> + >> mdh.offset = END_FLAG_FLAT_HEADER; >> mdh.buf_size = END_FLAG_FLAT_HEADER; >> >> @@ -853,13 +861,21 @@ static int write_buffer(DumpState *s, off_t offset, const void *buf, size_t size >> { >> size_t written_size; >> MakedumpfileDataHeader mdh; >> + loff_t seek_loc; > > Any reason to use loff_t over off_t here? It fails to compile on win32 > for ex. I can touch on PR commit otherwise. I think that was an oversight on my part: lseek() uses off_t. I see that qemu is compiled with _FILE_OFFSET_BITS=64 so off_t should be 64 bits even on 32-bit architectures. So this should be off_t. I believe the compile error would also happen in qmp_dump_guest_memory() where I have: if (kdump_raw && lseek(fd, 0, SEEK_CUR) == (loff_t) -1) { If it's easiest for you to tweak it in the PR, please do. Otherwise I can respin the series replacing loff_t with off_t. Thank you, Stephen >> >> - mdh.offset = cpu_to_be64(offset); >> - mdh.buf_size = cpu_to_be64(size); >> + if (s->kdump_raw) { >> + seek_loc = lseek(s->fd, offset, SEEK_SET); >> + if (seek_loc == (off_t) -1) { >> + return -1; >> + } >> + } else { >> + mdh.offset = cpu_to_be64(offset); >> + mdh.buf_size = cpu_to_be64(size); >> >> - written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); >> - if (written_size != sizeof(mdh)) { >> - return -1; >> + written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); >> + if (written_size != sizeof(mdh)) { >> + return -1; >> + } >> } >> >> written_size = qemu_write_full(s->fd, buf, size); >> @@ -1775,7 +1791,8 @@ static void vmcoreinfo_update_phys_base(DumpState *s) >> >> 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) >> + int64_t begin, int64_t length, bool kdump_raw, >> + Error **errp) >> { >> ERRP_GUARD(); >> VMCoreInfoState *vmci = vmcoreinfo_find(); >> @@ -1786,6 +1803,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, >> s->has_format = has_format; >> s->format = format; >> s->written_size = 0; >> + s->kdump_raw = kdump_raw; >> >> /* kdump-compressed is conflict with paging and filter */ >> if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { >> @@ -2168,7 +2186,7 @@ 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, errp); >> + begin, length, false, errp); >> if (*errp) { >> qatomic_set(&s->status, DUMP_STATUS_FAILED); >> return; >> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h >> index e27af8fb34..d702854853 100644 >> --- a/include/sysemu/dump.h >> +++ b/include/sysemu/dump.h >> @@ -157,6 +157,7 @@ typedef struct DumpState { >> MemoryMappingList list; >> bool resume; >> bool detached; >> + bool kdump_raw; >> hwaddr memory_offset; >> int fd; >> >> -- >> 2.39.3 >> > > > -- > Marc-André Lureau
diff --git a/dump/dump.c b/dump/dump.c index 74071a1565..10aa2c79e0 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -814,6 +814,10 @@ static int write_start_flat_header(DumpState *s) MakedumpfileHeader *mh; int ret = 0; + if (s->kdump_raw) { + return 0; + } + QEMU_BUILD_BUG_ON(sizeof *mh > MAX_SIZE_MDF_HEADER); mh = g_malloc0(MAX_SIZE_MDF_HEADER); @@ -837,6 +841,10 @@ static int write_end_flat_header(DumpState *s) { MakedumpfileDataHeader mdh; + if (s->kdump_raw) { + return 0; + } + mdh.offset = END_FLAG_FLAT_HEADER; mdh.buf_size = END_FLAG_FLAT_HEADER; @@ -853,13 +861,21 @@ static int write_buffer(DumpState *s, off_t offset, const void *buf, size_t size { size_t written_size; MakedumpfileDataHeader mdh; + loff_t seek_loc; - mdh.offset = cpu_to_be64(offset); - mdh.buf_size = cpu_to_be64(size); + if (s->kdump_raw) { + seek_loc = lseek(s->fd, offset, SEEK_SET); + if (seek_loc == (off_t) -1) { + return -1; + } + } else { + mdh.offset = cpu_to_be64(offset); + mdh.buf_size = cpu_to_be64(size); - written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); - if (written_size != sizeof(mdh)) { - return -1; + written_size = qemu_write_full(s->fd, &mdh, sizeof(mdh)); + if (written_size != sizeof(mdh)) { + return -1; + } } written_size = qemu_write_full(s->fd, buf, size); @@ -1775,7 +1791,8 @@ static void vmcoreinfo_update_phys_base(DumpState *s) 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) + int64_t begin, int64_t length, bool kdump_raw, + Error **errp) { ERRP_GUARD(); VMCoreInfoState *vmci = vmcoreinfo_find(); @@ -1786,6 +1803,7 @@ static void dump_init(DumpState *s, int fd, bool has_format, s->has_format = has_format; s->format = format; s->written_size = 0; + s->kdump_raw = kdump_raw; /* kdump-compressed is conflict with paging and filter */ if (has_format && format != DUMP_GUEST_MEMORY_FORMAT_ELF) { @@ -2168,7 +2186,7 @@ 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, errp); + begin, length, false, errp); if (*errp) { qatomic_set(&s->status, DUMP_STATUS_FAILED); return; diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h index e27af8fb34..d702854853 100644 --- a/include/sysemu/dump.h +++ b/include/sysemu/dump.h @@ -157,6 +157,7 @@ typedef struct DumpState { MemoryMappingList list; bool resume; bool detached; + bool kdump_raw; hwaddr memory_offset; int fd;
The flattened format (currently output by QEMU) is used by makedumpfile only when it is outputting a vmcore to a file which is not seekable. The flattened format functions essentially as a set of instructions of the form "seek to the given offset, then write the given bytes out". The flattened format can be reconstructed using makedumpfile -R, or makedumpfile-R.pl, but it is a slow process because it requires copying the entire vmcore. The flattened format can also be directly read by crash, but still, it requires a lengthy reassembly phase. To sum up, the flattened format is not an ideal one: it should only be used on files which are actually not seekable. This is the exact strategy which makedumpfile uses, as seen in the implementation of "write_buffer()" in makedumpfile [1]. However, QEMU has always used the flattened format. For compatibility it is best not to change the default output format without warning. So, add a flag to DumpState which changes the output to use the normal (i.e. raw) format. This flag will be added to the QMP and HMP commands in the next change. [1]: https://github.com/makedumpfile/makedumpfile/blob/f23bb943568188a2746dbf9b6692668f5a2ac3b6/makedumpfile.c#L5008-L5040 Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com> --- dump/dump.c | 32 +++++++++++++++++++++++++------- include/sysemu/dump.h | 1 + 2 files changed, 26 insertions(+), 7 deletions(-)