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

growparts from cloud-utils on bpo has a small sfdisk related bug



Hi - it appears that growparts in the backported cloud-utils for jessie
has a bug with sfdisk - it's related to the sfdisk before/after version
2.26 problem, but is a bit more of a corner case:

The old sfdisk which gives CHS style output can't handle partitions or
devices > 2 TiB in size: The report I received was as follows:

  “…it seems it has not fixed one of the key issues with the version
   of growpart which is that when you create a physical disk >2TB (with
   an MBR partition table), the growpart script will try to grow the
   partition beyond 2TB and overwrite itself. Therefore, if you created
   a disk that was 2049GB, you would end up with a 1GB partition because
   the growpart would overflow the partition and wrap back around. This of
   course will brick your install.”

The reporter also found the fix, which was in an unreleased version of
growpart here (rev 250 is the one with the fix):

  http://bazaar.launchpad.net/~cloud-utils-dev/cloud-utils/trunk/changes/262?start_revid=282

I've added the fix to a patch in the debian packaging from bpo and
attached the debdiff... is this fix acceptable and could we get it into
the debian package in sid, and from there to the bpo jessie package?

If not, what changes would you like to the fix?
diff -Nru cloud-utils-0.27/debian/changelog cloud-utils-0.27/debian/changelog
--- cloud-utils-0.27/debian/changelog	2016-02-11 13:08:43.000000000 +0000
+++ cloud-utils-0.27/debian/changelog	2016-05-12 15:39:50.000000000 +0100
@@ -1,3 +1,14 @@
+cloud-utils (0.27-2~bpo8+2) jessie-backports; urgency=medium
+
+  * If sfdisk returns CYL/HEAD/SECTOR data then it does not support
+    partitions larger than 2 TiB - if such a partition is requested
+    growpart should restrict itself to the supported limit.
+  * sfdisk fix imported from:
+    http://bazaar.launchpad.net/~cloud-utils-dev/cloud-utils/trunk/revision/250#bin/growpart
+  * Improve quoting of arguments in debug output of rq().
+
+ -- Vivek Das Mohapatra <vivek@collabora.com>  Thu, 12 May 2016 15:39:50 +0100
+
 cloud-utils (0.27-2~bpo8+1) jessie-backports; urgency=medium
 
   * Rebuild for jessie-backports.
