[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#468114: Loopback file system support.



On Wed, 27 Feb 2008, Luke Yelavich wrote:

> Attached is a patch to allow an initramfs to use a root filesystem as a
> loop device, on top of another file system. A good example and use of
> this code is for Ubuntu's Windows installer, which creates a loopback
> filesystem on top of the Windows NTFS partition. The Windows partition
> is mounted, then the loopback filesystem is mounted from the Windows
> partition. The patch applies against git head.

ack cool,
see specific critics below.

> diff -urN initramfs-tools/init initramfs-tools.new/init
> --- initramfs-tools/init	2008-02-26 10:36:10.347676705 +1100
> +++ initramfs-tools.new/init	2008-02-26 13:50:26.459676705 +1100
> @@ -85,6 +85,15 @@
>  			;;
>  		esac
>  		;;
> +	loop=*)
> +		LOOP="${x#loop=}"
> +		;;
> +	loopflags=*)
> +		LOOPFLAGS="-o ${x#loopflags=}"
> +		;;
> +	loopfstype=*)
> +		LOOPFSTYPE="${x#loopfstype=}"
> +		;;
>  	nfsroot=*)
>  		NFSROOT="${x#nfsroot=}"
>  		;;

don't see them in kernel-parameter.txt but this namespace seems sane,
so ack from me.

> diff -urN initramfs-tools/initramfs-tools.8 initramfs-tools.new/initramfs-tools.8
> --- initramfs-tools/initramfs-tools.8	2008-02-26 13:48:37.435676705 +1100
> +++ initramfs-tools.new/initramfs-tools.8	2008-02-26 13:49:26.251676705 +1100
<snipp manpage diff>
ok too.

> diff -urN initramfs-tools/scripts/local initramfs-tools.new/scripts/local
> --- initramfs-tools/scripts/local	2008-02-26 13:48:37.423676705 +1100
> +++ initramfs-tools.new/scripts/local	2008-02-26 13:49:26.239676705 +1100
> @@ -101,7 +101,8 @@
>  	run_scripts /scripts/local-premount
>  	[ "$quiet" != "y" ] && log_end_msg
>  
> -	if [ "${readonly}" = "y" ]; then
> +	if [ ${readonly} = y ] && \
> +	   ([ -z "$LOOP" ] || [ "${FSTYPE#ntfs}" = "$FSTYPE" ]); then
>  		roflag=-r
>  	else
>  		roflag=-w

hmm at first sight i found a bit strange that you use not a different boot
method for the loop fs stuff, like setting BOOT=loop if loop bootarg is
given and then just having scripts/loop.  but on the contrary i agree that
it is on a local fs almost identical path and would lead otherwise to
code duplication of the local boot scripts and thus fine with me.

> @@ -114,6 +115,43 @@
>  	# Mount root
>  	mount ${roflag} -t ${FSTYPE} ${ROOTFLAGS} ${ROOT} ${rootmnt}
>  
> +	if [ "$LOOP" ]; then
> +		mkdir -p /host
> +		mount -o move ${rootmnt} /host
> +
> +		while [ ! -e "/host/${LOOP#/}" ]; do
> +			panic "ALERT!  /host/${LOOP#/} does not exist.  Dropping to a shell!"
> +		done

that check should go up and check that aboves mount doesn't fail,
otherwise i don't see a possibility of that happening.

> +
> +		# Get the loop filesystem type if not set
> +		if [ -z "${LOOPFSTYPE}" ]; then
> +			eval $(fstype < "/host/${LOOP#/}")
> +		else
> +			FSTYPE="${LOOPFSTYPE}"
> +		fi
> +		if [ "$FSTYPE" = "unknown" ] && [ -x /lib/udev/vol_id ]; then
> +			FSTYPE=$(/lib/udev/vol_id -t "/host/${LOOP#/}")
> +			[ -z "$FSTYPE" ] && FSTYPE="unknown"
> +		fi

no thanks
duplication use get_fstype

> +
> +		if [ ${readonly} = y ]; then
> +			roflag=-r
> +		else
> +			roflag=-w
> +		fi
> +
> +		# FIXME This has no error checking
> +		modprobe loop
> +		modprobe ${FSTYPE}
> +
> +		# FIXME This has no error checking
> +		mount ${roflag} -o loop -t ${FSTYPE} ${LOOPFLAGS} "/host/${LOOP#/}" ${rootmnt}

yep that was the FIXME i was talking about above.
please add mount returnvalue check here too.
afais klibc mount returns sensible error codes.

> +
> +		if [ -d ${rootmnt}/host ]; then
> +			mount -o move /host ${rootmnt}/host
> +		fi
> +	fi
> +
>  	[ "$quiet" != "y" ] && log_begin_msg "Running /scripts/local-bottom"
>  	run_scripts /scripts/local-bottom
>  	[ "$quiet" != "y" ] && log_end_msg


thanks for the submission.

-- 
maks



Reply to: