On 10/28/2014 01:35 AM, Dave Anderson wrote:
A couple notes and questions re: your latest patch:
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;
What about FLAT_FORMAT()? Should the (is_incomplete_dump()/pd.offset==0) test below be
done before the FLAT_FORMAT() check?
In makedumpfile, we can't set incomplete flag in flat_format. So, the patch can't support the
flat_format.
+ } else if (is_incomplete_dump()&& (0 == pd.offset)) {
Is this how the makedumpfile patch marks pages that have been truncated, i.e., by
creating a page_desc_t that has a pd.offset == 0?
Actually, we just set incomplete flag in kdump part. When ENOSPACE happens, page_desc_t, haven't be
created, will not be created any more. And there is nothing in the position belong to it. When read
page_desc_t out at the position, pd.offset will == 0.
+ /*
+ * 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);
When --zero_excluded is entered on the crash command line, then the user knows exactly
what he is doing, and so there should not be any error/warning messages from the read
command itself. A message should only be displayed if debug mode is turned on. For
example, this is how it is done currently, but only if debug is set to 8:
if (CRASHDEBUG(8))
fprintf(fp, "read_diskdump: zero-fill: "
"paddr/pfn: %llx/%lx\n",
(ulonglong)paddr, 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);
Same thing here... if a user enters --zero_excluded on the command line, he should
expect strange behavior as a result. Intermingling a bunch of WARNING messages
like the one above will only make it more confusing.
+ bufptr += read_ret;
+ bzero(bufptr, cnt - read_ret);
+ return cnt;
+ }
if (CRASHDEBUG(8))
fprintf(fp, "read_netdump: READ_ERROR: "
"offset: %llx\n", (ulonglong)offset);
I fixed that. And add more descriptions to the option --zero_excluded.
Thanks
Zhou Wenjian
diff --git a/crash.8 b/crash.8
index 16f2e8d..9fef355 100644
--- a/crash.8
+++ b/crash.8
@@ -396,8 +396,9 @@ header.
.TP
.B --zero_excluded
If a kdump dumpfile has been filtered to exclude various types of non-essential
-pages, any attempt to read them will fail. With this flag,
-reads from any of those pages will return zero-filled memory.
+pages, any attempt to read them will fail. If dumpfile is incomplete, any attempt
+to read the lost pages will fail. With this flag, reads from any of those pages
+will return zero-filled memory.
.TP
.B --no_panic
Do not attempt to find the task that was running when the kernel crashed.
diff --git a/diskdump.c b/diskdump.c
index 1d6f7a5..b609cd8 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 incomplete flag is set in dump file, zero will be used to fill
+ * the lost part.
+ */
+ if (!(*diskdump_flags & ZERO_EXCLUDED))
+ return READ_ERROR;
+ if (CRASHDEBUG(8))
+ fprintf(fp, "cache_page: %s: data may be lost\n"
+ "zero-fill: pfn: %llx\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/help.c b/help.c
index 723e79e..5467489 100644
--- a/help.c
+++ b/help.c
@@ -270,7 +270,8 @@ char *program_usage_info[] = {
"",
" --zero_excluded",
" If a kdump dumpfile has been filtered to exclude various types of",
- " non-essential pages, any attempt to read them will fail. With",
+ " non-essential pages, any attempt to read them will fail. If dumpfile",
+ " is incomplete, any attempt to read the lost pages will fail. With",
" this flag, reads from any of those pages will return zero-filled",
" memory.",
"",
@@ -1043,8 +1044,8 @@ char *help_set[] = {
" (scrolling is turned off if silent is on)",
" edit vi | emacs set line editing mode (from .%src file only).",
" namelist filename name of kernel (from .%src file only).",
-" zero_excluded on | off controls whether excluded pages from a dumpfile",
-" should return zero-filled memory.",
+" zero_excluded on | off controls whether excluded pages or lost pages",
+" from a dumpfile should return zero-filled memory.",
" null-stop on | off if on, gdb's printing of character arrays will",
" stop at the first NULL encountered.",
" gdb on | off if on, the %s session will be run in a mode",
diff --git a/netdump.c b/netdump.c
index abc85e0..dce523d 100644
--- a/netdump.c
+++ b/netdump.c
@@ -608,7 +608,22 @@ 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) {
+ if (is_incomplete_dump() && (read_ret >= 0) &&
+ (*diskdump_flags & ZERO_EXCLUDED)) {
+ if (CRASHDEBUG(8))
+ fprintf(fp, "read_netdump: %s: data may be lost\n"
+ "zero-fill: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
