Merge ~paelzer/ubuntu/+source/mdadm:lp-1811795-1811228-cosmic into ubuntu/+source/mdadm:ubuntu/cosmic-devel

Proposed by Christian Ehrhardt  on 2019-04-10
Status: Merged
Approved by: Christian Ehrhardt  on 2019-04-23
Approved revision: 947b155c4c1b8350ed4121be1c983692586b9465
Merged at revision: 947b155c4c1b8350ed4121be1c983692586b9465
Proposed branch: ~paelzer/ubuntu/+source/mdadm:lp-1811795-1811228-cosmic
Merge into: ubuntu/+source/mdadm:ubuntu/cosmic-devel
Diff against target: 227 lines (+199/-0)
4 files modified
debian/changelog (+9/-0)
debian/patches/lp-1811228-imsm-Block-volume-creation-with-empty-name.patch (+50/-0)
debian/patches/lp-1811795-remove-external-arrays-and-devices.patch (+138/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
Andreas Hasenack 2019-04-10 Approve on 2019-04-22
Canonical Server Team 2019-04-10 Pending
Canonical Server packageset reviewers 2019-04-10 Pending
Review via email: mp+365795@code.launchpad.net
To post a comment you must log in.
Christian Ehrhardt  (paelzer) wrote :

PPA is available at: https://launchpad.net/~paelzer/+archive/ubuntu/bug-1811228-1811795-mdadm-imsm

Testing needs special IMSM HW which I don't have, lets check how active the reporters are in helping with that.

Andreas Hasenack (ahasenack) wrote :

+1

- sru template ok
- changelog ok
- sru version ok
- patches identical to upstream commits
- reporter confirmed ppa fixes the issue

review: Approve
Christian Ehrhardt  (paelzer) wrote :

Thanks for the review - I tagged, pushed and uploaded it now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 14f9660..74a8a67 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,12 @@
6+mdadm (4.1~rc1-4ubuntu1.1) cosmic; urgency=medium
7+
8+ * d/p/lp-1811228-imsm-Block-volume-creation-with-empty-name.patch: avoid
9+ creating whitespaced or empty names (LP: #1811228)
10+ * d/p/lp-1811795-remove-external-arrays-and-devices.patch: fix removal of
11+ failing disks with external metadata (LP: #1811795)
12+
13+ -- Christian Ehrhardt <christian.ehrhardt@canonical.com> Wed, 10 Apr 2019 16:14:17 +0200
14+
15 mdadm (4.1~rc1-4ubuntu1) cosmic; urgency=medium
16
17 * Ship finalrd hook.
18diff --git a/debian/patches/lp-1811228-imsm-Block-volume-creation-with-empty-name.patch b/debian/patches/lp-1811228-imsm-Block-volume-creation-with-empty-name.patch
19new file mode 100644
20index 0000000..00fffd2
21--- /dev/null
22+++ b/debian/patches/lp-1811228-imsm-Block-volume-creation-with-empty-name.patch
23@@ -0,0 +1,50 @@
24+From 9bd99a90e110bd39beaa539cb44a70ee3264a270 Mon Sep 17 00:00:00 2001
25+From: Roman Sobanski <roman.sobanski@intel.com>
26+Date: Fri, 10 Aug 2018 14:20:35 +0200
27+Subject: [PATCH] imsm: Block volume creation with empty name
28+
29+There is a possibility to create a RAID with empty name. Block it. Also
30+remove trailing and leading whitespaces from given name.
31+
32+Signed-off-by: Roman Sobanski <roman.sobanski@intel.com>
33+Signed-off-by: Jes Sorensen <jsorensen@fb.com>
34+
35+Origin: upstream, https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=9bd99a90e110bd39beaa539cb44a70ee3264a270
36+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1811228
37+Last-Update: 2019-04-10
38+
39+---
40+ super-intel.c | 14 +++++++++++++-
41+ 1 file changed, 13 insertions(+), 1 deletion(-)
42+
43+diff --git a/super-intel.c b/super-intel.c
44+index f011a31f..d3d256a4 100644
45+--- a/super-intel.c
46++++ b/super-intel.c
47+@@ -5285,10 +5285,22 @@ static int check_name(struct intel_super *super, char *name, int quiet)
48+ {
49+ struct imsm_super *mpb = super->anchor;
50+ char *reason = NULL;
51++ char *start = name;
52++ size_t len = strlen(name);
53+ int i;
54+
55+- if (strlen(name) > MAX_RAID_SERIAL_LEN)
56++ if (len > 0) {
57++ while (isspace(start[len - 1]))
58++ start[--len] = 0;
59++ while (*start && isspace(*start))
60++ ++start, --len;
61++ memmove(name, start, len + 1);
62++ }
63++
64++ if (len > MAX_RAID_SERIAL_LEN)
65+ reason = "must be 16 characters or less";
66++ else if (len == 0)
67++ reason = "must be a non-empty string";
68+
69+ for (i = 0; i < mpb->num_raid_devs; i++) {
70+ struct imsm_dev *dev = get_imsm_dev(super, i);
71+--
72+2.17.1
73+
74diff --git a/debian/patches/lp-1811795-remove-external-arrays-and-devices.patch b/debian/patches/lp-1811795-remove-external-arrays-and-devices.patch
75new file mode 100644
76index 0000000..f7f15ab
77--- /dev/null
78+++ b/debian/patches/lp-1811795-remove-external-arrays-and-devices.patch
79@@ -0,0 +1,138 @@
80+From cb8f5371352f6c16af5aab8a40861e13aa50fc2b Mon Sep 17 00:00:00 2001
81+From: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
82+Date: Fri, 3 Aug 2018 09:41:50 +0200
83+Subject: [PATCH] Incremental: remove external arrays and devices correctly
84+
85+Kernel returns EBUSY when device fail invokes array fail.
86+In external metadata if kernel returns it, mdadm doesn't stop member
87+arrays but it will try to stop container directly. It fails because
88+container still has working arrays, so udev remove is triggered.
89+
90+Try to set faulty state on device in member arrays first. If kernel
91+returns EBUSY, stop this array. After that remove the device from
92+container.
93+
94+In external metadata mdmon has to remove faulty devices from degraded
95+arrays, just remove device from container.
96+
97+Raid5 array doesn't return EBUSY, it allows to remove every device.
98+Mdadm shouldn't block it.
99+
100+Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@intel.com>
101+Signed-off-by: Jes Sorensen <jsorensen@fb.com>
102+
103+Origin: upstream, https://git.kernel.org/pub/scm/utils/mdadm/mdadm.git/commit/?id=cb8f5371352f6c16af5aab8a40861e13aa50fc2b
104+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1811795
105+Last-Update: 2019-04-10
106+
107+---
108+ Incremental.c | 78 ++++++++++++++++++++++++++++++++++-----------------
109+ 1 file changed, 52 insertions(+), 26 deletions(-)
110+
111+diff --git a/Incremental.c b/Incremental.c
112+index 0c5698ee..a4ff7d4b 100644
113+--- a/Incremental.c
114++++ b/Incremental.c
115+@@ -1683,6 +1683,44 @@ static void run_udisks(char *arg1, char *arg2)
116+ ;
117+ }
118+
119++static int force_remove(char *devnm, int fd, struct mdinfo *mdi, int verbose)
120++{
121++ int rv;
122++ int devid = devnm2devid(devnm);
123++
124++ run_udisks("--unmount", map_dev(major(devid), minor(devid), 0));
125++ rv = Manage_stop(devnm, fd, verbose, 1);
126++ if (rv) {
127++ /* At least we can try to trigger a 'remove' */
128++ sysfs_uevent(mdi, "remove");
129++ if (verbose)
130++ pr_err("Fail to stop %s too.\n", devnm);
131++ }
132++ return rv;
133++}
134++
135++static void remove_from_member_array(struct mdstat_ent *memb,
136++ struct mddev_dev *devlist, int verbose)
137++{
138++ int rv;
139++ struct mdinfo mmdi;
140++ int subfd = open_dev(memb->devnm);
141++
142++ if (subfd >= 0) {
143++ rv = Manage_subdevs(memb->devnm, subfd, devlist, verbose,
144++ 0, NULL, 0);
145++ if (rv & 2) {
146++ if (sysfs_init(&mmdi, -1, memb->devnm))
147++ pr_err("unable to initialize sysfs for: %s\n",
148++ memb->devnm);
149++ else
150++ force_remove(memb->devnm, subfd, &mmdi,
151++ verbose);
152++ }
153++ close(subfd);
154++ }
155++}
156++
157+ /*
158+ * IncrementalRemove - Attempt to see if the passed in device belongs to any
159+ * raid arrays, and if so first fail (if needed) and then remove the device.
160+@@ -1754,40 +1792,28 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
161+ strncmp(ent->metadata_version, "external:", 9) == 0) {
162+ struct mdstat_ent *mdstat = mdstat_read(0, 0);
163+ struct mdstat_ent *memb;
164+- for (memb = mdstat ; memb ; memb = memb->next)
165+- if (is_container_member(memb, ent->devnm)) {
166+- int subfd = open_dev(memb->devnm);
167+- if (subfd >= 0) {
168+- rv |= Manage_subdevs(
169+- memb->devnm, subfd,
170+- &devlist, verbose, 0,
171+- NULL, 0);
172+- close(subfd);
173+- }
174+- }
175++ for (memb = mdstat ; memb ; memb = memb->next) {
176++ if (is_container_member(memb, ent->devnm))
177++ remove_from_member_array(memb,
178++ &devlist, verbose);
179++ }
180+ free_mdstat(mdstat);
181+- } else
182++ } else {
183+ rv |= Manage_subdevs(ent->devnm, mdfd, &devlist,
184+ verbose, 0, NULL, 0);
185+- if (rv & 2) {
186++ if (rv & 2) {
187+ /* Failed due to EBUSY, try to stop the array.
188+ * Give udisks a chance to unmount it first.
189+ */
190+- int devid = devnm2devid(ent->devnm);
191+- run_udisks("--unmount", map_dev(major(devid),minor(devid), 0));
192+- rv = Manage_stop(ent->devnm, mdfd, verbose, 1);
193+- if (rv)
194+- /* At least we can try to trigger a 'remove' */
195+- sysfs_uevent(&mdi, "remove");
196+- if (verbose) {
197+- if (rv)
198+- pr_err("Fail to stop %s too.\n", ent->devnm);
199++ rv = force_remove(ent->devnm, mdfd, &mdi, verbose);
200++ goto end;
201+ }
202+- } else {
203+- devlist.disposition = 'r';
204+- rv = Manage_subdevs(ent->devnm, mdfd, &devlist,
205+- verbose, 0, NULL, 0);
206+ }
207++
208++ devlist.disposition = 'r';
209++ rv = Manage_subdevs(ent->devnm, mdfd, &devlist,
210++ verbose, 0, NULL, 0);
211++end:
212+ close(mdfd);
213+ free_mdstat(ent);
214+ return rv;
215+--
216+2.17.1
217+
218diff --git a/debian/patches/series b/debian/patches/series
219index 60f1eed..387d877 100644
220--- a/debian/patches/series
221+++ b/debian/patches/series
222@@ -44,3 +44,5 @@ sha1-includes.diff
223 readlink-path.patch
224 mdmonitor-service-simplify.diff
225 0001-udev.rules-make-safe-timeouts-compatible-with-split-.patch
226+lp-1811228-imsm-Block-volume-creation-with-empty-name.patch
227+lp-1811795-remove-external-arrays-and-devices.patch

Subscribers

People subscribed via source and target branches