[PATCH] Add '--nodes' option in GROW mode

[PATCH] Add '--nodes' option in GROW mode

[zhilong]

Bug description:
# mdadm -Cv /dev/md0 --bitmap=clustered -l1 -n2 /dev/sdd /dev/sdb 
--nodes=6 --assume-clean
# mdadm --grow /dev/md0 --bitmap=none
# mdadm --grow /dev/md0 --bitmap=clustered
     -> cluster nodes would be changed to 4, here is my method to 
resolve it.
# mdadm --grow /dev/md0 --bitmap=clustered --nodes=6

Patch:

Cluster nodes is set 4 as default when switch bitmap from
none to clustered under GROW mode, so add '--nodes'
option in GROW mode.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
  mdadm.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/mdadm.c b/mdadm.c
index 51e16f3..b542b8c 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -592,6 +592,7 @@ int main(int argc, char *argv[])
              ident.raid_disks = s.raiddisks;
              continue;
          case O(ASSEMBLE, Nodes):
+        case O(GROW, Nodes):
          case O(CREATE, Nodes):
              c.nodes = parse_num(optarg);
              if (c.nodes <= 0) {
-- 
2.1.4

Re: [PATCH] Add '--nodes' option in GROW mode

[Jes Sorensen]

zhilong <zlliu@suse.com> writes:
> Bug description:
> # mdadm -Cv /dev/md0 --bitmap=clustered -l1 -n2 /dev/sdd /dev/sdb
> --nodes=6 --assume-clean
> # mdadm --grow /dev/md0 --bitmap=none
> # mdadm --grow /dev/md0 --bitmap=clustered
>     -> cluster nodes would be changed to 4, here is my method to
> resolve it.
> # mdadm --grow /dev/md0 --bitmap=clustered --nodes=6
>
> Patch:
>
> Cluster nodes is set 4 as default when switch bitmap from
> none to clustered under GROW mode, so add '--nodes'
> option in GROW mode.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>  mdadm.c | 1 +
>  1 file changed, 1 insertion(+)

Hi,

I have a question about this, since I simply do not have the knowledge
about clustering to judge this approach.

If you remove the bitmap, does that 'uncluster' the array? If not,
shouldn't mdadm be fixed to not set #nodes to 4 when adding a clustered
bitmap, if a number of cluster nodes is already present?

Thanks,
Jes

>
> diff --git a/mdadm.c b/mdadm.c
> index 51e16f3..b542b8c 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -592,6 +592,7 @@ int main(int argc, char *argv[])
>              ident.raid_disks = s.raiddisks;
>              continue;
>          case O(ASSEMBLE, Nodes):
> +        case O(GROW, Nodes):
>          case O(CREATE, Nodes):
>              c.nodes = parse_num(optarg);
>              if (c.nodes <= 0) {

Re: [PATCH] Add '--nodes' option in GROW mode

[Guoqing Jiang]

On 03/23/2016 01:56 AM, Jes Sorensen wrote:
> zhilong <zlliu@suse.com> writes:
>> Bug description:
>> # mdadm -Cv /dev/md0 --bitmap=clustered -l1 -n2 /dev/sdd /dev/sdb
>> --nodes=6 --assume-clean
>> # mdadm --grow /dev/md0 --bitmap=none
>> # mdadm --grow /dev/md0 --bitmap=clustered
>>      -> cluster nodes would be changed to 4, here is my method to
>> resolve it.
>> # mdadm --grow /dev/md0 --bitmap=clustered --nodes=6
>>
>> Patch:
>>
>> Cluster nodes is set 4 as default when switch bitmap from
>> none to clustered under GROW mode, so add '--nodes'
>> option in GROW mode.
>>
>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>> ---
>>   mdadm.c | 1 +
>>   1 file changed, 1 insertion(+)
> Hi,
>
> I have a question about this, since I simply do not have the knowledge
> about clustering to judge this approach.
>
> If you remove the bitmap, does that 'uncluster' the array? If not,

Yes, remove the bitmap would 'uncluster' the array since leave()
will be called in kernel if switch the bitmap from 'clustered' to
'none'.

> shouldn't mdadm be fixed to not set #nodes to 4 when adding a clustered
> bitmap, if a number of cluster nodes is already present?

The nodes is set 4 by default if the 'nodes' parameter is not specified,
And it could be overwrite if re-create the clustered bitmap.

Thanks,
Guoqing

Re: [PATCH] Add '--nodes' option in GROW mode

[Jes Sorensen]

Guoqing Jiang <gqjiang@suse.com> writes:
> On 03/23/2016 01:56 AM, Jes Sorensen wrote:
>> zhilong <zlliu@suse.com> writes:
>>> Bug description:
>>> # mdadm -Cv /dev/md0 --bitmap=clustered -l1 -n2 /dev/sdd /dev/sdb
>>> --nodes=6 --assume-clean
>>> # mdadm --grow /dev/md0 --bitmap=none
>>> # mdadm --grow /dev/md0 --bitmap=clustered
>>>      -> cluster nodes would be changed to 4, here is my method to
>>> resolve it.
>>> # mdadm --grow /dev/md0 --bitmap=clustered --nodes=6
>>>
>>> Patch:
>>>
>>> Cluster nodes is set 4 as default when switch bitmap from
>>> none to clustered under GROW mode, so add '--nodes'
>>> option in GROW mode.
>>>
>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>> ---
>>>   mdadm.c | 1 +
>>>   1 file changed, 1 insertion(+)
>> Hi,
>>
>> I have a question about this, since I simply do not have the knowledge
>> about clustering to judge this approach.
>>
>> If you remove the bitmap, does that 'uncluster' the array? If not,
>
> Yes, remove the bitmap would 'uncluster' the array since leave()
> will be called in kernel if switch the bitmap from 'clustered' to
> 'none'.
>
>> shouldn't mdadm be fixed to not set #nodes to 4 when adding a clustered
>> bitmap, if a number of cluster nodes is already present?
>
> The nodes is set 4 by default if the 'nodes' parameter is not specified,
> And it could be overwrite if re-create the clustered bitmap.

Thanks for the explanation, in that case I am fine with this. It does
raise the issue whether specifying nodes shouldn't be a required
argument rather than defaulting to a default of 4?

Second, your patch was broken due to bad whitespace formatting, could
you please send me a fresh one.

Thanks,
Jes

Re: [PATCH] mdadm:Add '--nodes' option in GROW mode

[zhilong]

mdadm:add '--nodes' option in GROW mode, because
'Cluster nodes' is set 4 by default if the nodes
parameter is not specified when switch bitmap
from none to clustered.

Signed-off-by: Zhilong Liu <zlliu@suse.com>
---
  mdadm.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/mdadm.c b/mdadm.c
index d2afcb2..58ac32a 100644
--- a/mdadm.c
+++ b/mdadm.c
@@ -589,6 +589,7 @@ int main(int argc, char *argv[])
  			ident.raid_disks = s.raiddisks;
  			continue;
  		case O(ASSEMBLE, Nodes):
+		case O(GROW, Nodes):
  		case O(CREATE, Nodes):
  			c.nodes = parse_num(optarg);
  			if (c.nodes <= 0) {
-- 
2.1.4

On 03/25/2016 12:31 AM, Jes Sorensen wrote:
> Guoqing Jiang <gqjiang@suse.com> writes:
>> On 03/23/2016 01:56 AM, Jes Sorensen wrote:
>>> zhilong <zlliu@suse.com> writes:
>>>> Bug description:
>>>> # mdadm -Cv /dev/md0 --bitmap=clustered -l1 -n2 /dev/sdd /dev/sdb
>>>> --nodes=6 --assume-clean
>>>> # mdadm --grow /dev/md0 --bitmap=none
>>>> # mdadm --grow /dev/md0 --bitmap=clustered
>>>>       -> cluster nodes would be changed to 4, here is my method to
>>>> resolve it.
>>>> # mdadm --grow /dev/md0 --bitmap=clustered --nodes=6
>>>>
>>>> Patch:
>>>>
>>>> Cluster nodes is set 4 as default when switch bitmap from
>>>> none to clustered under GROW mode, so add '--nodes'
>>>> option in GROW mode.
>>>>
>>>> Signed-off-by: Zhilong Liu <zlliu@suse.com>
>>>> ---
>>>>    mdadm.c | 1 +
>>>>    1 file changed, 1 insertion(+)
>>> Hi,
>>>
>>> I have a question about this, since I simply do not have the knowledge
>>> about clustering to judge this approach.
>>>
>>> If you remove the bitmap, does that 'uncluster' the array? If not,
>> Yes, remove the bitmap would 'uncluster' the array since leave()
>> will be called in kernel if switch the bitmap from 'clustered' to
>> 'none'.
>>
>>> shouldn't mdadm be fixed to not set #nodes to 4 when adding a clustered
>>> bitmap, if a number of cluster nodes is already present?
>> The nodes is set 4 by default if the 'nodes' parameter is not specified,
>> And it could be overwrite if re-create the clustered bitmap.
> Thanks for the explanation, in that case I am fine with this. It does
> raise the issue whether specifying nodes shouldn't be a required
> argument rather than defaulting to a default of 4?
>
> Second, your patch was broken due to bad whitespace formatting, could
> you please send me a fresh one.
>
> Thanks,
> Jes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] mdadm:Add '--nodes' option in GROW mode

[Jes Sorensen]

zhilong <zlliu@suse.com> writes:
> mdadm:add '--nodes' option in GROW mode, because
> 'Cluster nodes' is set 4 by default if the nodes
> parameter is not specified when switch bitmap
> from none to clustered.
>
> Signed-off-by: Zhilong Liu <zlliu@suse.com>
> ---
>  mdadm.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/mdadm.c b/mdadm.c
> index d2afcb2..58ac32a 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -589,6 +589,7 @@ int main(int argc, char *argv[])
>  			ident.raid_disks = s.raiddisks;
>  			continue;
>  		case O(ASSEMBLE, Nodes):
> +		case O(GROW, Nodes):
>  		case O(CREATE, Nodes):
>  			c.nodes = parse_num(optarg);
>  			if (c.nodes <= 0) {

Applied!

Something funny happened to the formatting of your patch. Somehow it
ended up with an additional space at the beginning of each line. Not
sure if it is related to your mail client using format-flowed.

I manually fixed it up, so it's all good.

Thanks,
Jes