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

[Andrew Vagin]

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?
> 
> 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.dirname(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_dest))
>  			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")
> -- 
> 2.7.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu