NeilBrown wrote:
> On Fri, 24 Apr 2015 15:30:32 +0800 gqjiang@xxxxxxxx wrote:
>
>
>> From: Guoqing Jiang <gqjiang@xxxxxxxx>
>>
>> Specifies the maximum number of nodes in the cluster that may use
>> this device simultaneously. This is equivalent to the number of
>> bitmaps created in the internal superblock (patches to follow).
>>
>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
>> Signed-off-by: Guoqing Jiang <gqjiang@xxxxxxxx>
>>
>
> This doesn't really make much sense coming first in the series. It sets up a
> value that is never used.
>
> I would much rather 03/10 came first - which would just create 4 bitmaps.
> Then have this patch to allow that '4' to be changed.
>
>
>
Ok, I will re-arrange them, make 03 first, and keep the others with
original sequence.
>> ---
>> Create.c | 1 +
>> ReadMe.c | 1 +
>> mdadm.8.in | 5 +++++
>> mdadm.c | 20 +++++++++++++++++++-
>> mdadm.h | 3 +++
>> 5 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/Create.c b/Create.c
>> index ef28da0..b73f6cb 100644
>> --- a/Create.c
>> +++ b/Create.c
>> @@ -531,6 +531,7 @@ int Create(struct supertype *st, char *mddev,
>> st->ss->name);
>> warn = 1;
>> }
>> + st->nodes = c->nodes;
>>
>> if (warn) {
>> if (c->runstop!= 1) {
>> diff --git a/ReadMe.c b/ReadMe.c
>> index 87a4916..30c569d 100644
>> --- a/ReadMe.c
>> +++ b/ReadMe.c
>> @@ -140,6 +140,7 @@ struct option long_options[] = {
>> {"homehost", 1, 0, HomeHost},
>> {"symlinks", 1, 0, Symlinks},
>> {"data-offset",1, 0, DataOffset},
>> + {"nodes",1, 0, Nodes},
>>
>> /* For assemble */
>> {"uuid", 1, 0, 'u'},
>> diff --git a/mdadm.8.in b/mdadm.8.in
>> index a630310..bd8d59e 100644
>> --- a/mdadm.8.in
>> +++ b/mdadm.8.in
>> @@ -966,6 +966,11 @@ However for RAID0, it is not possible to add spares. So to increase
>> the number of devices in a RAID0, it is necessary to set the new
>> number of devices, and to add the new devices, in the same command.
>>
>> +.TP
>> +.BR \-\-nodes
>> +Specify the maximum number of nodes in the cluster that will use this
>> +device simultaneously. If not specified, this defaults to 4.
>> +
>>
>
> I think this should mention that it only makes sense with --bitmap=cluster
> (which of course isn't available at this point....)
>
>
>
Will add it.
>> .SH For assemble:
>>
>> .TP
>> diff --git a/mdadm.c b/mdadm.c
>> index 3e8c49b..bce6a76 100644
>> --- a/mdadm.c
>> +++ b/mdadm.c
>> @@ -588,7 +588,14 @@ int main(int argc, char *argv[])
>> }
>> ident.raid_disks = s.raiddisks;
>> continue;
>> -
>> + case O(CREATE, Nodes):
>> + c.nodes = parse_num(optarg);
>> + if (c.nodes <= 0) {
>> + pr_err("invalid number for the number of "
>> + "cluster nodes: %s\n", optarg);
>> + exit(2);
>> + }
>> + continue;
>> case O(CREATE,'x'): /* number of spare (eXtra) disks */
>> if (s.sparedisks) {
>> pr_err("spare-devices set twice: %d and %s\n",
>> @@ -1377,6 +1384,17 @@ int main(int argc, char *argv[])
>> case CREATE:
>> if (c.delay == 0)
>> c.delay = DEFAULT_BITMAP_DELAY;
>> +
>> + if (!strncmp(s.bitmap_file, "internal", 9) ||
>> + !strncmp(s.bitmap_file,"none", 4)) {
>>
>
> I'm sorry but I absolutely *hate* this construct.
> The '!' at the front makes it seem like you are testing that the string does
> *not* have that value, but it is exactly the reverse.
> And why "strncmp" rather than "strcmp" ??
>
> Please use
>
> if (strcmp(s.bitmap_file, "internal") == 0 ||
> strcmp(s.bitmap_file, "none) == 0) {
>
> except.... what about if bitmap_file in /path/to/somewhere. Presumably you
> want to exclude that case too.
> May be
> if (strcmp(s.bitmap_file, "cluster") != 0) {
> ??
>
Thanks, which is better for understanding.
Regards,
Guoqing
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
