[Crash-utility] add support for incomplete elf dump file

["Zhou, Wenjian/周文剑"]

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


-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: SUPPROT_INCOMPLETE_DUMPFILE.patch
URL: <http://listman.redhat.com/archives/crash-utility/attachments/20141028/ac5e6f4c/attachment.ksh>