From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sebastian Riemer Subject: Re: [PATCH v2] md: also hot remove disk from pers when hot removing Date: Mon, 19 Nov 2012 17:08:13 +0100 Message-ID: <50AA596D.4080003@profitbricks.com> References: <1353081849-10516-1-git-send-email-sebastian.riemer@profitbricks.com> <50AA0985.6060601@profitbricks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <50AA0985.6060601@profitbricks.com> Sender: linux-raid-owner@vger.kernel.org To: Dan Williams Cc: linux-raid List-Id: linux-raid.ids On 19.11.2012 11:27, Sebastian Riemer wrote: > On 17.11.2012 01:07, Dan Williams wrote: >> Hmm, how are you getting ->raid_disk set to -1 without the personality >> being notified? That seems to be the source of the bug. Otherwise it >> would seem the state_store("remove") path is also susceptible. >> >> -- >> Dan > Thanks for your feedback! > > Perhaps it's only something for my use-case of read-only > raid1-replicated volumes where I never have any spares. I don't set > ->raid_disk to -1 when hot adding them. I have to put them directly to > their stored slot because I assemble read-only on read-only rdevs. So > the syncer never runs. Those read-only rdevs are coming from remote > storage. Imagine HA CD-ROM images. > > My thought was that it would be cleaner behavior if the cleanup of an > rdev in the personality already happens when hot removing the disk. > > You're right, the state_store("remove") path would need this change, too. > > With the "slot_store" path I could trigger a panic when I hot added the > disk as spare, marked it insync and tried to give it the slot it belongs > to. I'll test this with the latest vanilla kernel and try to reproduce > this on rw volumes which I set to read-only. If I can also crash it that > way, I've got a proof that there is a bug. The kernel blocks most ioctls on a read-only mddev. So I guess I've got another custom patch. IMHO it would be still cleaner to also hot remove the disk from the personality when hot removing an rdev. > If the disk isn't cleaned up in the pers, then hot_add_disk doesn't > return any error for raid1. Instead it crashes when referencing the old > pointer when calling "print_conf" in raid1.c.