On 10/24/2014 10:05 PM, Dave Anderson wrote:
With the patch I that proposed, you can simplify this part of your patch:
+ 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) {
to just this:
if (is_incomplete_dump()&& (read_ret>= 0)) {
But what about compressed kdumps? In the case of DUMP_DH_COMPRESSED_INCOMPLETE
dumpfiles, will makedumpfile manipulate the bitmaps and the page_desc.offset values
of all "missing" pages to point to the special "pd_zero" page? Will it be
impossible to distinguish between legitimate zero-filled pages and "missing" pages?
I suppose that if that is true, then the behavior should be the same for
both ELF and compressed kdumps, which would be to return zero-filled data.
On the other hand...
What's bothersome about automatically returning zero-filled data is that,
currently, if an ELF vmcore was originally created correctly, but gets truncated
*after* the fact, for example, when downloading it from a customer site,
check_dumpfile_size() will report the error. But when data from the truncated
region gets read, a read error is returned.
If zero-filled data is silently returned, I can imagine that there will be all
kinds of errors that will occur -- both display/content errors, but it could
easily cause SIGSEGV's in the crash utility when it tries to use zero-filled
data thinking that it's legitimate. How would the user even know whether it's
because the page was truncated or not?
The patch has been adjusted according to your suggestions. The kdump part has been
fixed. And the warning information is added in both elf part and kdump part.
Thank you for your guiding.
The crash utility already has a --zero_excluded flag for returning zero-filled
memory if by chance a*legitimately* excluded/filtered page is read. It's
hardly ever used, because those pages should never be required. But I
wonder whether it could be re-purposed in this case, where by default an error
would be returned when reading missing pages, and zero-filled only if it is
explicitly requested with the --zero_excluded command line option? Or would
that be impossible with DUMP_DH_COMPRESSED_INCOMPLETE compressed kdumps?
I think it's a good idea and is completed in patch. It can also work in kdump part.
The attachment is the latest patch.
Thanks
Zhou Wenjian
diff --git a/diskdump.c b/diskdump.c
index 1d6f7a5..850b174 100644
--- a/diskdump.c
+++ b/diskdump.c
@@ -999,6 +999,17 @@ cache_page(physaddr_t paddr)
if (FLAT_FORMAT()) {
if (!read_flattened_format(dd->dfd, pd.offset, dd->compressed_page, pd.size))
return READ_ERROR;
+ } else if (is_incomplete_dump() && (0 == pd.offset)) {
+ /*
+ * if --zero_excluded is specified and incomplete flag is set in dump file,
+ * zero will be used to fill the lost part.
+ */
+ if (!(*diskdump_flags & ZERO_EXCLUDED))
+ return READ_ERROR;
+ error(WARNING, "%s: data may be lost\n"
+ " pfn:%lld\n\n"
+ ,pc->dumpfile,pfn);
+ memset(dd->compressed_page, 0, dd->block_size);
} else {
if (lseek(dd->dfd, pd.offset, SEEK_SET) == failed)
return SEEK_ERROR;
diff --git a/netdump.c b/netdump.c
index abc85e0..c8f057e 100644
--- a/netdump.c
+++ b/netdump.c
@@ -608,7 +608,21 @@ 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 --zero_excluded is specified and 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) {
+ if (is_incomplete_dump() && (read_ret >= 0 &&
+ *diskdump_flags & ZERO_EXCLUDED)) {
+ error(WARNING, "%s: data may be lost\n"
+ " offset:%llx\n\n",
+ pc->dumpfile, offset);
+ 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
