Re: [PATCH] Add RAID 1 chunksize test

From: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>
To: Jes Sorensen <jes@trained-monkey.org>
Cc: Doug Ledford <dledford@redhat.com>,
	Mateusz Kusiak <mateusz.kusiak@intel.com>,
	linux-raid@vger.kernel.org, colyli@suse.de
Subject: Re: [PATCH] Add RAID 1 chunksize test
Date: Thu, 7 Apr 2022 09:09:50 +0200	[thread overview]
Message-ID: <20220407090950.0000501f@linux.intel.com> (raw)
In-Reply-To: <9653c1f1-1aae-3c5d-2a43-4cb3109392b0@trained-monkey.org>

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