[CRIU] Re: [PATCH v2] parasite-syscall: some cleanup

[Cyrill Gorcunov]

On Tue, Feb 21, 2012 at 08:55:31PM +0400, Kinsbursky Stanislav wrote:
> 21.02.2012 20:47, Cyrill Gorcunov пишет:
> >On Tue, Feb 21, 2012 at 07:19:06PM +0300, Kinsbursky Stanislav wrote:
> >>v2: command is set prior to parasite_execute() call now
> >>
> >>Removed redundant on-stack variables typeof parasite_status_t and memcpy
> >>between them and parasite_ctl->addr_args object. Ando a lot of cleanup done
> >>around.
> >>
> >>Signed-off-by: Stanislav Kinsbursky<skinsbursky at openvz.org>
> >>---
> >I thought it'll be a good idea, but after looking into final code
> >I fear it's not.
> >
> >Lets clean it up step-by-step (I'm enumerating what we have now
> >and some ideas so we would choose ones which are suitable for us)
> >
> >1) Every parasite argument passed must have a header with
> >
> >	parasite_status_t       status;
> >
> >    on top, where the parasite will write the result of operations
> >    on return (if there some code where we do not write the result
> >    I think we should add it).
> 
> There are few parasite commands, which doesn't use this arguments at
> all. What about them?
> But all others, which use - have status on top (if I'm not mistaken).
> 
> >2) If parasite finished without error we should use some macro such
> >    as say
> >
> >#define SET_PARASITE_STATUS_SUCCESS(st)			\
> >	do {						\
> >		(st)->ret	= 0,			\
> >		(st)->sys_ret	= 0,			\
> >		(st)->line	= __LINE__;		\
> >	} while (0)
> >
> 
> I had this idea. But it's useless without implementing of passing
> arguments in every command.

Better to pass poisoned arguments even to commands which do not
require them. That was the point. Unified approach to any command
parasite can handle. Without exceptions.

> 
> >3) Parasite alway should pick up arguments. Explicitly. If there
> >    a command which requires no arguments -- we must zap arguments space with
> >    some predefined poisoned values, so we will be able to catch cases
> >    when parasite is called with some command but for some reason caller
> >    forgot to pass new arguments.
> >
> 
> 1) What "pick up" means in this context?

I meant the parasite should check for poisoned arguments and scream
if such argument found on command which expects for valid data.

> 2) No objections about poisoning before call. But I don't like
> on-stack variable copying. Let's use final destination instead.

Stas, as I said I prefer to have on stack copying than code
snippets posted here and there. It's really not the case where
we should care about performance. The arguments size in most
cases are not big and perf penalty is very-very small.

	Cyrill