On 10/23/2014 08:10 PM, Dave Anderson wrote:
----- Original Message -----
Hello Dave,
On 10/22/2014 09:49 PM, Dave Anderson wrote:
Hello Wenjian,
First -- please send patches as email attachments. Even if I cut-and-paste
your
patch from the crash-utility archives (which normally works), there are
still no tabs in your patch.
Appreciate your suggestion.
Now, with respect to the patch, it's not clear to me what would happen if you
make no changes to check_dumpfile_size()?
It seems to me that a read error would occur regardless whether you change
the PT_LOAD-related contents or not.
If no changes are made:
If a readmem() of a truncated page is attempted, read_netdump() would
calculate an offset based upon the original PT_LOAD contents, but then
the subsequent read() would fail, and would return a READ_ERROR.
If your patch is applied:
If a readmem() of a truncated page is attempted, read_netdump() would not
be able to calculate an offset, and would return a READ_ERROR.
What's the difference? Why even bother making the changes?
If my patch is applied:
If a readmem() of a truncated page is attempted,
(pls->zero_fill&& (paddr>= pls->phys_end)&& (paddr< pls->zero_fill)),
,his will be right. So read_netdump() will fill bufptr with zero and
return cnt.
In my patch, information of PT_LOAD segment is modified, so that the segment will
not go beyond the file size. And use zero to replace the lost part.
Previously, we intend to do this work in makedumpfile by modifying the PT_LOAD
header of the ELF dump file. But kumagai atsushi thought it's not good to make the
irreversible change. So we think do it in crash maybe a better choice.
I also don't understand what the difference between "truncated" and "incomplete"
is? Why did you separate the messages into two?
At first, I thought the difference is the incomplete flag.
Now, I think it's not necessary to distinguish them.
Anyway, I had already created a patch in preparation for the changes to
makedumpfile for ELF and compressed kdump vmcores. The patch will apply
to the current github master branch. Please apply it alone, and tell me
what happens.
The patch can show the incomplete information, but will be interrupted by the
READ_ERROR (we don't change the PT_LOAD headers in makedumpfile any more).
Well, it *should* be interrupted. What's worse, "interruption" or the receipt
of completely bogus zero-filled data?
This whole scheme is even crazier than I thought. I'm going to have
to make the initialization-time warning message even more ominous than
I already have it.
Actually, I haven't realized it until doing the work in crash.
I think which is better or worse depends on what people want to do.
If incomplete flag exists, it may indicate people want to analyse the file,
and returning zero-filled data is better. Otherwise, "interruption" may be expected.
But I still don't think it's necessary to modify the original PT_LOAD
contents. If read_netdump() finds a legitimate offset, but that offset
is beyond the end of the incomplete/truncated file, it can just check the
INCOMPLETE flag and return a zero-filled buffer if it's set. Wouldn't
that work?
It does work. And now, the question turns to be which way is better,
first: do it in check_dumpfile_size() by modifying relevant information
(not modifying the information in ELF file)
second: do it in read_netdump() by return zero-filled data with
checking offset and flag
, isn't it?
The attachment is the patch of second method.
Thanks
Zhou Wenjian
diff --git a/netdump.c b/netdump.c
index abc85e0..0c7cc80 100644
--- a/netdump.c
+++ b/netdump.c
@@ -608,7 +608,20 @@ read_netdump(int fd, void *bufptr, int cnt, ulong addr, physaddr_t paddr)
"offset: %llx\n", (ulonglong)offset);
return SEEK_ERROR;
}
- if (read(nd->ndfd, bufptr, cnt) != cnt) {
+ /*
+ * if incomplete flag is set in ELF file, zero will be used to fill
+ * the lost part.
+ */
+ ssize_t read_ret = read(nd->ndfd, bufptr, cnt);
+ if (read_ret != cnt) {
+ unsigned int e_flag = (NULL == nd->elf64) ?
+ (nd->elf32)->e_flags : (nd->elf64)->e_flags;
+ int status = e_flag & DUMP_ELF_INCOMPLETE;
+ if (status && read_ret >= 0) {
+ bufptr += read_ret;
+ bzero(bufptr, cnt - read_ret);
+ return cnt;
+ }
if (CRASHDEBUG(8))
fprintf(fp, "read_netdump: READ_ERROR: "
"offset: %llx\n", (ulonglong)offset);
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
