[Patch] fix error case of xen

[Patch] fix error case of xen

[Akio Takebe]

Hi,

Curret kdump-xen with makdumpfile always return 0.
So if we fail to kdump, we will get $?=0.
This patch improve it.

Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>

--- makedumpfile/makedumpfile.c	2008-03-28 11:32:51.000000000 +0900
+++ makedumpfile.mod/makedumpfile.c	2008-04-01 02:14:12.000000000 +0900
@@ -993,6 +993,8 @@ get_symbol_addr(char *symname)
 		if (!strcmp(sym_name, symname)) {
 			symbol = sym.st_value;
 			break;
+		} else if (i >= (shdr.sh_size/shdr.sh_entsize)-1) {
+			DEBUG_MSG("Can't get symbol of %s.\n", symname);
 		}
 	}
 out:
@@ -6146,7 +6148,8 @@ main(int argc, char *argv[])
 			goto out;
 		}
 		info->dump_level |= DL_EXCLUDE_XEN;
-		return handle_xen();
+		if (!handle_xen())
+			goto out;
 
 	} else if (info->flag_rearrange) {
 		if (!open_files_for_rearranging_dumpdata())


Best Regards,

Akio Takebe


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Patch] fix error case of xen

[Ken'ichi Ohmichi]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]


Hi Takebe-san,

Thank you for the patch.

Akio Takebe wrote:
> Curret kdump-xen with makdumpfile always return 0.
> So if we fail to kdump, we will get $?=0.
> This patch improve it.
> 
> Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>
> 
> --- makedumpfile/makedumpfile.c	2008-03-28 11:32:51.000000000 +0900
> +++ makedumpfile.mod/makedumpfile.c	2008-04-01 02:14:12.000000000 +0900
> @@ -993,6 +993,8 @@ get_symbol_addr(char *symname)
>  		if (!strcmp(sym_name, symname)) {
>  			symbol = sym.st_value;
>  			break;
> +		} else if (i >= (shdr.sh_size/shdr.sh_entsize)-1) {
> +			DEBUG_MSG("Can't get symbol of %s.\n", symname);

This patch is for fixing error code of xen, but the above code adds
the debugging message. So the above code doesn't match its purpose.
Is it necessary ?


> -		return handle_xen();
> +		if (!handle_xen())
> +			goto out;

I guess that the above code is not enough. If handle_xen() succeeds,
it should return TRUE instead of COMPLETED like the attached patch, right ?


Thanks
Ken'ichi Ohmichi

Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>
Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
---
diff -puN backup/v1.2.5/makedumpfile.c makedumpfile/makedumpfile.c
--- backup/v1.2.5/makedumpfile.c	2008-03-28 11:54:48.000000000 +0900
+++ makedumpfile/makedumpfile.c	2008-04-01 14:32:03.000000000 +0900
@@ -5855,7 +5855,7 @@ handle_xen()
 	MSG("\n");
 	MSG("The dumpfile is saved to %s.\n", info->name_dumpfile);
 
-	return COMPLETED;
+	return TRUE;
 out:
 	return FALSE;
 #endif
@@ -6146,7 +6146,8 @@ main(int argc, char *argv[])
 			goto out;
 		}
 		info->dump_level |= DL_EXCLUDE_XEN;
-		return handle_xen();
+		if (!handle_xen())
+			goto out;
 
 	} else if (info->flag_rearrange) {
 		if (!open_files_for_rearranging_dumpdata())



[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Patch] fix error case of xen

[Ken'ichi Ohmichi]

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]


Hi Takebe-san,

Ken'ichi Ohmichi wrote:
> Hi Takebe-san,
> 
> Thank you for the patch.
> 
> Akio Takebe wrote:
>> Curret kdump-xen with makdumpfile always return 0.
>> So if we fail to kdump, we will get $?=0.
>> This patch improve it.
>>
>> Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>
>>
>> --- makedumpfile/makedumpfile.c	2008-03-28 11:32:51.000000000 +0900
>> +++ makedumpfile.mod/makedumpfile.c	2008-04-01 02:14:12.000000000 +0900
>> @@ -993,6 +993,8 @@ get_symbol_addr(char *symname)
>>  		if (!strcmp(sym_name, symname)) {
>>  			symbol = sym.st_value;
>>  			break;
>> +		} else if (i >= (shdr.sh_size/shdr.sh_entsize)-1) {
>> +			DEBUG_MSG("Can't get symbol of %s.\n", symname);
> 
> This patch is for fixing error code of xen, but the above code adds
> the debugging message. So the above code doesn't match its purpose.
> Is it necessary ?

Sorry for incomprehensible comment.

The existences of symbols depend on linux version, cpu architecture,
and linux configuration (.config file). If the above code is merged,
the debugging message will be big and unreadable.
Do you want to know the lacks of necessary symbol for dump filtering 
of Xen ?  -D option prints them.


Thanks
Ken'ichi Ohmichi


[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Patch] fix error case of xen

[Akio Takebe]

Hi, Ohmichi-san

Thank you for your review.

>> Akio Takebe wrote:
>>> Curret kdump-xen with makdumpfile always return 0.
>>> So if we fail to kdump, we will get $?=0.
>>> This patch improve it.
>>>
>>> Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>
>>>
>>> --- makedumpfile/makedumpfile.c	2008-03-28 11:32:51.000000000 +0900
>>> +++ makedumpfile.mod/makedumpfile.c	2008-04-01 02:14:12.000000000 +0900
>>> @@ -993,6 +993,8 @@ get_symbol_addr(char *symname)
>>>  		if (!strcmp(sym_name, symname)) {
>>>  			symbol = sym.st_value;
>>>  			break;
>>> +		} else if (i >= (shdr.sh_size/shdr.sh_entsize)-1) {
>>> +			DEBUG_MSG("Can't get symbol of %s.\n", symname);
>> 
>> This patch is for fixing error code of xen, but the above code adds
>> the debugging message. So the above code doesn't match its purpose.
>> Is it necessary ?
>
>Sorry for incomprehensible comment.
>
>The existences of symbols depend on linux version, cpu architecture,
>and linux configuration (.config file). If the above code is merged,
>the debugging message will be big and unreadable.
>Do you want to know the lacks of necessary symbol for dump filtering 
>of Xen ?  -D option prints them.
>
Yes, that's right.
The concept of this hunk is to catch error at making vmcoreinfo file.
Current vmcoreinfo doesn't show error if some necessary symbols are not included.
Then we will find it at failing kdump.
So I wanted to add the debug message into the function finding symbol.
Since the hunk is not important, please drop it.

Best Regards,

Akio Takebe


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Patch] fix error case of xen

[Akio Takebe]

Hi, Ohmichi-san

>
>> -		return handle_xen();
>> +		if (!handle_xen())
>> +			goto out;
>
>I guess that the above code is not enough. If handle_xen() succeeds,
>it should return TRUE instead of COMPLETED like the attached patch, right ?

You're right!
Your patch looks good, thank you very much for your review.

Best Regards,

Akio Takebe


>
>
>Thanks
>Ken'ichi Ohmichi
>
>Signed-off-by: Akio Takebe <takebe_akio@jp.fujitsu.com>
>Signed-off-by: Ken'ichi Ohmichi <oomichi@mxs.nes.nec.co.jp>
>---
>diff -puN backup/v1.2.5/makedumpfile.c makedumpfile/makedumpfile.c
>--- backup/v1.2.5/makedumpfile.c	2008-03-28 11:54:48.000000000 +0900
>+++ makedumpfile/makedumpfile.c	2008-04-01 14:32:03.000000000 +0900
>@@ -5855,7 +5855,7 @@ handle_xen()
> 	MSG("\n");
> 	MSG("The dumpfile is saved to %s.\n", info->name_dumpfile);
> 
>-	return COMPLETED;
>+	return TRUE;
> out:
> 	return FALSE;
> #endif
>@@ -6146,7 +6146,8 @@ main(int argc, char *argv[])
> 			goto out;
> 		}
> 		info->dump_level |= DL_EXCLUDE_XEN;
>-		return handle_xen();
>+		if (!handle_xen())
>+			goto out;
> 
> 	} else if (info->flag_rearrange) {
> 		if (!open_files_for_rearranging_dumpdata())


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [Patch] fix error case of xen

[Ken'ichi Ohmichi]

[-- Attachment #1: Type: text/plain, Size: 377 bytes --]


Hi Takebe-san,

Akio Takebe wrote:
>> I guess that the above code is not enough. If handle_xen() succeeds,
>> it should return TRUE instead of COMPLETED like the attached patch, right ?
> 
> You're right!
> Your patch looks good, thank you very much for your review.

Thank you for the comment.
I'll release the next makedumpfile which includes it.


Thanks
Ken'ichi Ohmichi


[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec