Linux RAID subsystem development
 help / color / mirror / Atom feed
* [PATCH dm] dm thin: __blkdev_issue_discard_async() can be static
From: kbuild test robot @ 2015-05-22 21:37 UTC (permalink / raw)
  To: Joe Thornber
  Cc: kbuild-all, Mike Snitzer, Alasdair Kergon, dm-devel, Neil Brown,
	linux-raid, linux-kernel
In-Reply-To: <201505230502.WN0Nq7Zv%fengguang.wu@intel.com>


Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---
 dm-thin.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-thin.c b/drivers/md/dm-thin.c
index c2634a5..ef47420 100644
--- a/drivers/md/dm-thin.c
+++ b/drivers/md/dm-thin.c
@@ -335,7 +335,7 @@ struct thin_c {
  *    that is being kept local to DM thinp until the block changes to allow
  *    late bio splitting land upstream.
  */
-int __blkdev_issue_discard_async(struct block_device *bdev, sector_t sector,
+static int __blkdev_issue_discard_async(struct block_device *bdev, sector_t sector,
 				 sector_t nr_sects, gfp_t gfp_mask, unsigned long flags,
 				 struct bio *parent_bio)
 {

^ permalink raw reply related

* [dm:for-next 30/30] drivers/md/dm-thin.c:338:5: sparse: symbol '__blkdev_issue_discard_async' was not declared. Should it be static?
From: kbuild test robot @ 2015-05-22 21:37 UTC (permalink / raw)
  To: Joe Thornber
  Cc: kbuild-all, Mike Snitzer, Alasdair Kergon, dm-devel, Neil Brown,
	linux-raid, linux-kernel

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
head:   9f1e427e6d6c80f0c3d7b38ddfbd98c9f8f2af41
commit: 9f1e427e6d6c80f0c3d7b38ddfbd98c9f8f2af41 [30/30] dm thin: range discard support
reproduce:
  # apt-get install sparse
  git checkout 9f1e427e6d6c80f0c3d7b38ddfbd98c9f8f2af41
  make ARCH=x86_64 allmodconfig
  make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/md/dm-thin.c:338:5: sparse: symbol '__blkdev_issue_discard_async' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild                 Intel Corporation

^ permalink raw reply

* Re: [PATCH] Fix minor typo in mdadm manpage.
From: NeilBrown @ 2015-05-22 23:30 UTC (permalink / raw)
  To: Andrew Burgess; +Cc: linux-raid
In-Reply-To: <1427303869-17319-1-git-send-email-andrew.burgess@embecosm.com>

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

On Wed, 25 Mar 2015 17:17:49 +0000 Andrew Burgess
<andrew.burgess@embecosm.com> wrote:

> Appologies if this is the wrong mailing list for this patch.
> 
> This is a very small patch for the manual page for the mdadm utility.
> 
> Thanks,
> Andrew
> 
> ---
> Change 'is and of' to 'is one of' in mdadm manual page.
> 
> Signed-off-by: Andrew Burgess <andrew.burgess@embecosm.com>
> ---
>  mdadm.8.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mdadm.8.in b/mdadm.8.in
> index a630310..b4a21d9 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -214,7 +214,7 @@ to detect and assemble arrays \(em possibly in an
>  
>  .P
>  If a device is given before any options, or if the first option is
> -and of
> +one of
>  .BR \-\-add ,
>  .BR \-\-re\-add ,
>  .BR \-\-add\-spare ,

Applied, thanks. (And thanks for the reminder).

NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: Shaohua Li @ 2015-05-22 23:44 UTC (permalink / raw)
  To: NeilBrown; +Cc: linux-raid
In-Reply-To: <20150522053058.2117.29026.stgit@notabene.brown>

On Fri, May 22, 2015 at 03:30:58PM +1000, NeilBrown wrote:
> If a stripe is a member of a batch, but not the head, it must
> not be handled separately from the rest of the batch.
> 
> 'clear_batch_ready()' handles this requirement to some
> extent but not completely.  If a member is passed to handle_stripe()
> a second time it returns '0' indicating the stripe can be handled,
> which is wrong.
> So add an extra test.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  drivers/md/raid5.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index c3ccefbd4fe7..9a803b735848 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -4192,9 +4192,13 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
>  
>  static int clear_batch_ready(struct stripe_head *sh)
>  {
> +	/* Return '1' if this is a member of batch, or
> +	 * '0' if it is a lone stripe or a head which can now be
> +	 * handled.
> +	 */
>  	struct stripe_head *tmp;
>  	if (!test_and_clear_bit(STRIPE_BATCH_READY, &sh->state))
> -		return 0;
> +		return (sh->batch_head && sh->batch_head != sh);
>  	spin_lock(&sh->stripe_lock);
>  	if (!sh->batch_head) {
>  		spin_unlock(&sh->stripe_lock);

which case can this happen in?

Patches look good. But I'm not in Fusionio any more, so can't check the
performance in big raid array with fast flash cards. I'm doing some tests here.
I hit a warning in break_stripe_batch_list, STRIPE_BIT_DELAY is set in the
stripe state. I'm checking the reason, but if you have thoughts I can try
immediately, please let me know.

Thanks,
Shaohua

^ permalink raw reply

* Re: [PATCH 2/8] md/raid5: Ensure a batch member is not handled prematurely.
From: NeilBrown @ 2015-05-23  0:26 UTC (permalink / raw)
  To: Shaohua Li; +Cc: linux-raid
In-Reply-To: <20150522234402.GA86128@kernel.org>

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

On Fri, 22 May 2015 16:44:02 -0700 Shaohua Li <shli@kernel.org> wrote:

> On Fri, May 22, 2015 at 03:30:58PM +1000, NeilBrown wrote:
> > If a stripe is a member of a batch, but not the head, it must
> > not be handled separately from the rest of the batch.
> > 
> > 'clear_batch_ready()' handles this requirement to some
> > extent but not completely.  If a member is passed to handle_stripe()
> > a second time it returns '0' indicating the stripe can be handled,
> > which is wrong.
> > So add an extra test.
> > 
> > Signed-off-by: NeilBrown <neilb@suse.de>
> > ---
> >  drivers/md/raid5.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> > index c3ccefbd4fe7..9a803b735848 100644
> > --- a/drivers/md/raid5.c
> > +++ b/drivers/md/raid5.c
> > @@ -4192,9 +4192,13 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s)
> >  
> >  static int clear_batch_ready(struct stripe_head *sh)
> >  {
> > +	/* Return '1' if this is a member of batch, or
> > +	 * '0' if it is a lone stripe or a head which can now be
> > +	 * handled.
> > +	 */
> >  	struct stripe_head *tmp;
> >  	if (!test_and_clear_bit(STRIPE_BATCH_READY, &sh->state))
> > -		return 0;
> > +		return (sh->batch_head && sh->batch_head != sh);
> >  	spin_lock(&sh->stripe_lock);
> >  	if (!sh->batch_head) {
> >  		spin_unlock(&sh->stripe_lock);
> 
> which case can this happen in?

It definitely happens as I had reliable problems until I added this fix.
'retry_aligned_read()' can call handle_stripe() on any stripe at any time,
but I doubt that would apply.  I might try putting a warn-on there and see if
it provides any hints.

> 
> Patches look good. But I'm not in Fusionio any more, so can't check the
> performance in big raid array with fast flash cards. I'm doing some tests here.
> I hit a warning in break_stripe_batch_list, STRIPE_BIT_DELAY is set in the
> stripe state. I'm checking the reason, but if you have thoughts I can try
> immediately, please let me know.

I got STRIPE_BIT_DELAY a few times.  That was the main reason for

  md/raid5: ensure whole batch is delayed for all required bitmap updates.

and they went away after I got that patch right.

Maybe there is a race in there..

If you can reproduce it, maybe WARN whenever STRIPE_BIT_DELAY gets set on a
stripe with ->batch_head.

> 
> Thanks,
> Shaohua

Thanks,
NeilBrown

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-24  1:18 UTC (permalink / raw)
  To: linux-raid
In-Reply-To: <CAOS+5GEP6+7OAHkqQjeyGHAB5u-_-Vq2JWGpcOemYHdCjmR5Lg@mail.gmail.com>

OK.

I've now read a LOT of stuff on this and have a much better
understanding.....but also probably some misunderstandings as well
(but I've learnt a lot so that's good).

I have now realised that I can actually store the RAID configuration
within the mdadm.conf file and therefore can incorporate that into the
initramfs configuration, however I have a few areas of uncertainty
that perhaps some people on the list could clarify.

1.  Fedora currently has a bug whereby efibootmgr during the install
won't play nice with efi systems....

https://fedoraproject.org/wiki/Common_F21_bugs#Cannot_place_bootloader_target_partition_.28e.g._.2Fboot.2Fefi.29_on_a_disk_other_than_the_first_in_custom_partitioning

This causes the install to fail as the bootloader doesn't get installed.

As Fedora installs GRUB2 as standard and based on what I had read
elsewhere I decided to create a bios_boot partition as the first
partition on the drives, during the install GRUB2 sees this and
configures it appropriately thereby bypassing the bug above.

So I now have 5 partitions.

a  -  bios_boot
b  -  efi
c  -  boot
d  -  root
e  -  swap

I'll be adding one more when I'm happy this is working.

f  -  home

2.  On my Asus mobo that I'm using it insists on treating the
installed Live USB as sda.....frankly a real pain in the a** as it
means during the first boot the root partition is mapped incorrectly
to sdb4, not a major problem but still annoying so if anyone knows a
way to 'force' a USB stick to not be the first device I'd love to hear
it.

3.  Using the methods above I have now created a bootable fedora
system, on a single drive in preparation to now RAID the required
partitions.  However my concern comes regarding the mdadm metadata,
simplistically metadata=1.2 apparently writes it's superblock to 4k
after the start of the device, this is exactly where my efi partition
(b above) starts, so my concern is will this superblock overwrite or
mess with my current partition table?

4.  If the next stage works then I think what I'll actually end up
doing is......

scrub what I have now.

create the arrays before running the Fedora Live installer (this
assumes the installer will see /md[x] devices and allow them to be
used to install to). Then incorporate the mdadm.conf data into the
initramfs and regenerate initramfs.

Ideas/Thoughts/Criticisms?

Tony





On 13 May 2015 at 01:02, Adam Goryachev
<mailinglists@websitemanagers.com.au> wrote:
> On 12/05/15 20:08, Another Sillyname wrote:
>>
>> I've tried to do some research on this but the information out there
>> seems a bit contradictory (mainly because some is so old).
>>
>> I want to install Fedora directly onto a RAID array (no separate boot
>> disk).
>>
>> My plan is to 'pre configure' the 6 drives as a clean RAID6 array,
>> effectively sd[a-f] without partitions and then attempt to install
>> Fedora 21, from sources it looks like Grub2 should recognise the array
>> and then allow the Kernel to boot thereby 'enabling' the array to
>> become visible and active.
>>
>> However I have not been able to find an actual example of someone
>> trying this......thoughts?
>>
>> The reason to do this is I'm intending to use a Mini ITX board with 6
>> sata ports and want to use 8TB drives in Raid6 to give me a very high
>> density data resilient small form factor storage box.
>>
>> Ideas/Suggestions?
>
>
> Create a small RAID1 partition at the beginning of every disk, use this for
> /boot and install grub. Use the rest of each disk as your RAID6 array.
>
> This means as long as the bios can find any one of your disks, then the
> system will boot. Personally, I'd be inclined to put the complete OS on the
> RAID1, and then only use the RAID6 for the "data" mountpoint. This would
> allow the system to come up fully, even without the RAID6 (eg, 4 out of 6
> drives has died), and allow remote debug/diagnostics/etc without needing to
> try and boot from a rescue disk/etc.
>
> This doesn't help you with what you asked, but just an idea/suggestion.
>
> Regards,
> Adam
>
> --
> Adam Goryachev Website Managers www.websitemanagers.com.au

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-24  8:36 UTC (permalink / raw)
  To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GHKwQRPfNAw0EpSjjDUt=9PmjjrkUg+dhGxFS+uOWWVug@mail.gmail.com>

On Sun, 24 May 2015, Another Sillyname wrote:

> So I now have 5 partitions.
>
> a  -  bios_boot
> b  -  efi
> c  -  boot
> d  -  root
> e  -  swap
>
> I'll be adding one more when I'm happy this is working.
>
> f  -  home
>
> 3.  Using the methods above I have now created a bootable fedora
> system, on a single drive in preparation to now RAID the required
> partitions.  However my concern comes regarding the mdadm metadata,
> simplistically metadata=1.2 apparently writes it's superblock to 4k
> after the start of the device, this is exactly where my efi partition
> (b above) starts, so my concern is will this superblock overwrite or
> mess with my current partition table?
>
> 4.  If the next stage works then I think what I'll actually end up
> doing is......
>
> scrub what I have now.
>
> create the arrays before running the Fedora Live installer (this
> assumes the installer will see /md[x] devices and allow them to be
> used to install to). Then incorporate the mdadm.conf data into the
> initramfs and regenerate initramfs.
>
> Ideas/Thoughts/Criticisms?

You don't want to run MD on the entire drive in this case, you most likely 
want to create multiple RAID1 and RAID6 mirrors. RAID1 your boot, root and 
swap, then run RAID6 on your home partition. Use use superblock type that 
creates the superblock at the end for the RAID1 partitions.

Also, you don't want to refer to "sda" when booting, you want to use 
UUID=<uuid> in fstab, crypttab etc.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-24  9:08 UTC (permalink / raw)
  To: Mikael Abrahamsson, linux-raid
In-Reply-To: <alpine.DEB.2.02.1505241032210.9487@uplift.swm.pp.se>

I suspect you're correct in that I'll end up with the boot partitions
being in RAID1 and the data in RAID6, however I am seriously
considering having the boot in RAID6 as well...if I can integrate the
mdadm.conf into the initramfs properly I can't see a reason not to do
this?

Had a look at the metadata=0.9 option but reading the info on mdadm
metadata I think I'd prefer to have the metadata at the start of the
drive, also it looks like metadata=1.2 has extra functionality that I
may want to use later.

On 24 May 2015 at 09:36, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> On Sun, 24 May 2015, Another Sillyname wrote:
>
>> So I now have 5 partitions.
>>
>> a  -  bios_boot
>> b  -  efi
>> c  -  boot
>> d  -  root
>> e  -  swap
>>
>> I'll be adding one more when I'm happy this is working.
>>
>> f  -  home
>>
>> 3.  Using the methods above I have now created a bootable fedora
>> system, on a single drive in preparation to now RAID the required
>> partitions.  However my concern comes regarding the mdadm metadata,
>> simplistically metadata=1.2 apparently writes it's superblock to 4k
>> after the start of the device, this is exactly where my efi partition
>> (b above) starts, so my concern is will this superblock overwrite or
>> mess with my current partition table?
>>
>> 4.  If the next stage works then I think what I'll actually end up
>> doing is......
>>
>> scrub what I have now.
>>
>> create the arrays before running the Fedora Live installer (this
>> assumes the installer will see /md[x] devices and allow them to be
>> used to install to). Then incorporate the mdadm.conf data into the
>> initramfs and regenerate initramfs.
>>
>> Ideas/Thoughts/Criticisms?
>
>
> You don't want to run MD on the entire drive in this case, you most likely
> want to create multiple RAID1 and RAID6 mirrors. RAID1 your boot, root and
> swap, then run RAID6 on your home partition. Use use superblock type that
> creates the superblock at the end for the RAID1 partitions.
>
> Also, you don't want to refer to "sda" when booting, you want to use
> UUID=<uuid> in fstab, crypttab etc.
>
> --
> Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-24  9:46 UTC (permalink / raw)
  To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GEZN5d41tGxN+PP6S3ynSWnrXaNK+27rxJyXa-zw1KFaw@mail.gmail.com>

On Sun, 24 May 2015, Another Sillyname wrote:

> I suspect you're correct in that I'll end up with the boot partitions
> being in RAID1 and the data in RAID6, however I am seriously
> considering having the boot in RAID6 as well...if I can integrate the
> mdadm.conf into the initramfs properly I can't see a reason not to do
> this?

The initramfs is usually located on the boot partition. A raid1 partition 
with a filesystem can be read by the bootloader even if the bootloader 
doesn't understand to start the raid first. This is not possible with 
raid6.

So /boot is usually raid1.

> Had a look at the metadata=0.9 option but reading the info on mdadm
> metadata I think I'd prefer to have the metadata at the start of the
> drive, also it looks like metadata=1.2 has extra functionality that I
> may want to use later.

Look at v1.0 superblock, it has the same functionality but stores the 
superblock at the end.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-24 10:07 UTC (permalink / raw)
  To: Mikael Abrahamsson; +Cc: linux-raid
In-Reply-To: <alpine.DEB.2.02.1505241142350.9487@uplift.swm.pp.se>

But reading this.....

https://raid.wiki.kernel.org/index.php/RAID_superblock_formats#The_version-1_Superblock_Format

it makes a sound argument for the metadata superblock being at the
beginning of the drive.......in a nutshell the kernel can 'construct'
the md device easier if the metadata is at the beginning....as well as
data resilience issues in the case of a crash.



On 24 May 2015 at 10:46, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> On Sun, 24 May 2015, Another Sillyname wrote:
>
>> I suspect you're correct in that I'll end up with the boot partitions
>> being in RAID1 and the data in RAID6, however I am seriously
>> considering having the boot in RAID6 as well...if I can integrate the
>> mdadm.conf into the initramfs properly I can't see a reason not to do
>> this?
>
>
> The initramfs is usually located on the boot partition. A raid1 partition
> with a filesystem can be read by the bootloader even if the bootloader
> doesn't understand to start the raid first. This is not possible with raid6.
>
> So /boot is usually raid1.
>
>> Had a look at the metadata=0.9 option but reading the info on mdadm
>> metadata I think I'd prefer to have the metadata at the start of the
>> drive, also it looks like metadata=1.2 has extra functionality that I
>> may want to use later.
>
>
> Look at v1.0 superblock, it has the same functionality but stores the
> superblock at the end.
>
>
> --
> Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-24 10:35 UTC (permalink / raw)
  To: Another Sillyname; +Cc: linux-raid
In-Reply-To: <CAOS+5GF22YN8OagwQiXy4gxn94eTLt-X31VhWoU47r_kgLLYnQ@mail.gmail.com>

On Sun, 24 May 2015, Another Sillyname wrote:

> But reading this.....
>
> https://raid.wiki.kernel.org/index.php/RAID_superblock_formats#The_version-1_Superblock_Format
>
> it makes a sound argument for the metadata superblock being at the
> beginning of the drive.......in a nutshell the kernel can 'construct'
> the md device easier if the metadata is at the beginning....as well as
> data resilience issues in the case of a crash.

I don't see how you have any choice in case of /boot. I am a strong 
supporter of v1.2 superblock (4k into the partition) as can be seen from 
the discussion about this some years back, but in the case of /boot when 
the boot loader needs to access files on /boot before the raid is started, 
I don't see how you have a choice. So stay away from v0.90, but v1.0 has 
the same functionality apart from where the superblock is placed.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Another Sillyname @ 2015-05-24 10:42 UTC (permalink / raw)
  To: Mikael Abrahamsson, linux-raid
In-Reply-To: <alpine.DEB.2.02.1505241233540.9487@uplift.swm.pp.se>

That's the whole point of trying this test......

As I have a working booting machine with currently 8 drives.......all
partitioned as above.

I have just created RAID5 mdadm setups on drives [d-h] on partitions
[2-5], once they've synced I'm going to see if I can install Live USB
Fedora to them and then boot from there.  If it doesn't work I've just
lost a bit of time.



On 24 May 2015 at 11:35, Mikael Abrahamsson <swmike@swm.pp.se> wrote:
> On Sun, 24 May 2015, Another Sillyname wrote:
>
>> But reading this.....
>>
>>
>> https://raid.wiki.kernel.org/index.php/RAID_superblock_formats#The_version-1_Superblock_Format
>>
>> it makes a sound argument for the metadata superblock being at the
>> beginning of the drive.......in a nutshell the kernel can 'construct'
>> the md device easier if the metadata is at the beginning....as well as
>> data resilience issues in the case of a crash.
>
>
> I don't see how you have any choice in case of /boot. I am a strong
> supporter of v1.2 superblock (4k into the partition) as can be seen from the
> discussion about this some years back, but in the case of /boot when the
> boot loader needs to access files on /boot before the raid is started, I
> don't see how you have a choice. So stay away from v0.90, but v1.0 has the
> same functionality apart from where the superblock is placed.
>
>
> --
> Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Wols Lists @ 2015-05-24 11:12 UTC (permalink / raw)
  To: Mikael Abrahamsson, Another Sillyname; +Cc: linux-raid
In-Reply-To: <alpine.DEB.2.02.1505241032210.9487@uplift.swm.pp.se>

On 24/05/15 09:36, Mikael Abrahamsson wrote:
> You don't want to run MD on the entire drive in this case, you most
> likely want to create multiple RAID1 and RAID6 mirrors. RAID1 your boot,
> root and swap, then run RAID6 on your home partition. Use use superblock
> type that creates the superblock at the end for the RAID1 partitions.

Don't bother raid'ing swap at all! If you set the priority equal on all
the swaps, linux will do a raid 0 for you and, frankly, what's the point
of doing raid on your swap partition? You really shouldn't (in normal
usage) be using swap at all.

NB - with swap, I always create one swap partition per drive, and make
it twice the maximum ram the mobo will take. I have still not had
anybody come back to me with any evidence that the swap algorithm
doesn't work better with that amount of swap, and the belief that "twice
ram no longer applies" was PROVEN to be an urban myth with linux 2.4
(yes, that was a long time ago, but as I said no-one has ever given me
any evidence that things have changed since).

I'm a bit unusual, running gentoo I need large chunks of temporary space
so I have a couple of huge tmpfs drives.

Cheers,
Wol

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Brad Campbell @ 2015-05-24 11:57 UTC (permalink / raw)
  To: Wols Lists, Mikael Abrahamsson, Another Sillyname; +Cc: linux-raid
In-Reply-To: <5561B237.4040709@youngman.org.uk>

On 24/05/15 19:12, Wols Lists wrote:

> Don't bother raid'ing swap at all! If you set the priority equal on all
> the swaps, linux will do a raid 0 for you and, frankly, what's the point
> of doing raid on your swap partition? You really shouldn't (in normal
> usage) be using swap at all.

When you develop a bad block in one of your non-RAID swap partitions and 
you spend a week trying to figure out why processes randomly die for no 
apparent reason you'll reconsider that attitude.

Brad
-- 
Dolphins are so intelligent that within a few weeks they can
train Americans to stand at the edge of the pool and throw them
fish.

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Wols Lists @ 2015-05-24 12:41 UTC (permalink / raw)
  To: Brad Campbell, Mikael Abrahamsson, Another Sillyname; +Cc: linux-raid
In-Reply-To: <5561BCA4.4060904@fnarfbargle.com>

On 24/05/15 12:57, Brad Campbell wrote:
> On 24/05/15 19:12, Wols Lists wrote:
> 
>> Don't bother raid'ing swap at all! If you set the priority equal on all
>> the swaps, linux will do a raid 0 for you and, frankly, what's the point
>> of doing raid on your swap partition? You really shouldn't (in normal
>> usage) be using swap at all.
> 
> When you develop a bad block in one of your non-RAID swap partitions and
> you spend a week trying to figure out why processes randomly die for no
> apparent reason you'll reconsider that attitude.
> 
aiui raid won't help you at all here, will it?

Firstly, if you get write failures, the hard drive should swap the
block, or the write layer should swap it. Nothing to do with raid
whatsoever.

And if you get read errors, well, aiui, raid won't help here either -
especially with mirrored raid, you just get a read failure. Raid does
NOT give you error recovery unless the drive physically fails, and if
it's a bad block it gets fixed at the disk or disk driver level - well
below the raid driver.

Cheers,
Wol

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Brad Campbell @ 2015-05-24 13:48 UTC (permalink / raw)
  To: Wols Lists, Mikael Abrahamsson, Another Sillyname; +Cc: linux-raid
In-Reply-To: <5561C6E9.3060706@youngman.org.uk>

On 24/05/15 20:41, Wols Lists wrote:

> aiui raid won't help you at all here, will it?

Of course it will.

> Firstly, if you get write failures, the hard drive should swap the
> block, or the write layer should swap it. Nothing to do with raid
> whatsoever.

Yep..

> And if you get read errors, well, aiui, raid won't help here either -
> especially with mirrored raid, you just get a read failure. Raid does
> NOT give you error recovery unless the drive physically fails, and if
> it's a bad block it gets fixed at the disk or disk driver level - well
> below the raid driver.

Nope. If you get a read failure it will try and pull from another 
mirror, and *that* will succeed. Drives don't return dud* data. They 
either succeed or fail.

* Of course I've heard anecdotes from people about that happening, but 
it's not supposed to happen under *any* circumstances.

-- 
Dolphins are so intelligent that within a few weeks they can
train Americans to stand at the edge of the pool and throw them
fish.

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-24 14:06 UTC (permalink / raw)
  To: Wols Lists; +Cc: Brad Campbell, Another Sillyname, linux-raid
In-Reply-To: <5561C6E9.3060706@youngman.org.uk>

On Sun, 24 May 2015, Wols Lists wrote:

> And if you get read errors, well, aiui, raid won't help here either - 
> especially with mirrored raid, you just get a read failure. Raid does 
> NOT give you error recovery unless the drive physically fails, and if 
> it's a bad block it gets fixed at the disk or disk driver level - well 
> below the raid driver.

You're wrong. In case of a read error from the physical drive on RAID1, 
RAID5 or RAID6 then the information will be re-created from another drive, 
and written to the drive that threw a read error. This is the whole point 
of RAID with parity information.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Wols Lists @ 2015-05-24 14:53 UTC (permalink / raw)
  To: Mikael Abrahamsson; +Cc: Brad Campbell, Another Sillyname, linux-raid
In-Reply-To: <alpine.DEB.2.02.1505241604370.9487@uplift.swm.pp.se>

On 24/05/15 15:06, Mikael Abrahamsson wrote:
> On Sun, 24 May 2015, Wols Lists wrote:
> 
>> And if you get read errors, well, aiui, raid won't help here either -
>> especially with mirrored raid, you just get a read failure. Raid does
>> NOT give you error recovery unless the drive physically fails, and if
>> it's a bad block it gets fixed at the disk or disk driver level - well
>> below the raid driver.
> 
> You're wrong. In case of a read error from the physical drive on RAID1,
> RAID5 or RAID6 then the information will be re-created from another
> drive, and written to the drive that threw a read error. This is the
> whole point of RAID with parity information.
> 
Except raid 1 isn't parity ... :-)

Personally, I still don't think "raid"ing swap is worth it, though.
Horses for courses, ram is cheap, and in my circumstances I don't think
I'd gain anything.

Cheers,
Wol


^ permalink raw reply

* Re: Fwd: Installing Linux directly onto RAID6 Array...........
From: Mikael Abrahamsson @ 2015-05-24 14:57 UTC (permalink / raw)
  To: Wols Lists; +Cc: Brad Campbell, Another Sillyname, linux-raid
In-Reply-To: <5561E5CE.7000304@youngman.org.uk>

On Sun, 24 May 2015, Wols Lists wrote:

> On 24/05/15 15:06, Mikael Abrahamsson wrote:
>> On Sun, 24 May 2015, Wols Lists wrote:
>>
>>> And if you get read errors, well, aiui, raid won't help here either -
>>> especially with mirrored raid, you just get a read failure. Raid does
>>> NOT give you error recovery unless the drive physically fails, and if
>>> it's a bad block it gets fixed at the disk or disk driver level - well
>>> below the raid driver.
>>
>> You're wrong. In case of a read error from the physical drive on RAID1,
>> RAID5 or RAID6 then the information will be re-created from another
>> drive, and written to the drive that threw a read error. This is the
>> whole point of RAID with parity information.
>>
> Except raid 1 isn't parity ... :-)

RAID1 means every drive will have the same information, it's mirrored 
between the member disks. What do you think RAID1 is?

> Personally, I still don't think "raid"ing swap is worth it, though.
> Horses for courses, ram is cheap, and in my circumstances I don't think
> I'd gain anything.

You're welcome to believe anything you want, but if you're publically 
telling people things that are just not true then you should expect to be 
told so.

You're welcome to tell people to not use SWAP at all, but telling people 
RAID1 has no benefit for SWAP because it won't protect you from read 
erorrs is just wrong.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Does --freeze-reshape only work during initrd boot phase?
From: NeilBrown @ 2015-05-25  2:20 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid
In-Reply-To: <1625238601.2947919.1432290609781.JavaMail.zimbra@redhat.com>

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

On Fri, 22 May 2015 06:30:09 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:

> Hi all
> 
>    I'm trying to use --freeze-reshape, I search how to use it, there is just some 
> explanation in man mdadm. And I see the main usage about this is during initrd boot phase.
> Does --freeze-reshape only work during initrd boot phase?

No it can be used at any time.
However it doesn't apply to all array types, and that isn't clear from the
documentation.

--freeze-reshape is only relevant for arrays which require a user-space
process to help with the reshape - specifically to make temporary backups of
portions of the array while they are being reshaped.
If --freeze-reshape is given, then when mdadm discovers that it needs to
start such a helper, it doesn't and just leaves reshape frozen.
If it never finds that a helper is needed, there is no value in freezing the
reshape.

I should probably improve the documentation.

NeilBrown

> 
>    I tried this after the machine started. The steps are as follows:
> [root@intel-canoepass-02 tmp]# mdadm -CR /dev/md0 -l5 -n5 /dev/loop[0-4] --assume-clean
> mdadm: /dev/loop0 appears to be part of a raid array:
>        level=raid5 devices=5 ctime=Fri May 22 06:14:35 2015
> mdadm: /dev/loop1 appears to be part of a raid array:
>        level=raid5 devices=5 ctime=Fri May 22 06:14:35 2015
> mdadm: /dev/loop2 appears to be part of a raid array:
>        level=raid5 devices=5 ctime=Fri May 22 06:14:35 2015
> mdadm: /dev/loop3 appears to be part of a raid array:
>        level=raid5 devices=5 ctime=Fri May 22 06:14:35 2015
> mdadm: /dev/loop4 appears to be part of a raid array:
>        level=raid5 devices=5 ctime=Fri May 22 06:14:35 2015
> mdadm: Defaulting to version 1.2 metadata
> mdadm: array /dev/md0 started.
> [root@intel-canoepass-02 tmp]# mdadm /dev/md0 -a /dev/loop5 
> mdadm: added /dev/loop5
> [root@intel-canoepass-02 tmp]# mdadm -Es
> ARRAY /dev/md/0  metadata=1.2 UUID=24d3d007:8ec28d4e:edb482fe:db7cab71 name=0
>    spares=1
> [root@intel-canoepass-02 tmp]# mdadm -Es > /etc/mdadm.conf 
> [root@intel-canoepass-02 tmp]# mdadm --grow /dev/md0 --raid-devices 6
> [root@intel-canoepass-02 tmp]# cat /proc/mdstat 
> Personalities : [raid6] [raid5] [raid4] 
> md0 : active raid5 loop5[5] loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
>       2043904 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6] [UUUUUU]
>       [====>................]  reshape = 23.9% (122368/510976) finish=0.8min speed=7648K/sec
>       
> unused devices: <none>
> [root@intel-canoepass-02 tmp]# mdadm -S /dev/md0 
> mdadm: stopped /dev/md0
> [root@intel-canoepass-02 tmp]# mdadm -As --freeze-reshape
> mdadm: /dev/md/0 has been started with 6 drives.
> [root@intel-canoepass-02 tmp]# cat /proc/mdstat 
> Personalities : [raid6] [raid5] [raid4] 
> md0 : active raid5 loop0[0] loop5[5] loop4[4] loop3[3] loop2[2] loop1[1]
>       2043904 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6] [UUUUUU]
>       [=========>...........]  reshape = 46.9% (240128/510976) finish=0.1min speed=23808K/sec
>       
> unused devices: <none>
> 
>    The reshape is not freeze. I think it should be freeze now, am I right? The reason I want to
> use this is that I want to use --grow --continue. I see --continue is used when reshape is 
> interrupted. 
> 
> Best Regards
> Xiao
> --
> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: raid5 reshape is stuck
From: NeilBrown @ 2015-05-25  3:50 UTC (permalink / raw)
  To: Xiao Ni; +Cc: linux-raid
In-Reply-To: <1822959676.2432469.1432211518647.JavaMail.zimbra@redhat.com>

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

On Thu, 21 May 2015 08:31:58 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:

> 
> 
> ----- Original Message -----
> > From: "Xiao Ni" <xni@redhat.com>
> > To: "NeilBrown" <neilb@suse.de>
> > Cc: linux-raid@vger.kernel.org
> > Sent: Thursday, May 21, 2015 11:37:57 AM
> > Subject: Re: raid5 reshape is stuck
> > 
> > 
> > 
> > ----- Original Message -----
> > > From: "NeilBrown" <neilb@suse.de>
> > > To: "Xiao Ni" <xni@redhat.com>
> > > Cc: linux-raid@vger.kernel.org
> > > Sent: Thursday, May 21, 2015 7:48:37 AM
> > > Subject: Re: raid5 reshape is stuck
> > > 
> > > On Fri, 15 May 2015 03:00:24 -0400 (EDT) Xiao Ni <xni@redhat.com> wrote:
> > > 
> > > > Hi Neil
> > > > 
> > > >    I encounter the problem when I reshape a 4-disks raid5 to raid5. It
> > > >    just
> > > >    can
> > > > appear with loop devices.
> > > > 
> > > >    The steps are:
> > > > 
> > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm -CR /dev/md0 -l5 -n5 /dev/loop[0-4]
> > > > --assume-clean
> > > > mdadm: /dev/loop0 appears to be part of a raid array:
> > > >        level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > mdadm: /dev/loop1 appears to be part of a raid array:
> > > >        level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > mdadm: /dev/loop2 appears to be part of a raid array:
> > > >        level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > mdadm: /dev/loop3 appears to be part of a raid array:
> > > >        level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > mdadm: /dev/loop4 appears to be part of a raid array:
> > > >        level=raid5 devices=6 ctime=Fri May 15 13:47:17 2015
> > > > mdadm: Defaulting to version 1.2 metadata
> > > > mdadm: array /dev/md0 started.
> > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm /dev/md0 -a /dev/loop5
> > > > mdadm: added /dev/loop5
> > > > [root@dhcp-12-158 mdadm-3.3.2]# mdadm --grow /dev/md0 --raid-devices 6
> > > > mdadm: Need to backup 10240K of critical section..
> > > > [root@dhcp-12-158 mdadm-3.3.2]# cat /proc/mdstat
> > > > Personalities : [raid6] [raid5] [raid4]
> > > > md0 : active raid5 loop5[5] loop4[4] loop3[3] loop2[2] loop1[1] loop0[0]
> > > >       8187904 blocks super 1.2 level 5, 512k chunk, algorithm 2 [6/6]
> > > >       [UUUUUU]
> > > >       [>....................]  reshape =  0.0% (0/2046976)
> > > >       finish=6396.8min
> > > >       speed=0K/sec
> > > >       
> > > > unused devices: <none>
> > > > 
> > > >    It because the sync_max is set to 0 when run the command --grow
> > > > 
> > > > [root@dhcp-12-158 mdadm-3.3.2]# cd /sys/block/md0/md/
> > > > [root@dhcp-12-158 md]# cat sync_max
> > > > 0
> > > > 
> > > >    I tried reproduce with normal sata devices. The progress of reshape is
> > > >    no problem. Then
> > > > I checked the Grow.c. If I use sata devices, in function reshape_array,
> > > > the
> > > > return value
> > > > of set_new_data_offset is 0. But if I used loop devices, it return 1.
> > > > Then
> > > > it call the function
> > > > start_reshape.
> > > 
> > > set_new_data_offset returns '0' if there is room on the devices to reduce
> > > the
> > > data offset so that the reshape starts writing to unused space on the
> > > array.
> > > This removes the need for a backup file, or the use of a spare device to
> > > store a temporary backup.
> > > It returns '1' if there was no room for relocating the data_offset.
> > > 
> > > So on your sata devices (which are presumably larger than your loop
> > > devices)
> > > there was room.  On your loop devices there was not.
> > > 
> > > 
> > > > 
> > > >    In the function start_reshape it set the sync_max to reshape_progress.
> > > >    But in sysfs_read it
> > > > doesn't read reshape_progress. So it's 0 and the sync_max is set to 0.
> > > > Why
> > > > it need to set the
> > > > sync_max at this? I'm not sure about this.
> > > 
> > > sync_max is set to 0 so that the reshape does not start until the backup
> > > has
> > > been taken.
> > > Once the backup is taken, child_monitor() should set sync_max to "max".
> > > 
> > > Can you  check if that is happening?
> > > 
> > > Thanks,
> > > NeilBrown
> > > 
> > > 
> > 
> >   Thanks very much for the explaining. The problem maybe is fixed. I tried
> >   reproduce this with newest
> > kernel and newest mdadm. Now the problem don't exist. I'll do more tests and
> > give the answer above later.
> > 
> 
> Hi Neil
> 
>    As you said, it doesn't enter child monitor. The problem still exist.
> 
> The kernel version :
> [root@intel-canoepass-02 tmp]# uname -r
> 4.0.4
> 
> mdadm I used is the newest git code from git://git.neil.brown.name/mdadm.git
> 
>    
>    In the function continue_via_systemd the parent find pid is bigger than 0 and
> status is 0. So it return 1. So it have no opportunity to call child_monitor.

If continue_via_systemd succeeded, that implies that 
  systemctl start mdadm-grow-continue@mdXXX.service

succeeded.  So 
   mdadm --grow --continue /dev/mdXXX

was run, so that mdadm should call 'child_monitor' and update sync_max when
appropriate.  Can you check if it does?


> 
> 
>    And if it want to set sync_max to 0 until the backup has been taken. Why does not 
> set sync_max to 0 directly, but use the value reshape_progress? There is a little confused.

When reshaping an array to a different array of the same size, such as a
4-driver RAID5 to a 5-driver RAID6, then mdadm needs to backup, one piece at
a time, the entire array (unless it can change data_offset, which is a
relatively new ability).

If you stop an array when it is in the middle of such a reshape, and then
reassemble the array, the backup process need to recommence where it left off.
So it tells the kernel that the reshape can progress as far as where it was
up to before.  So 'sync_max' is set based on the value of 'reshape_progress'.
(This will happen almost instantly).

Then the background mdadm (or the mdadm started by systemd) will backup the
next few stripes, update sync_max, wait for those stripes to be reshaped, then
discard the old backup, create a new one of the few stripes after that, and
continue.

Does that make it a little clearer?

And in response to your other email:
>     Does it should return 1 when pid > 0 and status is not zero?

No.  continue_via_systemd should return 1 precisely when the 'systemctl'
command was successfully run.  So 'status' must be zero.


Thanks,
NeilBrown




> 
> Best Regards
> Xiao
> --
> 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH V3 02/11] Add nodes option while creating md
From: NeilBrown @ 2015-05-25  4:13 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1432092043-24220-3-git-send-email-gqjiang@suse.com>

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

On Wed, 20 May 2015 11:20:34 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:

> 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@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  Create.c   |  1 +
>  ReadMe.c   |  1 +
>  mdadm.8.in |  6 ++++++
>  mdadm.c    | 34 +++++++++++++++++++++++++++++++++-
>  mdadm.h    |  3 +++
>  super1.c   |  1 +
>  6 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/Create.c b/Create.c
> index 69f5432..e4577af 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 4aec0db..9c1497e 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -971,6 +971,12 @@ 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
> +Only works when the array is for clustered environment. It specify the
> +maximum number of nodes in the cluster that will use this device
> +simultaneously. If not specified, this defaults to 4.
> +

"It specifies the maximum"...

>  .SH For assemble:
>  
>  .TP
> diff --git a/mdadm.c b/mdadm.c
> index 3e8c49b..15a43d2 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);

Please don't break strings on to multiple lines.
    pr_err("invalid number for the number of cluster node: %s\n",
           optarg);

it doesn't matter if it exceeds 80 columns for this case.


> +				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",
> @@ -1097,6 +1104,15 @@ int main(int argc, char *argv[])
>  				s.bitmap_file = optarg;
>  				continue;
>  			}
> +			if (strcmp(optarg, "clustered")== 0) {
> +			    s.bitmap_file = optarg;
> +			    /* Set the default number of cluster nodes
> +			     * to 4 if not already set by user
> +			     */
> +			    if (c.nodes < 1)
> +				    c.nodes = 4;
> +			    continue;
> +			}
>  			/* probable typo */
>  			pr_err("bitmap file must contain a '/', or be 'internal', or 'none'\n"
>  				"       not '%s'\n", optarg);
> @@ -1377,6 +1393,22 @@ int main(int argc, char *argv[])
>  	case CREATE:
>  		if (c.delay == 0)
>  			c.delay = DEFAULT_BITMAP_DELAY;
> +
> +		if (strcmp(s.bitmap_file, "clustered") == 0) {
> +			if (s.level != 1) {
> +			    pr_err("--bitmap=clustered is currently supported with RAID mirror only\n");
> +			    rv = 1;
> +			    break;
> +			}
> +		} else {
> +			if (c.nodes) {
> +				pr_err("--nodes argument is incompatible with --bitmap=%s.\n",
> +					s.bitmap_file);
> +				rv = 1;
> +				break;
> +			}

That's a bit careless....

 ./mdadm -C /dev/md0 -l1 -n2 --nodes=5 /dev/loop[01]
 Segmentation fault


NeilBrown



> +		}
> +
>  		if (s.write_behind && !s.bitmap_file) {
>  			pr_err("write-behind mode requires a bitmap.\n");
>  			rv = 1;
> diff --git a/mdadm.h b/mdadm.h
> index 141f963..9d55801 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -344,6 +344,7 @@ enum special_options {
>  	Dump,
>  	Restore,
>  	Action,
> +	Nodes,
>  };
>  
>  enum prefix_standard {
> @@ -418,6 +419,7 @@ struct context {
>  	char	*backup_file;
>  	int	invalid_backup;
>  	char	*action;
> +	int	nodes;
>  };
>  
>  struct shape {
> @@ -1029,6 +1031,7 @@ struct supertype {
>  			 */
>  	int devcnt;
>  	int retry_soon;
> +	int nodes;
>  
>  	struct mdinfo *devs;
>  
> diff --git a/super1.c b/super1.c
> index 7928a3d..78d98a7 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -2144,6 +2144,7 @@ add_internal_bitmap1(struct supertype *st,
>  	bms->daemon_sleep = __cpu_to_le32(delay);
>  	bms->sync_size = __cpu_to_le64(size);
>  	bms->write_behind = __cpu_to_le32(write_behind);
> +	bms->nodes = __cpu_to_le32(st->nodes);
>  
>  	*chunkp = chunk;
>  	return 1;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH V3 03/11] home-cluster while creating an array
From: NeilBrown @ 2015-05-25  4:19 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1432092043-24220-4-git-send-email-gqjiang@suse.com>

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

On Wed, 20 May 2015 11:20:35 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:

> The home-cluster is stored in the bitmap super block of the
> array. The device can be assembled on a cluster with the
> cluster name same as the one recorded in the bitmap.
> 
> If home-cluster is not specified, this is auto-detected using
> dlopen corosync cmap library.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  Create.c   |  1 +
>  Makefile   |  1 +
>  ReadMe.c   |  1 +
>  config.c   | 27 ++++++++++++++++++++++++++-
>  mdadm.8.in |  6 ++++++
>  mdadm.c    | 25 +++++++++++++++++++++++++
>  mdadm.h    |  5 +++++
>  super1.c   |  3 +++
>  util.c     | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  9 files changed, 118 insertions(+), 1 deletion(-)
> 
> diff --git a/Create.c b/Create.c
> index e4577af..9663dc4 100644
> --- a/Create.c
> +++ b/Create.c
> @@ -532,6 +532,7 @@ int Create(struct supertype *st, char *mddev,
>  		warn = 1;
>  	}
>  	st->nodes = c->nodes;
> +	st->cluster_name = c->homecluster;
>  
>  	if (warn) {
>  		if (c->runstop!= 1) {
> diff --git a/Makefile b/Makefile
> index a7d8c5c..431f08b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -101,6 +101,7 @@ endif
>  # If you want a static binary, you might uncomment these
>  # LDFLAGS = -static
>  # STRIP = -s
> +LDLIBS=-ldl
>  
>  INSTALL = /usr/bin/install
>  DESTDIR =
> diff --git a/ReadMe.c b/ReadMe.c
> index 30c569d..c6286ae 100644
> --- a/ReadMe.c
> +++ b/ReadMe.c
> @@ -141,6 +141,7 @@ struct option long_options[] = {
>      {"symlinks",  1, 0,  Symlinks},
>      {"data-offset",1, 0, DataOffset},
>      {"nodes",1, 0, Nodes},
> +    {"home-cluster",1, 0, ClusterName},
>  
>      /* For assemble */
>      {"uuid",      1, 0, 'u'},
> diff --git a/config.c b/config.c
> index 7342c42..21b6afd 100644
> --- a/config.c
> +++ b/config.c
> @@ -77,7 +77,7 @@ char DefaultAltConfFile[] = CONFFILE2;
>  char DefaultAltConfDir[] = CONFFILE2 ".d";
>  
>  enum linetype { Devices, Array, Mailaddr, Mailfrom, Program, CreateDev,
> -		Homehost, AutoMode, Policy, PartPolicy, LTEnd };
> +		Homehost, HomeCluster, AutoMode, Policy, PartPolicy, LTEnd };
>  char *keywords[] = {
>  	[Devices]  = "devices",
>  	[Array]    = "array",
> @@ -86,6 +86,7 @@ char *keywords[] = {
>  	[Program]  = "program",
>  	[CreateDev]= "create",
>  	[Homehost] = "homehost",
> +	[HomeCluster] = "homecluster",
>  	[AutoMode] = "auto",
>  	[Policy]   = "policy",
>  	[PartPolicy]="part-policy",
> @@ -562,6 +563,21 @@ void homehostline(char *line)
>  	}
>  }
>  
> +static char *home_cluster = NULL;
> +void homeclusterline(char *line)
> +{
> +	char *w;
> +
> +	for (w=dl_next(line); w != line ; w=dl_next(w)) {
> +		if (home_cluster == NULL) {
> +			if (strcasecmp(w, "<none>")==0)
> +				home_cluster = xstrdup("");
> +			else
> +				home_cluster = xstrdup(w);
> +		}
> +	}
> +}
> +
>  char auto_yes[] = "yes";
>  char auto_no[] = "no";
>  char auto_homehost[] = "homehost";
> @@ -724,6 +740,9 @@ void conf_file(FILE *f)
>  		case Homehost:
>  			homehostline(line);
>  			break;
> +		case HomeCluster:
> +			homeclusterline(line);
> +			break;
>  		case AutoMode:
>  			autoline(line);
>  			break;
> @@ -884,6 +903,12 @@ char *conf_get_homehost(int *require_homehostp)
>  	return home_host;
>  }
>  
> +char *conf_get_homecluster(void)
> +{
> +	load_conffile();
> +	return home_cluster;
> +}
> +
>  struct createinfo *conf_get_create_info(void)
>  {
>  	load_conffile();
> diff --git a/mdadm.8.in b/mdadm.8.in
> index 9c1497e..56d9bcc 100644
> --- a/mdadm.8.in
> +++ b/mdadm.8.in
> @@ -415,6 +415,12 @@ This functionality is currently only provided by
>  and
>  .BR \-\-monitor .
>  
> +.TP
> +.B \-\-home\-cluster=
> +specifies the cluster name for the md device. The md device can be assembled
> +only on the cluster which matches the name specified. If this option is not
> +provided, mdadm tried to detect the cluster name automatically.

"... mdadm tries to detect...."   s, not d.


> +
>  .SH For create, build, or grow:
>  
>  .TP
> diff --git a/mdadm.c b/mdadm.c
> index 15a43d2..8f567d8 100644
> --- a/mdadm.c
> +++ b/mdadm.c
> @@ -596,6 +596,13 @@ int main(int argc, char *argv[])
>  				exit(2);
>  			}
>  			continue;
> +		case O(CREATE, ClusterName):
> +			c.homecluster = optarg;
> +			if (strlen(c.homecluster) > 64) {
> +				pr_err("Cluster name too big.\n");
> +				exit(ERANGE);
> +			}
> +			continue;
>  		case O(CREATE,'x'): /* number of spare (eXtra) disks */
>  			if (s.sparedisks) {
>  				pr_err("spare-devices set twice: %d and %s\n",
> @@ -1276,6 +1283,18 @@ int main(int argc, char *argv[])
>  		c.require_homehost = 0;
>  	}
>  
> +	if (c.homecluster == NULL && (c.nodes > 0)) {
> +		c.homecluster = conf_get_homecluster();
> +		if (c.homecluster == NULL)
> +			rv = get_cluster_name(&c.homecluster);
> +		if (rv == 0) {
> +			c.homehost = xstrdup(c.homecluster);
> +			/* Add a : to differentiate between a host
> +			 * and a cluster */
> +			strcat(c.homehost, ":");
> +		}

That ':' will be copied over the last byte allocated by 'strdup', and the
'\0' will be written beyond the memory allocated by strdup.  That could
corrupt something.


> +	}
> +
>  	if (c.backup_file && data_offset != INVALID_SECTORS) {
>  		pr_err("--backup-file and --data-offset are incompatible\n");
>  		exit(2);
> @@ -1407,6 +1426,12 @@ int main(int argc, char *argv[])
>  				rv = 1;
>  				break;
>  			}
> +			if (c.homecluster) {
> +				pr_err("--home-cluster argument is incompatible with --bitmap=%s.\n",
> +					s.bitmap_file);
> +				rv = 1;
> +				break;
> +			}

Better make sure s.bitmap_file is not NULL here too.


Thanks,
NeilBrown


>  		}
>  
>  		if (s.write_behind && !s.bitmap_file) {
> diff --git a/mdadm.h b/mdadm.h
> index 9d55801..f56d9d6 100644
> --- a/mdadm.h
> +++ b/mdadm.h
> @@ -345,6 +345,7 @@ enum special_options {
>  	Restore,
>  	Action,
>  	Nodes,
> +	ClusterName,
>  };
>  
>  enum prefix_standard {
> @@ -420,6 +421,7 @@ struct context {
>  	int	invalid_backup;
>  	char	*action;
>  	int	nodes;
> +	char	*homecluster;
>  };
>  
>  struct shape {
> @@ -1032,6 +1034,7 @@ struct supertype {
>  	int devcnt;
>  	int retry_soon;
>  	int nodes;
> +	char *cluster_name;
>  
>  	struct mdinfo *devs;
>  
> @@ -1308,6 +1311,7 @@ extern char *conf_get_mailaddr(void);
>  extern char *conf_get_mailfrom(void);
>  extern char *conf_get_program(void);
>  extern char *conf_get_homehost(int *require_homehostp);
> +extern char *conf_get_homecluster(void);
>  extern char *conf_line(FILE *file);
>  extern char *conf_word(FILE *file, int allow_key);
>  extern void print_quoted(char *str);
> @@ -1416,6 +1420,7 @@ extern char *stat2devnm(struct stat *st);
>  extern char *fd2devnm(int fd);
>  
>  extern int in_initrd(void);
> +extern int get_cluster_name(char **name);
>  
>  #define _ROUND_UP(val, base)	(((val) + (base) - 1) & ~(base - 1))
>  #define ROUND_UP(val, base)	_ROUND_UP(val, (typeof(val))(base))
> diff --git a/super1.c b/super1.c
> index 78d98a7..60f470b 100644
> --- a/super1.c
> +++ b/super1.c
> @@ -2145,6 +2145,9 @@ add_internal_bitmap1(struct supertype *st,
>  	bms->sync_size = __cpu_to_le64(size);
>  	bms->write_behind = __cpu_to_le32(write_behind);
>  	bms->nodes = __cpu_to_le32(st->nodes);
> +	if (st->cluster_name)
> +	    strncpy((char *)bms->cluster_name,
> +		    st->cluster_name, strlen(st->cluster_name));
>  
>  	*chunkp = chunk;
>  	return 1;
> diff --git a/util.c b/util.c
> index cc98d3b..ed9a745 100644
> --- a/util.c
> +++ b/util.c
> @@ -34,6 +34,8 @@
>  #include	<ctype.h>
>  #include	<dirent.h>
>  #include	<signal.h>
> +#include	<dlfcn.h>
> +#include	<corosync/cmap.h>
>  
>  /*
>   * following taken from linux/blkpg.h because they aren't
> @@ -1976,3 +1978,51 @@ void reopen_mddev(int mdfd)
>  	if (fd >= 0 && fd != mdfd)
>  		dup2(fd, mdfd);
>  }
> +
> +int get_cluster_name(char **cluster_name)
> +{
> +        void *lib_handle = NULL;
> +        int rv = -1;
> +
> +        cmap_handle_t handle;
> +        static int (*initialize)(cmap_handle_t *handle);
> +        static int (*get_string)(cmap_handle_t handle,
> +                        const char *string,
> +                        char **name);
> +        static int (*finalize)(cmap_handle_t handle);
> +
> +
> +        lib_handle = dlopen("libcmap.so.4", RTLD_NOW | RTLD_LOCAL);
> +        if (!lib_handle)
> +                return rv;
> +
> +        initialize = dlsym(lib_handle, "cmap_initialize");
> +        if (!initialize)
> +                goto out;
> +
> +        get_string = dlsym(lib_handle, "cmap_get_string");
> +        if (!get_string)
> +                goto out;
> +
> +        finalize = dlsym(lib_handle, "cmap_finalize");
> +        if (!finalize)
> +                goto out;
> +
> +        rv = initialize(&handle);
> +        if (rv != CS_OK)
> +                goto out;
> +
> +        rv = get_string(handle, "totem.cluster_name", cluster_name);
> +        if (rv != CS_OK) {
> +                free(*cluster_name);
> +                rv = -1;
> +                goto name_err;
> +        }
> +
> +        rv = 0;
> +name_err:
> +        finalize(handle);
> +out:
> +        dlclose(lib_handle);
> +        return rv;
> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH V3 04/11] Show all bitmaps while examining bitmap
From: NeilBrown @ 2015-05-25  4:23 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1432092043-24220-5-git-send-email-gqjiang@suse.com>

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

On Wed, 20 May 2015 11:20:36 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:

> This adds capability of exmining bitmaps corresponding to all
> nodes/slots on the device.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  bitmap.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 54 insertions(+), 18 deletions(-)
> 
> diff --git a/bitmap.c b/bitmap.c
> index 920033a..bccc67c 100644
> --- a/bitmap.c
> +++ b/bitmap.c
> @@ -260,7 +260,7 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
>  	int rv = 1;
>  	char buf[64];
>  	int swap;
> -	int fd;
> +	int fd, i;
>  	__u32 uuid32[4];
>  
>  	fd = bitmap_file_open(filename, &st);
> @@ -317,23 +317,59 @@ int ExamineBitmap(char *filename, int brief, struct supertype *st)
>  		       uuid32[2],
>  		       uuid32[3]);
>  
> -	printf("          Events : %llu\n", (unsigned long long)sb->events);
> -	printf("  Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> -	printf("           State : %s\n", bitmap_state(sb->state));
> -	printf("       Chunksize : %s\n", human_chunksize(sb->chunksize));
> -	printf("          Daemon : %ds flush period\n", sb->daemon_sleep);
> -	if (sb->write_behind)
> -		sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> -	else
> -		sprintf(buf, "Normal");
> -	printf("      Write Mode : %s\n", buf);
> -	printf("       Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> -					human_size(sb->sync_size * 512));
> -	if (brief)
> -		goto free_info;
> -	printf("          Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> -			info->total_bits, info->dirty_bits,
> -			100.0 * info->dirty_bits / (info->total_bits?:1));
> +	if (sb->nodes == 0) {
> +		printf("          Events : %llu\n", (unsigned long long)sb->events);
> +		printf("  Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> +		printf("           State : %s\n", bitmap_state(sb->state));
> +		printf("       Chunksize : %s\n", human_chunksize(sb->chunksize));
> +		printf("          Daemon : %ds flush period\n", sb->daemon_sleep);
> +		if (sb->write_behind)
> +			sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> +		else
> +			sprintf(buf, "Normal");
> +		printf("      Write Mode : %s\n", buf);
> +		printf("       Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> +						human_size(sb->sync_size * 512));
> +		if (brief)
> +			goto free_info;
> +		printf("          Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> +				info->total_bits, info->dirty_bits,
> +				100.0 * info->dirty_bits / (info->total_bits?:1));
> +	} else {
> +		printf("       Chunksize : %s\n", human_chunksize(sb->chunksize));
> +		printf("          Daemon : %ds flush period\n", sb->daemon_sleep);
> +		if (sb->write_behind)
> +			sprintf(buf, "Allow write behind, max %d", sb->write_behind);
> +		else
> +			sprintf(buf, "Normal");
> +		printf("      Write Mode : %s\n", buf);
> +		printf("       Sync Size : %llu%s\n", (unsigned long long)sb->sync_size/2,
> +						human_size(sb->sync_size * 512));

This is too much duplication.  Duplicate code leads to maintenance errors.

if (sb->nodes == 0)
    print Events/ Events cleared / state

print chunksuze, daemon flush period, write mode sync size

if (sb->nodes > 0) {
   print Events/ Events cleared etc for each bitmap 
}


> +		printf("   Cluster nodes : %d\n", sb->nodes);
> +		printf("    Cluster name : %s\n", sb->cluster_name);

sb->cluster_name is not nul terminated if it is exactly 64 bytes long.
You need "%64s" ... and you need to test that case.


> +		i = 0;
> +		do {

Please make this a regular
   for(i = 0; i < nodes; i++)
loop.  do/while should  only be used when some non-trivial computation is
needed before the condition can be tested.


> +			if (i) {
> +				free(info);
> +				info = bitmap_fd_read(fd, brief);
> +				sb = &info->sb;
> +			}
> +			if (sb->magic != BITMAP_MAGIC)
> +				pr_err("invalid bitmap magic 0x%x, the bitmap file appears to be corrupted\n", sb->magic);
> +
> +			printf("       Node Slot : %d\n", i);
> +			printf("          Events : %llu\n", (unsigned long long)sb->events);
> +			printf("  Events Cleared : %llu\n", (unsigned long long)sb->events_cleared);
> +			printf("           State : %s\n", bitmap_state(sb->state));
> +			if (brief)
> +				continue;
> +			printf("          Bitmap : %llu bits (chunks), %llu dirty (%2.1f%%)\n",
> +					info->total_bits, info->dirty_bits,
> +					100.0 * info->dirty_bits / (info->total_bits?:1));
> +
> +		} while (++i < (int)sb->nodes);
> +	}
> +
>  free_info:
>  	free(info);
>  	return rv;

Thanks,
NeilBrown


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply

* Re: [PATCH V3 05/11] Add a new clustered disk
From: NeilBrown @ 2015-05-25  4:35 UTC (permalink / raw)
  To: Guoqing Jiang; +Cc: linux-raid, rgoldwyn
In-Reply-To: <1432092043-24220-6-git-send-email-gqjiang@suse.com>

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

On Wed, 20 May 2015 11:20:37 +0800 Guoqing Jiang <gqjiang@suse.com> wrote:

> A clustered disk is added by the traditional --add sequence.
> However, other nodes need to acknowledge that they can "see"
> the device. This is done by --cluster-confirm:
> 
> --cluster-confirm SLOTNUM:/dev/whatever (if disk is found)
> or
> --cluster-confirm SLOTNUM:missing (if disk is not found)
> 
> The node initiating the --add, has the disk state tagged with
> MD_DISK_CLUSTER_ADD and the one confirming tag the disk with
> MD_DISK_CANDIDATE.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
> Signed-off-by: Guoqing Jiang <gqjiang@suse.com>
> ---
>  Manage.c   | 33 +++++++++++++++++++++++++++++----
>  ReadMe.c   |  1 +
>  md_p.h     |  7 +++++++
>  md_u.h     |  1 +
>  mdadm.8.in |  9 +++++++++
>  mdadm.c    |  4 ++++
>  mdadm.h    |  2 ++
>  util.c     | 10 ++++++++++
>  8 files changed, 63 insertions(+), 4 deletions(-)
> 
> diff --git a/Manage.c b/Manage.c
> index d3cfb55..4c3d451 100644
> --- a/Manage.c
> +++ b/Manage.c
> @@ -690,7 +690,8 @@ skip_re_add:
>  int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  	       struct supertype *tst, mdu_array_info_t *array,
>  	       int force, int verbose, char *devname,
> -	       char *update, unsigned long rdev, unsigned long long array_size)
> +	       char *update, unsigned long rdev, unsigned long long array_size,
> +	       int raid_slot)
>  {
>  	unsigned long long ldsize;
>  	struct supertype *dev_st = NULL;
> @@ -879,7 +880,10 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  	}
>  	disc.major = major(rdev);
>  	disc.minor = minor(rdev);
> -	disc.number =j;
> +	if (raid_slot < 0)
> +		disc.number = j;
> +	else
> +		disc.number = raid_slot;
>  	disc.state = 0;
>  	if (array->not_persistent==0) {
>  		int dfd;
> @@ -920,6 +924,14 @@ int Manage_add(int fd, int tfd, struct mddev_dev *dv,
>  			}
>  		free(used);
>  	}
> +
> +	if (array->state & (1 << MD_SB_CLUSTERED)) {
> +		if (dv->disposition == 'c')
> +			disc.state |= (1 << MD_DISK_CANDIDATE);
> +		else
> +			disc.state |= (1 << MD_DISK_CLUSTER_ADD);
> +	}
> +
>  	if (dv->writemostly == 1)
>  		disc.state |= (1 << MD_DISK_WRITEMOSTLY);
>  	if (tst->ss->external) {
> @@ -1239,6 +1251,7 @@ int Manage_subdevs(char *devname, int fd,
>  	 *        variant on 'A'
>  	 *  'F' - Another variant of 'A', where the device was faulty
>  	 *        so must be removed from the array first.
> +	 *  'c' - confirm the device as found (for clustered environments)
>  	 *
>  	 * For 'f' and 'r', the device can also be a kernel-internal
>  	 * name such as 'sdb'.
> @@ -1254,6 +1267,7 @@ int Manage_subdevs(char *devname, int fd,
>  	struct mdinfo info;
>  	int frozen = 0;
>  	int busy = 0;
> +	int raid_slot = -1;
>  
>  	if (ioctl(fd, GET_ARRAY_INFO, &array)) {
>  		pr_err("Cannot get array info for %s\n",
> @@ -1282,6 +1296,11 @@ int Manage_subdevs(char *devname, int fd,
>  		int rv;
>  		int mj,mn;
>  
> +		raid_slot = -1;
> +		if (dv->disposition == 'c')
> +			parse_cluster_confirm_arg(dv->devname, &dv->devname,
> +					&raid_slot);
> +

This function returns -1 if it couldn't successfully parse dv->devname, but
you aren't catching that error and reporting it.


NeilBrown


>  
> +int parse_cluster_confirm_arg(char *input, char **devname, int *slot)
> +{
> +	char *dev;
> +	*slot = strtoul(input, &dev, 10);
> +	if (dev == input || dev[0] != ':')
> +		return -1;
> +	*devname = dev+1;
> +	return 0;
> +}
> +

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox