[CRIU] [RFC PATCH 01/21] zdtm: zdtm.py: elaborate file copying logic for ns tests

[Ivan Shapovalov]

On 2016-02-25 at 12:01 -0800, Andrew Vagin wrote:
> On Fri, Feb 19, 2016 at 05:50:24PM +0300, Ivan Shapovalov wrote:
> > 
> > From: Ivan Shapovalov <intelfx100 at gmail.com>
> > 
> > When copying files (tests' deps and supplementary binaries) to the
> > "chroot",
> > do not blindly copy them using provided pathes (like
> > "/sbin/iptables").
> > Instead, walk the path manually and replicate all encountered
> > symlinks in
> > the "chroot".
> > 
> > This helps to support setups like Arch, where /{,s}bin are links to
> > /usr/bin,
> > and only /usr/bin is in $PATH.
> Why is it not enough to call os.path.realpath?

Consider the following example:

- /bin, /sbin are symlinks to /usr/bin
- /usr/bin/iptables is a symlink to xtables-multi
- $PATH is /usr/bin (just this one directory)
- test says /sbin/iptables in deps

Just realpath will copy /usr/bin/xtables-multi and there will be no
iptables at all.

realpath plus naïve symlink will copy /usr/bin/xtables-multi and make
/sbin/iptables a link to it, and iptables will be missing from $PATH.

-- 
Ivan Shapovalov / intelfx /

> > Signed-off-by: Ivan Shapovalov <intelfx at intelfx.name>
> > ---
> >  test/zdtm.py | 72
> > ++++++++++++++++++++++++++++++++++++++++++++++++++----------
> >  1 file changed, 61 insertions(+), 11 deletions(-)
> > 
> > diff --git a/test/zdtm.py b/test/zdtm.py
> > index 01b20e7..79b7a95 100755
> > --- a/test/zdtm.py
> > +++ b/test/zdtm.py
> > @@ -105,8 +105,15 @@ class host_flavor:
> >  	def clean():
> >  		pass
> >  
> > +def splitall(path, tail = []):
> > +	(dir, name) = os.path.split(path)
> > +	if dir and dir != path:
> > +		return splitall(dir, [name] + tail)
> > +	else:
> > +		return [dir or name] + tail
> > +
> >  class ns_flavor:
> > -	__root_dirs = ["/bin", "/sbin", "/etc", "/lib", "/lib64",
> > "/dev", "/dev/pts", "/dev/net", "/tmp", "/usr", "/proc"]
> > +	__root_dirs = ["/etc", "/dev", "/dev/pts", "/dev/net",
> > "/tmp", "/proc"]
> >  
> >  	def __init__(self, opts):
> >  		self.name = "ns"
> > @@ -115,18 +122,64 @@ class ns_flavor:
> >  		self.root = make_tests_root()
> >  		self.root_mounted = False
> >  
> > -	def __copy_one(self, fname):
> > -		tfname = self.root + fname
> > -		if not os.access(tfname, os.F_OK):
> > +	def __copy_one(self, src):
> > +		#
> > +		# let's copy the whole subhierarchy, accounting
> > neatly for cases like /sbin/iptables on Arch
> > +		# (where /sbin -> /usr/bin, /usr/bin/iptables ->
> > /usr/bin/xtables-multi, and only /usr/bin is in $PATH).`
> > +		#
> > +
> > +		src_bits = splitall(src)
> > +		if src_bits[0] != "/":
> > +			raise test_fail_exc("Deps check (%s is not
> > absolute)" % src)
> > +		del src_bits[0]
> > +
> > +		src_dir_so_far = "/"
> > +
> > +		# copy all directories and links accurately
> > +		for bit_src in src_bits:
> > +			src_so_far = os.path.join(src_dir_so_far,
> > bit_src)
> > +			dest_so_far = self.root + src_so_far
> > +
> > +			# handle chained symlinks
> > +			while os.path.islink(src_so_far):
> > +				bit_dst = os.readlink(src_so_far)
> > +				# relativize symlink target
> > +				bit_dst_rel =
> > os.path.relpath(bit_dst, src_dir_so_far) if os.path.isabs(bit_dst)
> > else bit_dst
> Can you avoid long lines? Our coding style says that a length should
> be
> shorter than 80 symbols and it must be shorter than 120.
> > 
> > +
> > +				try:
> > +					os.symlink(bit_dst_rel,
> > dest_so_far)
> > +				except:
> > +					pass
> I don't like an idea to ignore all exceptions. Can we ignore EEXIST
> only?
> > 
> > +
> > +				src_so_far =
> > os.path.join(src_dir_so_far, bit_dst)
> > +				dest_so_far = self.root +
> > src_so_far
> > +				src_dir_so_far =
> > os.path.dirname(src_so_far)
> > +				try:
> > +					os.makedirs(os.path.dirnam
> > e(dest_so_far))
> > +				except:
> > +					pass
> > +
> > +			if os.path.isdir(src_so_far):
> > +				src_dir_so_far = src_so_far
> > +				try:
> > +					os.makedirs(dest_so_far)
> > +				except:
> > +					pass
> > +			else:
> > +				break
> > +
> > +		# finally, copy the file itself
> > +		real_dest = self.root + os.path.realpath(src)
> > +		if not os.access(real_dest, os.F_OK):
> >  			# Copying should be atomic as tests can be
> >  			# run in parallel
> >  			try:
> > -				os.makedirs(self.root +
> > os.path.dirname(fname))
> > +				os.makedirs(os.path.dirname(real_d
> > est))
> >  			except:
> >  				pass
> > -			dst = tempfile.mktemp(".tso", "",
> > self.root + os.path.dirname(fname))
> > -			shutil.copy2(fname, dst)
> > -			os.rename(dst, tfname)
> > +			temp = tempfile.mktemp(".tso", "",
> > os.path.dirname(real_dest))
> > +			shutil.copy2(src, temp)
> > +			os.rename(temp, real_dest)
> >  
> >  	def __copy_libs(self, binary):
> >  		ldd = subprocess.Popen(["ldd", binary], stdout =
> > subprocess.PIPE)
> > @@ -164,9 +217,6 @@ class ns_flavor:
> >  			os.mkdir(self.root + dir)
> >  			os.chmod(self.root + dir, 0777)
> >  
> > -		for ldir in [ "/bin", "/sbin", "/lib", "/lib64" ]:
> > -			os.symlink(".." + ldir, self.root + "/usr"
> > + ldir)
> > -
> >  		self.__mknod("tty", os.makedev(5, 0))
> >  		self.__mknod("null", os.makedev(1, 3))
> >  		self.__mknod("net/tun")
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.openvz.org/pipermail/criu/attachments/20160226/0acdefd9/attachment.sig>