Merge lp:~xnox/partman-auto/avoid-hung-blkid into lp:~ubuntu-core-dev/partman-auto/ubuntu

Proposed by Dimitri John Ledkov
Status: Merged
Merged at revision: 646
Proposed branch: lp:~xnox/partman-auto/avoid-hung-blkid
Merge into: lp:~ubuntu-core-dev/partman-auto/ubuntu
Diff against target: 75 lines (+20/-8)
3 files modified
automatically_partition/replace/choices (+3/-4)
automatically_partition/reuse/choices (+3/-4)
debian/changelog (+14/-0)
To merge this branch: bzr merge lp:~xnox/partman-auto/avoid-hung-blkid
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+139450@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

On Wed, Dec 12, 2012 at 11:56:27AM -0000, Dmitrijs Ledkovs wrote:
> - [ "$fs" != free ] || continue
> + [ ! $(echo "$fs" | grep -s -e free -e fat -e ntfs -e hfs+ -e linux-swap) ] || continue

This is dangerously underquoted around the $(...), and I think I would
suggest using egrep -s 'foo|bar|baz' rather than grep -s -e foo -e bar
-e baz ... it's just a bit more standard in d-i code. (And, in that
case, be careful to escape the + in hfs+.)

> + - Skip mounting filesystems that ought to not be Ubuntu root
> + filesystems (empty, swap, fat, ntfs, hfs). This also should speed up
> + automatic partitioning page.

hfs and hfs+ are different filesystems, and at the moment you only
exclude hfs+.

Any reason not to exclude hfs and hfsx as well?

647. By Dimitri John Ledkov

* Exclude all types of hfs
* Use egrep
* Quote the test safely

Revision history for this message
Dimitri John Ledkov (xnox) wrote :

Yes, other types of hfs should also be excluded.
Switched to egrep, re-read word-splitting guide and hopefully the quoting is correct now.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'automatically_partition/replace/choices'
--- automatically_partition/replace/choices 2012-10-16 15:24:12 +0000
+++ automatically_partition/replace/choices 2012-12-12 13:17:22 +0000
@@ -8,7 +8,7 @@
8mountpoint="$(mktemp -d)"8mountpoint="$(mktemp -d)"
9cleanup () {9cleanup () {
10 if [ -e $mountpoint ]; then10 if [ -e $mountpoint ]; then
11 umount -l $mountpoint || true11 umount -l $mountpoint 2>/dev/null || true
12 rmdir $mountpoint || true12 rmdir $mountpoint || true
13 fi13 fi
14 for p in $cleanup_ro_partitions; do14 for p in $cleanup_ro_partitions; do
@@ -45,11 +45,10 @@
45 cd $dev45 cd $dev
46 open_dialog PARTITIONS46 open_dialog PARTITIONS
47 while { read_line num id size type fs path name; [ "$id" ]; }; do47 while { read_line num id size type fs path name; [ "$id" ]; }; do
48 [ "$fs" != free ] || continue48 [ ! "$(echo "$fs" | egrep -s 'free|fat|ntfs|hfs|swap')" ] || continue
49 [ -e "$path" ] || continue49 [ -e "$path" ] || continue
50 part=$dev//$id50 part=$dev//$id
51 t="$(blkid -o value -s TYPE $path)" || true51 if [ -n "$fs" ]; then
52 if [ -n "$t" ] && [ "$t" != "swap" ]; then
53 mounted=52 mounted=
54 if type grub-mount >/dev/null 2>&1 && \53 if type grub-mount >/dev/null 2>&1 && \
55 grub-mount "$path" "$mountpoint" 2>/dev/null; then54 grub-mount "$path" "$mountpoint" 2>/dev/null; then
5655
=== modified file 'automatically_partition/reuse/choices'
--- automatically_partition/reuse/choices 2012-10-16 15:24:12 +0000
+++ automatically_partition/reuse/choices 2012-12-12 13:17:22 +0000
@@ -8,7 +8,7 @@
8mountpoint="$(mktemp -d)"8mountpoint="$(mktemp -d)"
9cleanup () {9cleanup () {
10 if [ -e $mountpoint ]; then10 if [ -e $mountpoint ]; then
11 umount -l $mountpoint || true11 umount -l $mountpoint 2>/dev/null || true
12 rmdir $mountpoint || true12 rmdir $mountpoint || true
13 fi13 fi
14 for p in $cleanup_ro_partitions; do14 for p in $cleanup_ro_partitions; do
@@ -45,11 +45,10 @@
45 cd $dev45 cd $dev
46 open_dialog PARTITIONS46 open_dialog PARTITIONS
47 while { read_line num id size type fs path name; [ "$id" ]; }; do47 while { read_line num id size type fs path name; [ "$id" ]; }; do
48 [ "$fs" != free ] || continue48 [ ! "$(echo "$fs" | egrep -s 'free|fat|ntfs|hfs|swap')" ] || continue
49 [ -e "$path" ] || continue49 [ -e "$path" ] || continue
50 part=$dev//$id50 part=$dev//$id
51 t="$(blkid -o value -s TYPE $path)" || true51 if [ -n "$fs" ]; then
52 if [ -n "$t" ] && [ "$t" != "swap" ]; then
53 mounted=52 mounted=
54 if type grub-mount >/dev/null 2>&1 && \53 if type grub-mount >/dev/null 2>&1 && \
55 grub-mount "$path" "$mountpoint" 2>/dev/null; then54 grub-mount "$path" "$mountpoint" 2>/dev/null; then
5655
=== modified file 'debian/changelog'
--- debian/changelog 2012-11-09 13:54:07 +0000
+++ debian/changelog 2012-12-12 13:17:22 +0000
@@ -1,3 +1,17 @@
1partman-auto (105ubuntu2) UNRELEASED; urgency=low
2
3 * Reuse & Replace recipes:
4 - Avoid using blkid, as it may sometimes hang indefinitely without
5 timing out. See blkid bugs http://pad.lv/987718 and
6 http://pad.lv/1044111. This change should address (LP: #1085991)
7 - Skip mounting filesystems that ought to not be Ubuntu root
8 filesystems (empty, swap, fat, ntfs, hfs). This also should speed up
9 automatic partitioning page.
10 - Silence warnings from umount in the clean-up (typically should
11 already be unmounted)
12
13 -- Dmitrijs Ledkovs <dmitrij.ledkov@ubuntu.com> Wed, 12 Dec 2012 11:29:39 +0000
14
1partman-auto (105ubuntu1) raring; urgency=low15partman-auto (105ubuntu1) raring; urgency=low
216
3 * Resynchronise with Debian. Remaining changes:17 * Resynchronise with Debian. Remaining changes:

Subscribers

People subscribed via source and target branches