[Devel] Re: [PATCH 2/2] c/r: define s390-specific checkpoint-restart code (v5)

[Serge E. Hallyn]

Quoting Dan Smith (danms at us.ibm.com):
> >> +struct cr_hdr_cpu {
> >> +	__u64 args[1];
> 
> SH> Dave wanted this to not be an array, right?
> 
> I think he was okay with it since it matched what is in the rest of
> the s390 code.  I think that the use of the CR_COPY() macros makes it
> nicer to have matching types as closely as possible.

Ok with me.  Dave can speak up if he needs to :)

> >> +	union {
> >> +		float f;
> >> +		double d;
> >> +		__u64 ui;
> 
> SH> Since this is a union, and you don't deal with its members but
> SH> just memcpy it, why not just change it to
> 
> That's a fair argument, although I was thinking that there was some
> expectation of being able to include this in userspace at some point
> to inspect the contents of the CR stream.  The #ifdef __KERNEL__ at
> the top of the file makes me think that's true.  If so, does it make
> sense to leave this as-is for easier inspection of the contents?

But what is userspace supposed to gain from seeing that in the
headers?  No matter how many other ways we represent the data
containined in a fprs union, all we know based on the checkpoint
image is what the size of NUM_FPRS*sizeof(*fprs) is, right?

Or do you mean the programmers will see that and be able to tell
more easiliy where in ptrace.h the corresponding union is?

-serge
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers