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

Proposed by Dimitri John Ledkov on 2012-12-12
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 2012-12-12 Approve on 2013-03-23
Review via email: mp+139450@code.launchpad.net
To post a comment you must log in.
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 on 2012-12-12

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

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.

Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'automatically_partition/replace/choices'
2--- automatically_partition/replace/choices 2012-10-16 15:24:12 +0000
3+++ automatically_partition/replace/choices 2012-12-12 13:17:22 +0000
4@@ -8,7 +8,7 @@
5 mountpoint="$(mktemp -d)"
6 cleanup () {
7 if [ -e $mountpoint ]; then
8- umount -l $mountpoint || true
9+ umount -l $mountpoint 2>/dev/null || true
10 rmdir $mountpoint || true
11 fi
12 for p in $cleanup_ro_partitions; do
13@@ -45,11 +45,10 @@
14 cd $dev
15 open_dialog PARTITIONS
16 while { read_line num id size type fs path name; [ "$id" ]; }; do
17- [ "$fs" != free ] || continue
18+ [ ! "$(echo "$fs" | egrep -s 'free|fat|ntfs|hfs|swap')" ] || continue
19 [ -e "$path" ] || continue
20 part=$dev//$id
21- t="$(blkid -o value -s TYPE $path)" || true
22- if [ -n "$t" ] && [ "$t" != "swap" ]; then
23+ if [ -n "$fs" ]; then
24 mounted=
25 if type grub-mount >/dev/null 2>&1 && \
26 grub-mount "$path" "$mountpoint" 2>/dev/null; then
27
28=== modified file 'automatically_partition/reuse/choices'
29--- automatically_partition/reuse/choices 2012-10-16 15:24:12 +0000
30+++ automatically_partition/reuse/choices 2012-12-12 13:17:22 +0000
31@@ -8,7 +8,7 @@
32 mountpoint="$(mktemp -d)"
33 cleanup () {
34 if [ -e $mountpoint ]; then
35- umount -l $mountpoint || true
36+ umount -l $mountpoint 2>/dev/null || true
37 rmdir $mountpoint || true
38 fi
39 for p in $cleanup_ro_partitions; do
40@@ -45,11 +45,10 @@
41 cd $dev
42 open_dialog PARTITIONS
43 while { read_line num id size type fs path name; [ "$id" ]; }; do
44- [ "$fs" != free ] || continue
45+ [ ! "$(echo "$fs" | egrep -s 'free|fat|ntfs|hfs|swap')" ] || continue
46 [ -e "$path" ] || continue
47 part=$dev//$id
48- t="$(blkid -o value -s TYPE $path)" || true
49- if [ -n "$t" ] && [ "$t" != "swap" ]; then
50+ if [ -n "$fs" ]; then
51 mounted=
52 if type grub-mount >/dev/null 2>&1 && \
53 grub-mount "$path" "$mountpoint" 2>/dev/null; then
54
55=== modified file 'debian/changelog'
56--- debian/changelog 2012-11-09 13:54:07 +0000
57+++ debian/changelog 2012-12-12 13:17:22 +0000
58@@ -1,3 +1,17 @@
59+partman-auto (105ubuntu2) UNRELEASED; urgency=low
60+
61+ * Reuse & Replace recipes:
62+ - Avoid using blkid, as it may sometimes hang indefinitely without
63+ timing out. See blkid bugs http://pad.lv/987718 and
64+ http://pad.lv/1044111. This change should address (LP: #1085991)
65+ - Skip mounting filesystems that ought to not be Ubuntu root
66+ filesystems (empty, swap, fat, ntfs, hfs). This also should speed up
67+ automatic partitioning page.
68+ - Silence warnings from umount in the clean-up (typically should
69+ already be unmounted)
70+
71+ -- Dmitrijs Ledkovs <dmitrij.ledkov@ubuntu.com> Wed, 12 Dec 2012 11:29:39 +0000
72+
73 partman-auto (105ubuntu1) raring; urgency=low
74
75 * Resynchronise with Debian. Remaining changes:

Subscribers

People subscribed via source and target branches