On Thu, 27 Jun 2013 21:38:15 +0200 Martin Wilck <mwilck@xxxxxxxx> wrote:
> Hi Neil,
>
> I wrote:
> > Well, the problem might be fixed differently by having "mdadm -Dbv" in
> > the container case print the container device.
>
> I have come up with another patch, based on that idea, that I'll submit
> in a minute. Do you like it better?
>
No I don't really. The container name is already there is "container=" so
shouldn't be in "device=" as wel. I did like the factored out "add_device",
so I kept that :-)
However you have helped me understand what the issue is and I know how I want
to fix it.
What I have ended up with is similar to your original patch, except that it
only ignores "device=" early if "container=" is set, as that is the only case
that is really a problem.
I've included my patch below and pushed it out into my git tree.
Thanks for your persistence.
NeilBrown
From c39b2e633fd6eb82a8a8e822ef01339806b05bfa Mon Sep 17 00:00:00 2001
From: NeilBrown <neilb@xxxxxxx>
Date: Tue, 2 Jul 2013 11:07:38 +1000
Subject: [PATCH] Assemble: ignore devices= if container= is present.
If "container=" is present, then we are going to assemble from the
given container where that container is made of those devices or not.
So in this case the "devices=" is purely documentation and is best
ignored.
As part of this, move the test on the "container=" value when that
start with "/" up before the device is opened. There sooner we test
things, the better.
Reported-by: Martin Wilck <mwilck@xxxxxxxx>
Signed-off-by: NeilBrown <neilb@xxxxxxx>
diff --git a/Assemble.c b/Assemble.c
index a0041c6..afe5b05 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -171,8 +171,20 @@ static int select_devices(struct mddev_dev *devlist,
if (tmpdev->used > 1)
continue;
- if (ident->devices &&
- !match_oneof(ident->devices, devname)) {
+ if (ident->container) {
+ if (ident->container[0] == '/' &&
+ !same_dev(ident->container, devname)) {
+ if (report_mismatch)
+ pr_err("%s is not the container required (%s)\n",
+ devname, ident->container);
+ continue;
+ }
+ } else if (ident->devices &&
+ !match_oneof(ident->devices, devname)) {
+ /* Note that we ignore the "device=" identifier if a
+ * "container=" is given. Checking both is unnecessarily
+ * complicated.
+ */
if (report_mismatch)
pr_err("%s is not one of %s\n", devname, ident->devices);
continue;
@@ -289,29 +301,20 @@ static int select_devices(struct mddev_dev *devlist,
}
close(dfd);
- if (ident->container) {
- if (ident->container[0] == '/' &&
- !same_dev(ident->container, devname)) {
+ if (ident->container && ident->container[0] != '/') {
+ /* we have a uuid */
+ int uuid[4];
+
+ content = *contentp;
+ tst->ss->getinfo_super(tst, content, NULL);
+
+ if (!parse_uuid(ident->container, uuid) ||
+ !same_uuid(content->uuid, uuid, tst->ss->swapuuid)) {
if (report_mismatch)
- pr_err("%s is not the container required (%s)\n",
- devname, ident->container);
+ pr_err("%s has wrong UUID to be required container\n",
+ devname);
goto loop;
}
- if (ident->container[0] != '/') {
- /* we have a uuid */
- int uuid[4];
-
- content = *contentp;
- tst->ss->getinfo_super(tst, content, NULL);
-
- if (!parse_uuid(ident->container, uuid) ||
- !same_uuid(content->uuid, uuid, tst->ss->swapuuid)) {
- if (report_mismatch)
- pr_err("%s has wrong UUID to be required container\n",
- devname);
- goto loop;
- }
- }
}
/* It is worth looking inside this container.
*/
Attachment:
signature.asc
Description: PGP signature
