[CRIU] [PATCH 00/32] tools, cpt2 introduction, v1

Cyrill Gorcunov gorcunov at openvz.org
Mon Apr 1 07:11:13 EDT 2013


On Mon, Apr 01, 2013 at 02:49:57PM +0400, Pavel Emelyanov wrote:
> On 04/01/2013 02:43 PM, Cyrill Gorcunov wrote:
> > On Mon, Apr 01, 2013 at 02:13:59PM +0400, Pavel Emelyanov wrote:
> >> On 04/01/2013 02:05 PM, Cyrill Gorcunov wrote:
> >>> On Mon, Apr 01, 2013 at 01:52:40PM +0400, Pavel Emelyanov wrote:
> >>>>>  57 files changed, 8360 insertions(+), 3 deletions(-)
> >>>>
> >>>> I gave up on patch #9. How much of these 8.3K lines are just copy-n-paste
> >>>> from original crtools/ code?
> >>>
> >>> this is abstract fdset engine which is not bound to anything else (i did
> >>> think of moving this code to crtools and reuse it in cpt2).
> >>>
> >>> thus the things which look similar
> >>>
> >>>  - image engine
> >>>  - protobuf
> >>>  - fdset
> >>
> >> Please, use stuff from crtools/ next time. Presumably it's OK just to link
> >> respective .o files into conversion binary.
> > 
> > Pavel, I can't. for example our templates for criu files do have links
> > to "show" routines, which we don't have in cpt2. fdset code is suffering
> > from same problem, crtools code is too hooked into the rest of crtools codebase.
> 
> This excuses the .c code reuse, but not constants and .h helpers.

Pavel, I've reused anything I can (except bug.h and compiler.h, which I
preferred to copy just for debug purpose).

Which headers you claim I can use? Lets take image.h for example. It has
CR_FD_INVENTORY and such definitions, which are in crtools code declared
in crtools.h. But in crtools it's a big header with own #include directives
and lot of additional "externs" (and what is worse, not externs) definitions,
inlines and such, This is damn inconvenient to work with.

The solution is to move constants out of rest of crtools.h (and you might
note that I've added FIXME's in my headers, pointing out that it's code
duplication which is to be resolved). Still at this early point of project
I tried to not touch crtools code completely.

In easier cases, such as ctools/include/magic.h -- I happily include the
header without any modifications.

So don't blame me here, I know it's somehow ugly but it's fine for first
series. I'll resolve this problem before v2 set (patching crtools code).


More information about the CRIU mailing list