From: NeilBrown <neilb@suse.de>
To: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
Cc: linux-raid@vger.kernel.org, wojciech.neubauer@intel.com,
adam.kwolek@intel.com, dan.j.williams@intel.com,
ed.ciechanowski@intel.com
Subject: Re: [PATCH 5/6] Remove code for non re-striping operations.
Date: Thu, 13 Jan 2011 13:51:34 +1100 [thread overview]
Message-ID: <20110113135134.584c1264@notabene.brown> (raw)
In-Reply-To: <20110112160132.18013.92612.stgit@gklab-128-111.igk.intel.com>
On Wed, 12 Jan 2011 17:01:32 +0100 Krzysztof Wojcik
<krzysztof.wojcik@intel.com> wrote:
> Neil,
> How was your intention when you add this part of code?
> I can't find the case when it will be necessary.
> Moreover it breaks flow for takeover operations because
> ioctl not succeeded when more than one parameter is changed.
> I've remove this code temporally to be able test takeover
> operations.
> We can restore this code after we decide what it should do.
Asking what the purpose of some code is is perfectly OK. Asking by sending a
patch which removes the code seems ... odd.
If the number of data devices is 1, then a RAID5 can have layout/chunksize
changed without any restriping.
This code handles that case.
NeilBrown
>
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> ---
> Grow.c | 43 +++++--------------------------------------
> 1 files changed, 5 insertions(+), 38 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 253f289..6835f34 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1666,6 +1666,11 @@ static int reshape_array(char *container, int fd, char *devname,
> ping_monitor(container);
> }
>
> + if (reshape.backup_blocks == 0) {
> + /* If operation not not reuire re-striping we can finish */
> + return rv;
> + }
> +
> /* ->reshape_super might have chosen some spares from the
> * container that it wants to be part of the new array.
> * We can collect them with ->container_content and give
> @@ -1692,44 +1697,6 @@ static int reshape_array(char *container, int fd, char *devname,
> }
> }
>
> - if (reshape.backup_blocks == 0) {
> - /* No restriping needed, but we might need to impose
> - * some more changes: layout, raid_disks, chunk_size
> - */
> - if (info->new_layout != UnSet &&
> - info->new_layout != info->array.layout) {
> - info->array.layout = info->new_layout;
> - if (ioctl(fd, SET_ARRAY_INFO, &info->array) != 0) {
> - fprintf(stderr, Name ": failed to set new layout\n");
> - rv = 1;
> - } else if (!quiet)
> - printf("layout for %s set to %d\n",
> - devname, info->array.layout);
> - }
> - if (info->delta_disks != UnSet &&
> - info->delta_disks != 0) {
> - info->array.raid_disks += info->delta_disks;
> - if (ioctl(fd, SET_ARRAY_INFO, &info->array) != 0) {
> - fprintf(stderr, Name ": failed to set raid disks\n");
> - rv = 1;
> - } else if (!quiet)
> - printf("raid_disks for %s set to %d\n",
> - devname, info->array.raid_disks);
> - }
> - if (info->new_chunk != 0 &&
> - info->new_chunk != info->array.chunk_size) {
> - if (sysfs_set_num(info, NULL,
> - "chunk_size", info->new_chunk) != 0) {
> - fprintf(stderr, Name ": failed to set chunk size\n");
> - rv = 1;
> - } else if (!quiet)
> - printf("chunk size for %s set to %d\n",
> - devname, info->array.chunk_size);
> - }
> -
> - return rv;
> - }
> -
> /*
> * There are three possibilities.
> * 1/ The array will shrink.
next prev parent reply other threads:[~2011-01-13 2:51 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-12 16:00 [PATCH 0/6] raid10->raid0 takeover for imsm metadata Krzysztof Wojcik
2011-01-12 16:00 ` [PATCH 1/6] reshape_super reorganization Krzysztof Wojcik
2011-01-13 2:45 ` NeilBrown
2011-01-12 16:01 ` [PATCH 2/6] Define imsm_analyze_change function Krzysztof Wojcik
2011-01-13 2:46 ` NeilBrown
2011-01-12 16:01 ` [PATCH 3/6] Set delta_disks for raid10 -> raid0 takeover Krzysztof Wojcik
2011-01-13 2:48 ` NeilBrown
2011-01-12 16:01 ` [PATCH 4/6] FIX: Mistake in delta_disk comparison Krzysztof Wojcik
2011-01-13 2:48 ` NeilBrown
2011-01-12 16:01 ` [PATCH 5/6] Remove code for non re-striping operations Krzysztof Wojcik
2011-01-13 2:51 ` NeilBrown [this message]
2011-01-13 16:00 ` Wojcik, Krzysztof
2011-01-17 15:49 ` Wojcik, Krzysztof
2011-01-19 20:52 ` NeilBrown
2011-01-20 10:59 ` Wojcik, Krzysztof
2011-01-12 16:01 ` [PATCH 6/6] Add raid10 -> raid0 takeover support Krzysztof Wojcik
2011-01-13 2:49 ` NeilBrown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110113135134.584c1264@notabene.brown \
--to=neilb@suse.de \
--cc=adam.kwolek@intel.com \
--cc=dan.j.williams@intel.com \
--cc=ed.ciechanowski@intel.com \
--cc=krzysztof.wojcik@intel.com \
--cc=linux-raid@vger.kernel.org \
--cc=wojciech.neubauer@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).