[CRIU] [PATCH v3 4/6] p.haul: add option to enable/disable pre-dumps

[Nikita Spiridonov]

On Thu, 2015-10-08 at 20:16 +0200, Adrian Reber wrote:
> On Thu, Oct 08, 2015 at 06:56:04PM +0400, Nikita Spiridonov wrote:
> > Acked-by: Nikita Spiridonov <nspiridonov at odin.com>
> > 
> > Acked, but it would be great to simplify --pre-dump/no-pre-dump logic in
> > further commits - e.g. repalce 0, 1 and 2 numbers with symbolic
> 
> Thanks. I will do a follow up patch to add symbolic constants.
> 
> > constants, move 'pre_dump' flag setting in
> > phaul_iter_worker.start_migration to separate method and so on.
> 
> I think this is already done in patch 6/6? Are you thinking about
> something else or is that change in 6/6 enough?
> 
> 		Adrian

I think 6/6 is enough for now.

In previous comment I mean phaul_iter_worker.start_migration at the
present moment actually contain all client side migration logic and we
have to move details of implementation to some methods/classes if it is
possible.

Even now phaul_iter_worker.start_migration method too complex and too
big (> 150 loc), have to split it in future.

> 
> > On Thu, 2015-10-08 at 14:06 +0000, Adrian Reber wrote:
> > > From: Adrian Reber <areber at redhat.com>
> > > 
> > > Although p.haul can now auto-detect if the used criu supports
> > > memory tracking this adds a command-line option to force
> > > pre-dumping on or off:
> > > 
> > >   --no-pre-dump        Force disable pre-dumps
> > >   --pre-dump           Force enable pre-dumps
> > > 
> > > Signed-off-by: Adrian Reber <areber at redhat.com>
> > > ---
> > >  p.haul                | 12 ++++++++++++
> > >  phaul/p_haul_iters.py | 18 ++++++++++++++----
> > >  2 files changed, 26 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/p.haul b/p.haul
> > > index e807c8f..6239709 100755
> > > --- a/p.haul
> > > +++ b/p.haul
> > > @@ -36,6 +36,18 @@ parser.add_argument("--log-file", help="Write logging messages to specified file
> > >  parser.add_argument("-j", "--shell-job",help ="Allow migration of shell jobs",
> > >  		    default=False, action='store_true')
> > >  
> > > +
> > > +parser.add_argument('--no-pre-dump', dest='pre_dump', action='store_const',
> > > +		    const=1, help='Force disable pre-dumps')
> > > +parser.add_argument('--pre-dump', dest='pre_dump', action='store_const',
> > > +		    const=2, help='Force enable pre-dumps')
> > > +
> > > +# If pre_dump is 0 then pre-dump autodetection should be used.
> > > +# If it is 1 or 2 it should be either enabled or disabled no
> > > +# matter if the kernel supports dirty page tracking or not.
> > > +
> > > +parser.set_defaults(pre_dump = 0)
> > > +
> > >  args = parser.parse_args()
> > >  
> > >  # Configure logging
> > > diff --git a/phaul/p_haul_iters.py b/phaul/p_haul_iters.py
> > > index ba96de3..96458b8 100644
> > > --- a/phaul/p_haul_iters.py
> > > +++ b/phaul/p_haul_iters.py
> > > @@ -42,6 +42,9 @@ class phaul_iter_worker:
> > >  		self.pid = self.htype.root_task_pid()
> > >  		self.fs.set_target_host(host[0])
> > >  
> > > +		# pre_dump 0 means auto-detection
> > > +		self.pre_dump = 0
> > > +
> > >  		logging.info("Setting up remote")
> > >  		self.target_host.setup(p_type)
> > >  
> > > @@ -55,6 +58,7 @@ class phaul_iter_worker:
> > >  		self.img.set_options(opts)
> > >  		self.htype.set_options(opts)
> > >  		self.__force = opts["force"]
> > > +		self.pre_dump = opts["pre_dump"]
> > >  
> > >  	def validate_cpu(self):
> > >  		logging.info("Checking CPU compatibility")
> > > @@ -87,11 +91,17 @@ class phaul_iter_worker:
> > >  		self.fs.set_work_dir(self.img.work_dir())
> > >  		self.fs.start_migration()
> > >  
> > > -		logging.info("Checking for Dirty Tracking")
> > > -		req = criu_req.make_dirty_tracking_req(self.htype, self.img)
> > > -		resp = self.criu_connection.send_req(req)
> > > +		if self.pre_dump == 0:
> > > +			# pre-dump auto-detection
> > > +			logging.info("Checking for Dirty Tracking")
> > > +			req = criu_req.make_dirty_tracking_req(self.htype, self.img)
> > > +			resp = self.criu_connection.send_req(req)
> > > +			pre_dump = resp.success
> > > +		elif self.pre_dump == 1:
> > > +			pre_dump = False
> > > +		else:
> > > +			pre_dump = True
> > >  
> > > -		pre_dump = False
> > >  		if resp.success:
> > >  			if resp.HasField('features'):
> > >  				if resp.features.HasField('mem_track'):