diff -Nru cloud-utils-0.27/debian/patches/growpart-over-2TB-with-mbr-fix.patch cloud-utils-0.27/debian/patches/growpart-over-2TB-with-mbr-fix.patch
--- cloud-utils-0.27/debian/patches/growpart-over-2TB-with-mbr-fix.patch	1970-01-01 01:00:00.000000000 +0100
+++ cloud-utils-0.27/debian/patches/growpart-over-2TB-with-mbr-fix.patch	2016-05-10 19:32:42.000000000 +0100
@@ -0,0 +1,94 @@
+--- a/bin/growpart
++++ b/bin/growpart
+@@ -185,7 +185,7 @@
+ }
+ 
+ mbr_resize() {
+-	RESTORE_HUMAN="${TEMP_D}/recovery"
++	local humanpt="${TEMP_D}/recovery"
+ 	MBR_BACKUP="${TEMP_D}/backup"
+ 
+ 	local change_out=${TEMP_D}/change.out
+@@ -194,6 +194,7 @@
+ 	local dump_mod=${TEMP_D}/dump.mod
+ 	local tmp="${TEMP_D}/tmp.out"
+ 	local err="${TEMP_D}/err.out"
++	local mbr_max_512="4294967296"
+ 
+ 	local _devc cyl _w1 heads _w2 sectors _w3 tot dpart
+ 	local pt_start pt_size pt_end max_end new_size change_info
+@@ -209,7 +210,7 @@
+ 		{
+ 			echo "## sfdisk --dump ${DISK}"
+ 			cat "${dump_out}"
+-		}  >"${RESTORE_HUMAN}"
++		}  >"${humanpt}"
+ 	else
+ 		# --show-pt-geometry outputs something like
+ 		#     /dev/sda: 164352 cylinders, 4 heads, 32 sectors/track
+@@ -219,21 +220,27 @@
+ 			fail "failed to get CHS from ${DISK}"
+ 
+ 		tot=$((${cyl}*${heads}*${sectors}))
++
+ 		debug 1 "geometry is ${MBR_CHS}. total size=${tot}"
++		[ "$tot" -gt "$mbr_max_512" ] &&
++		    debug 1 "WARN: disk is larger than 2TB. additional space will go unused."
+ 
+ 		rqe sfd_dump sfdisk ${MBR_CHS} --unit=S --dump "${DISK}" \
+ 			>"${dump_out}" ||
+ 			fail "failed to dump sfdisk info for ${DISK}"
++		RESTORE_HUMAN="$dump_out"
++
+ 		{
+ 			echo "## sfdisk ${MBR_CHS} --unit=S --dump ${DISK}"
+ 			cat "${dump_out}"
+-		}  >"${RESTORE_HUMAN}"
++		}  >"$humanpt"
+ 	fi
+ 
+ 
+ 	[ $? -eq 0 ] || fail "failed to save sfdisk -d output"
++	RESTORE_HUMAN="$humanpt"
+ 
+-	debugcat 1 "${RESTORE_HUMAN}"
++	debugcat 1 "$humanpt"
+ 
+ 	sed -e 's/,//g; s/start=/start /; s/size=/size /' "${dump_out}" \
+ 		>"${dump_mod}" ||
+@@ -263,6 +270,10 @@
+ 		[ -n "${max_end}" ] ||
+ 		fail "failed to get max_end for partition ${PART}"
+ 
++	if [ "$max_end" -gt "$mbr_max_512" ]; then
++		max_end=$mbr_max_512;
++	fi
++
+ 	debug 1 "max_end=${max_end} tot=${tot} pt_end=${pt_end}" \
+ 		"pt_start=${pt_start} pt_size=${pt_size}"
+ 	[ $((${pt_end})) -eq ${max_end} ] &&
+@@ -354,6 +365,7 @@
+ 	# used in case something goes wrong and human interaction is required
+ 	# to revert any changes.
+ 	rqe sgd_info sgdisk "--info=${PART}" --print "${DISK}" >"${pt_info}" ||
++		fail "${dev}: failed to dump original sgdisk info"
+ 	RESTORE_HUMAN="${pt_info}"
+ 
+ 	debug 1 "$dev: original sgdisk info:"
+@@ -476,7 +488,15 @@
+ 	local label="$1" ret="" efile=""
+ 	efile="$TEMP_D/$label.err"
+ 	shift;
+-	debug 2 "running[$label][$_capture]" "$@"
++
++	local cmd="" x=""
++	for x in "$@"; do
++		[ "${x#* }" != "$x" -o "${x#* \"}" != "$x" ] && x="'$x'"
++		cmd="$cmd $x"
++	done
++	cmd=${cmd# }
++
++	debug 2 "running[$label][$_capture]" "$cmd"
+ 	if [ "${_capture}" = "erronly" ]; then
+ 		"$@" 2>"$TEMP_D/$label.err"
+ 		ret=$?
diff -Nru cloud-utils-0.27/debian/patches/series cloud-utils-0.27/debian/patches/series
--- cloud-utils-0.27/debian/patches/series	2016-02-11 13:04:22.000000000 +0000
+++ cloud-utils-0.27/debian/patches/series	2016-05-10 18:15:24.000000000 +0100
@@ -1,2 +1,3 @@
 do-not-use-EXIT-when-trapping.patch
 also-support-sfdisk-2.26-and-higher.patch
+growpart-over-2TB-with-mbr-fix.patch

Reply to: