All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add RAID 1 chunksize test
@ 2022-04-04  7:08 Mateusz Kusiak
  2022-04-04 12:38 ` Doug Ledford
  0 siblings, 1 reply; 4+ messages in thread
From: Mateusz Kusiak @ 2022-04-04  7:08 UTC (permalink / raw)
  To: linux-raid; +Cc: jes, colyli

Specifying chunksize for raid 1 is forbidden.
Add test for blocking raid 1 creation with chunksize.

Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
---
 tests/01r1create-chunk | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 tests/01r1create-chunk

diff --git a/tests/01r1create-chunk b/tests/01r1create-chunk
new file mode 100644
index 00000000..717a5e5a
--- /dev/null
+++ b/tests/01r1create-chunk
@@ -0,0 +1,15 @@
+# RAID 1 volume, 2 disks, chunk 64
+# NEGATIVE test - creating raid 1 with chunksize specified is forbidden
+
+num_disks=2
+level=1
+device_list="$dev0 $dev1"
+chunk=64
+
+# Create raid 1 with chunk 64k and fail
+if ! mdadm --create --run $md0 --auto=md --level=$level --chunk=$chunk --raid-disks=$num_disks $device_list
+then
+	exit 0
+fi
+
+exit 1
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add RAID 1 chunksize test
  2022-04-04  7:08 [PATCH] Add RAID 1 chunksize test Mateusz Kusiak
@ 2022-04-04 12:38 ` Doug Ledford
  2022-04-06 16:46   ` Jes Sorensen
  0 siblings, 1 reply; 4+ messages in thread
From: Doug Ledford @ 2022-04-04 12:38 UTC (permalink / raw)
  To: Mateusz Kusiak, linux-raid; +Cc: jes, colyli

[-- Attachment #1: Type: text/plain, Size: 1366 bytes --]

On Mon, 2022-04-04 at 09:08 +0200, Mateusz Kusiak wrote:
> Specifying chunksize for raid 1 is forbidden.
> Add test for blocking raid 1 creation with chunksize.
> 
> Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
> ---
>  tests/01r1create-chunk | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>  create mode 100644 tests/01r1create-chunk
> 
> diff --git a/tests/01r1create-chunk b/tests/01r1create-chunk
> new file mode 100644
> index 00000000..717a5e5a
> --- /dev/null
> +++ b/tests/01r1create-chunk
> @@ -0,0 +1,15 @@
> +# RAID 1 volume, 2 disks, chunk 64
> +# NEGATIVE test - creating raid 1 with chunksize specified is
> forbidden
> +
> +num_disks=2
> +level=1
> +device_list="$dev0 $dev1"
> +chunk=64
> +
> +# Create raid 1 with chunk 64k and fail
> +if ! mdadm --create --run $md0 --auto=md --level=$level --
> chunk=$chunk --raid-disks=$num_disks $device_list
> +then
> +       exit 0
> +fi
> +
> +exit 1

This is a case of overkill IMO.  Chunk size with raid1 isn't really a
problem and shouldn't result in mdadm refusing to work.  Chunk size with
raid1 simply has no effect and should just be ignored with at most a
warning by mdadm.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add RAID 1 chunksize test
  2022-04-04 12:38 ` Doug Ledford
@ 2022-04-06 16:46   ` Jes Sorensen
  2022-04-07  7:09     ` Mariusz Tkaczyk
  0 siblings, 1 reply; 4+ messages in thread
From: Jes Sorensen @ 2022-04-06 16:46 UTC (permalink / raw)
  To: Doug Ledford, Mateusz Kusiak, linux-raid; +Cc: colyli

On 4/4/22 08:38, Doug Ledford wrote:
> On Mon, 2022-04-04 at 09:08 +0200, Mateusz Kusiak wrote:
>> Specifying chunksize for raid 1 is forbidden.
>> Add test for blocking raid 1 creation with chunksize.
>>
>> Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
>> ---
>>  tests/01r1create-chunk | 15 +++++++++++++++
>>  1 file changed, 15 insertions(+)
>>  create mode 100644 tests/01r1create-chunk
>>
>> diff --git a/tests/01r1create-chunk b/tests/01r1create-chunk
>> new file mode 100644
>> index 00000000..717a5e5a
>> --- /dev/null
>> +++ b/tests/01r1create-chunk
>> @@ -0,0 +1,15 @@
>> +# RAID 1 volume, 2 disks, chunk 64
>> +# NEGATIVE test - creating raid 1 with chunksize specified is
>> forbidden
>> +
>> +num_disks=2
>> +level=1
>> +device_list="$dev0 $dev1"
>> +chunk=64
>> +
>> +# Create raid 1 with chunk 64k and fail
>> +if ! mdadm --create --run $md0 --auto=md --level=$level --
>> chunk=$chunk --raid-disks=$num_disks $device_list
>> +then
>> +       exit 0
>> +fi
>> +
>> +exit 1
> 
> This is a case of overkill IMO.  Chunk size with raid1 isn't really a
> problem and shouldn't result in mdadm refusing to work.  Chunk size with
> raid1 simply has no effect and should just be ignored with at most a
> warning by mdadm.

I agree with Doug here. I think a warning from mdadm that chunksize
makes no sense for raid1 would be good, but having a failed test over it
makes little sense. If anything the test should detect the warning is
happening.

Thanks,
Jes




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Add RAID 1 chunksize test
  2022-04-06 16:46   ` Jes Sorensen
@ 2022-04-07  7:09     ` Mariusz Tkaczyk
  0 siblings, 0 replies; 4+ messages in thread
From: Mariusz Tkaczyk @ 2022-04-07  7:09 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: Doug Ledford, Mateusz Kusiak, linux-raid, colyli

On Wed, 6 Apr 2022 12:46:03 -0400
Jes Sorensen <jes@trained-monkey.org> wrote:

> On 4/4/22 08:38, Doug Ledford wrote:
> > On Mon, 2022-04-04 at 09:08 +0200, Mateusz Kusiak wrote:  
> >> Specifying chunksize for raid 1 is forbidden.
> >> Add test for blocking raid 1 creation with chunksize.
> >>
> >> Signed-off-by: Mateusz Kusiak <mateusz.kusiak@intel.com>
> >> ---
> >>  tests/01r1create-chunk | 15 +++++++++++++++
> >>  1 file changed, 15 insertions(+)
> >>  create mode 100644 tests/01r1create-chunk
> >>
> >> diff --git a/tests/01r1create-chunk b/tests/01r1create-chunk
> >> new file mode 100644
> >> index 00000000..717a5e5a
> >> --- /dev/null
> >> +++ b/tests/01r1create-chunk
> >> @@ -0,0 +1,15 @@
> >> +# RAID 1 volume, 2 disks, chunk 64
> >> +# NEGATIVE test - creating raid 1 with chunksize specified is
> >> forbidden
> >> +
> >> +num_disks=2
> >> +level=1
> >> +device_list="$dev0 $dev1"
> >> +chunk=64
> >> +
> >> +# Create raid 1 with chunk 64k and fail
> >> +if ! mdadm --create --run $md0 --auto=md --level=$level --
> >> chunk=$chunk --raid-disks=$num_disks $device_list
> >> +then
> >> +       exit 0
> >> +fi
> >> +
> >> +exit 1  
> > 
> > This is a case of overkill IMO.  Chunk size with raid1 isn't really
> > a problem and shouldn't result in mdadm refusing to work.  Chunk
> > size with raid1 simply has no effect and should just be ignored
> > with at most a warning by mdadm.  
> 
> I agree with Doug here. I think a warning from mdadm that chunksize
> makes no sense for raid1 would be good, but having a failed test over
> it makes little sense. If anything the test should detect the warning
> is happening.
>

Ideally, chunk parameter for RAID1 should not be recognized, because it
is not an option. For that reason I'm closer to say that we shouldn't
continue if chunk is passed. It helps to limit abuses from less
experienced users.

Jes, you asked for this test:
https://lore.kernel.org/linux-raid/bdcc98ec-72bc-d0ab-7e82-7e00bc02447f@trained-monkey.org/

Thanks,
Mariusz


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-04-07  7:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-04  7:08 [PATCH] Add RAID 1 chunksize test Mateusz Kusiak
2022-04-04 12:38 ` Doug Ledford
2022-04-06 16:46   ` Jes Sorensen
2022-04-07  7:09     ` Mariusz Tkaczyk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.