* [PATCH 1/3] DDF tests: allow to run on systems without /dev/sda @ 2013-09-14 20:47 mwilck 2013-09-14 20:47 ` [PATCH 2/3] DDF test: make sure mdmon isn't started by systemd mwilck 2013-09-14 20:47 ` [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check mwilck 0 siblings, 2 replies; 4+ messages in thread From: mwilck @ 2013-09-14 20:47 UTC (permalink / raw) To: neilb, linux-raid; +Cc: mwilck Some ddf tests scripts assume that /dev/sda is always present. That's wrong e.g. on VMs. Use a more general approach. --- tests/10ddf-create | 10 ++++++---- tests/10ddf-fail-two-spares | 5 +++-- tests/env-ddf-template | 9 +++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/10ddf-create b/tests/10ddf-create index 50c85ae..2f7747c 100644 --- a/tests/10ddf-create +++ b/tests/10ddf-create @@ -9,6 +9,8 @@ # add some data, tear down the array, reassemble # and make sure it is still there. set -e +. tests/env-ddf-template +sda=$(get_rootdev) || exit 1 mdadm -CR /dev/md/ddf0 -e ddf -n 5 $dev8 $dev9 $dev10 $dev11 $dev12 mdadm -CR r5 -l5 -n5 /dev/md/ddf0 -z 5000 @@ -23,10 +25,10 @@ testdev /dev/md/r10 2 5000 512 # r0/r10 will use 4608 due to chunk size, so that leaves 23552 for the rest testdev /dev/md/r1 1 23552 64 testdev /dev/md/r0 3 23552 512 -dd if=/dev/sda of=/dev/md/r0 || true -dd if=/dev/sda of=/dev/md/r10 || true -dd if=/dev/sda of=/dev/md/r1 || true -dd if=/dev/sda of=/dev/md/r5 || true +dd if=$sda of=/dev/md/r0 || true +dd if=$sda of=/dev/md/r10 || true +dd if=$sda of=/dev/md/r1 || true +dd if=$sda of=/dev/md/r5 || true s0=`sha1sum /dev/md/r0` s10=`sha1sum /dev/md/r10` diff --git a/tests/10ddf-fail-two-spares b/tests/10ddf-fail-two-spares index cc2cbb4..fa6e2e8 100644 --- a/tests/10ddf-fail-two-spares +++ b/tests/10ddf-fail-two-spares @@ -1,5 +1,6 @@ # Simulate two disks failing shorty after each other . tests/env-ddf-template +sda=$(get_rootdev) || exit 1 tmp=$(mktemp /tmp/mdtest-XXXXXX) mdadm --zero-superblock $dev8 $dev9 $dev10 $dev11 $dev12 $dev13 @@ -13,8 +14,8 @@ mdadm -CR $member0 -l raid6 -n 4 $dev10 $dev11 $dev12 $dev13 -z 16384 # >/tmp/mdmon.txt 2>&1 mdadm -CR $member1 -l raid10 -n 4 $dev10 $dev11 $dev12 $dev13 -z 16384 -dd if=/dev/sda of=$member0 bs=1M -dd if=/dev/sda of=$member1 bs=1M skip=16 +dd if=$sda of=$member0 bs=1M +dd if=$sda of=$member1 bs=1M skip=16 check wait diff --git a/tests/env-ddf-template b/tests/env-ddf-template index 1c1ca12..aa57fab 100644 --- a/tests/env-ddf-template +++ b/tests/env-ddf-template @@ -1,3 +1,12 @@ +get_rootdev() { + local dev=$(stat -c %D /) + local maj=$(expr $dev : '\(..\)') + local min=${dev#$maj} + local bd=/dev/$(basename $(readlink /sys/dev/block/$((0x$maj)):$((0x$min)))) + [ -b $bd ] || exit 1 + echo $bd +} + get_sysdir() { local mddev=$1 [ -L $mddev ] && mddev=$(readlink -f $mddev) -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/3] DDF test: make sure mdmon isn't started by systemd 2013-09-14 20:47 [PATCH 1/3] DDF tests: allow to run on systems without /dev/sda mwilck @ 2013-09-14 20:47 ` mwilck 2013-09-14 20:47 ` [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check mwilck 1 sibling, 0 replies; 4+ messages in thread From: mwilck @ 2013-09-14 20:47 UTC (permalink / raw) To: neilb, linux-raid; +Cc: mwilck For testing we usually want the locally built mdmon, not the one systemd prefers. --- tests/env-ddf-template | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/tests/env-ddf-template b/tests/env-ddf-template index aa57fab..a607c10 100644 --- a/tests/env-ddf-template +++ b/tests/env-ddf-template @@ -104,3 +104,6 @@ member1=/dev/md/vol1 member2=/dev/md/vol2 member3=/dev/md/vol3 member4=/dev/md/vol4 + +# We don't want systemd to start system mdmon; start our own +export MDADM_NO_SYSTEMCTL=1 -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check 2013-09-14 20:47 [PATCH 1/3] DDF tests: allow to run on systems without /dev/sda mwilck 2013-09-14 20:47 ` [PATCH 2/3] DDF test: make sure mdmon isn't started by systemd mwilck @ 2013-09-14 20:47 ` mwilck 2013-09-16 19:53 ` Martin Wilck 1 sibling, 1 reply; 4+ messages in thread From: mwilck @ 2013-09-14 20:47 UTC (permalink / raw) To: neilb, linux-raid; +Cc: mwilck The sequence number check in compare_super_ddf was broken, anchor sequence number is always -1. With this patch, mdadm will refuse to add a disk with non-matching sequence number. This fixes Francis Moreau's problem reported with subject "mdadm 3.3 fails to kick out non fresh disk". FIXME: More work is needed here. Currently mdadm won't even add the disk to the container, that's wrong. It should be added as a spare. --- super-ddf.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/super-ddf.c b/super-ddf.c index 3673cb3..002b271 100644 --- a/super-ddf.c +++ b/super-ddf.c @@ -3892,10 +3892,10 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst) if (memcmp(first->anchor.guid, second->anchor.guid, DDF_GUID_LEN) != 0) return 2; - if (!be32_eq(first->anchor.seq, second->anchor.seq)) { - dprintf("%s: sequence number mismatch %u/%u\n", __func__, - be32_to_cpu(first->anchor.seq), - be32_to_cpu(second->anchor.seq)); + if (!be32_eq(first->active->seq, second->active->seq)) { + dprintf("%s: sequence number mismatch %u<->%u\n", __func__, + be32_to_cpu(first->active->seq), + be32_to_cpu(second->active->seq)); return 3; } if (first->max_part != second->max_part || -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check 2013-09-14 20:47 ` [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check mwilck @ 2013-09-16 19:53 ` Martin Wilck 0 siblings, 0 replies; 4+ messages in thread From: Martin Wilck @ 2013-09-16 19:53 UTC (permalink / raw) To: linux-raid; +Cc: neilb On 09/14/2013 10:47 PM, mwilck@arcor.de wrote: > The sequence number check in compare_super_ddf was broken, > anchor sequence number is always -1. > > With this patch, mdadm will refuse to add a disk with non-matching > sequence number. > > This fixes Francis Moreau's problem reported with subject > "mdadm 3.3 fails to kick out non fresh disk". > > FIXME: More work is needed here. Currently mdadm won't even add the > disk to the container, that's wrong. It should be added as a spare. > --- > super-ddf.c | 8 ++++---- > 1 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/super-ddf.c b/super-ddf.c > index 3673cb3..002b271 100644 > --- a/super-ddf.c > +++ b/super-ddf.c > @@ -3892,10 +3892,10 @@ static int compare_super_ddf(struct supertype *st, struct supertype *tst) > if (memcmp(first->anchor.guid, second->anchor.guid, DDF_GUID_LEN) != 0) > return 2; > > - if (!be32_eq(first->anchor.seq, second->anchor.seq)) { > - dprintf("%s: sequence number mismatch %u/%u\n", __func__, > - be32_to_cpu(first->anchor.seq), > - be32_to_cpu(second->anchor.seq)); > + if (!be32_eq(first->active->seq, second->active->seq)) { > + dprintf("%s: sequence number mismatch %u<->%u\n", __func__, > + be32_to_cpu(first->active->seq), > + be32_to_cpu(second->active->seq)); > return 3; > } > if (first->max_part != second->max_part || While the fix itself is certainly correct, I don't think any more that this is the right thing to do. If we reject a disk in compare_super() just because of a sequence number mismatch, we won't be able to handle incremental scanning gracefully, because mdadm will never even try to add it any more. This is exactly what Francis just noticed. However, just ignoring the sequence number as the code did without the patch is also incorrect, as Francis noted previously in the "fails to kick out non fresh disk" thread. The container loading code already has some logic to deal with different sequence numbers during assembly (just pick the highest). In case of incremental assembly, things get more complex. We must be prepared to read the disk with the "best" (i.e. highest) meta data after other disk, possibly after some subarrays have already been started. It is possible that the new best meta data indicates failures previously loaded disks and already started arrays. I am still trying to understand exactly how mdadm, mdmon, and kernel have to play together during incremental assembly in order to get this right. I think most of this should take place in mdmon/manager, but I may be wrong about that. Martin ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-16 19:53 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-14 20:47 [PATCH 1/3] DDF tests: allow to run on systems without /dev/sda mwilck 2013-09-14 20:47 ` [PATCH 2/3] DDF test: make sure mdmon isn't started by systemd mwilck 2013-09-14 20:47 ` [PATCH 3/3] DDF: compare_super_ddf: fix sequence number check mwilck 2013-09-16 19:53 ` Martin Wilck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).