[CRIU] Re: [PATCH] restorer: close log file before detaching from crtools

Kinsbursky Stanislav skinsbursky at openvz.org
Mon Feb 20 11:52:05 EST 2012


20.02.2012 20:35, Cyrill Gorcunov пишет:
> On Mon, Feb 20, 2012 at 08:21:45PM +0400, Kinsbursky Stanislav wrote:
>> 20.02.2012 19:31, Cyrill Gorcunov пишет:
>>> On Mon, Feb 20, 2012 at 07:12:14PM +0400, Kinsbursky Stanislav wrote:
>>> ...
>>>>>> +
>>>>>> +core_restore_failed:
>>>>>> +	asm volatile(
>>>>>> +		"movq %0, %%rsp				\n"
>>>>>> +		"jmp *%1				\n"
>>>>>> +		:
>>>>>> +		: "r"(ret), "r"(line)
>>>>>> +		: );
>>>>>> +	return ret;
>>>>> We have a similar code in BUG_ON_HANDLER, but this code is better, so
>>>>> I think you can improve BUG_ON_HANDLER and use it.
>>>> Thanks for your valuable opinion. I'll keep that in mind.
>>>>
>>> Since both IP and SP are printed in dmesg I guess we might load
>>> line number into SP and cause NULL dereference, something like
>>>
>>> core_restore_failed:
>>> 	asm volatile(
>>> 		"movq %0, %%rsp				\n"
>>> 		"xorl %%eax, %%eax			\n"
>>> 		"movl %%eax, (%%eax)			\n"
>>> 		:
>>> 		: "r"(line)
>>> 		: );
>> This is not as good, as my version is. My version also shows error code.
>> Could you add it somehow?
>>
>>> or, as Andrew mentioned, extend BUG_ON_HANDLER. Hm?
>>>
>>> 	Cyrill
>> This BUG_ON_HANDLER relies on log fd is valid. It's not our case.
>>
> I thought about extending it this way -- split it to two parts,
> one to use without logfd (ie say BUG_RAISE macro which would simply
> dereference NULL and setup sp to line number) and second part which
> would be BUG_ON() with BUG_RAISE in body. This way we could use both
> -- for regular bugs BUG_ON and for this case BUG_RAISE.
>
> Stas, I somehow missed -- can't we use sys_exit here? With error
> code combined by line and error number. Hm?

1) Don't we going to trigger segmentation fault?
2) If we will use sys_exit() instead, which process will handle error code from 
your POW? From my it's "init", because crtools detached from restoring process 
already.


-- 
Best regards,
Stanislav Kinsbursky




More information about the CRIU mailing list