[CRIU] Re: Before we do the CRIU v0.1 release

Cyrill Gorcunov gorcunov at openvz.org
Fri Jul 20 02:30:23 EDT 2012


On Fri, Jul 20, 2012 at 07:56:08AM +0400, Pavel Emelyanov wrote:
> OK. Let's do a reshuffle of the patch
> 
> 1.
> 
> > +message arch_x86_entry {
> > +	required user_x86_regs_entry	gpregs		= 1;
> > +	required user_x86_fpregs_entry	fpregs		= 2;
> 
> I see only dump of this stuff. Why do we carry this at all?

Yes, we've no FPU context restore which is our underdone part
of crtools. It might be not critical at moment but better to
carry this bits.

> > +message core_entry {
> > +	enum march {
> > +		UNKNOWN		= 0;
> > +		X86_64		= 1;
> > +		X86_32		= 2;
> 
> Drop it for now. Only unknown and x64 should be left.

ok

> > +	required uint32			version		= 1;
> 
> Do we REALLY need this here? IMO it was a mistake to leave this field in
> the per-task core image. If we want the images version to be it should either
> sit in every file, or be in the root file -- pstree.img, or we should have
> a bounding image file titled description.img or smth like this. I'd vote
> for the last scenario.

I would vote for per-file version, right after magic number (since then
we can check version when someone does "show" -f). Anyway, i'll drop
this field.

> Thus the core entry consists of 2 items.
> 
> a) parts, that are individual for every thread
> b) part relevant for the process as a whole
> 
> That said, let's declare the message core_entry like this
> 
> message x864_64_thread_info {
> 	/* registers */
> 	/* clear tid address */
> }
> 
> message core_entry {
> 	required march march;
> 	optional x86_64_thread_info thread_info
> 	/* process-wide stuff */
> }

OK

> 
> 3. The master thread core entry isn't get __free_unpacked-ed. Leak?

It'll be freed by our self-VMA unmapper but sure better to call it
explicitly, thanks!

	Cyrill


More information about the CRIU mailing list