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