On Wed, 13 Feb 2013 13:56:45 -0500 (EST) Joe Lawrence
<Joe.Lawrence@xxxxxxxxxxx> wrote:
> On Wed, 13 Feb 2013, Sebastian Riemer wrote:
>
> > On 13.02.2013 03:38, NeilBrown wrote:
> > > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > > index 8b557d2..292cc2f 100644
> > > --- a/drivers/md/md.c
> > > +++ b/drivers/md/md.c
> > > @@ -6529,7 +6529,17 @@ static int md_ioctl(struct block_device *bdev, fmode_t mode,
> > > mddev->ro = 0;
> > > sysfs_notify_dirent_safe(mddev->sysfs_state);
> > > set_bit(MD_RECOVERY_NEEDED, &mddev->recovery);
> > > - md_wakeup_thread(mddev->thread);
> > > + /* mddev_unlock will wake thread */
> > > + /* If a device failed while we were read-only, we
> > > + * need to make sure the metadata is updated now.
> > > + */
> > > + if (test_bit(MD_CHANGE_DEVS, &mddev->flags)) {
> > > + mddev_unlock(mddev);
> > > + wait_event(mddev->sb_wait,
> > > + !test_bit(MD_CHANGE_DEVS, &mddev->flags) &&
> > > + !test_bit(MD_CHANGE_PENDING, &mddev->flags));
> > > + mddev_lock(mddev);
> > > + }
> > > } else {
> > > err = -EROFS;
> > > goto abort_unlock;
> > >
> >
> > Thanks, Neil!
> >
> > I can confirm the issue on 3.4.y and that your patch fixes it reliably.
> >
> > Acked-by: Sebastian Riemer <sebastian.riemer@xxxxxxxxxxxxxxxx>
>
> Running with this patch against 3.8.0rc7 with the following results:
>
> auto-read-only
> * mdadm --fail - success
> * mdadm --remove - success
>
> read-only
> * mdadm --fail - success
> * mdadm --remove - EROFS
That all looks good. I could probably argue that removing a device from a
read-only array should be permitted but I don't know that it is really
important.
>
> Quick question for Neil -- should the sysfs MD component device state
> file interface perform the same transition from auto-read-only to
> read-write, or is that route intended for more granular changes?
>
Good question.
I think they should definitely transition from auto-read-only to read-write.
Whether they should be denied for read-only devices I'm less certain of.
I'll look into that.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
