On Wed, 12 Jan 2011 15:57:45 +0000 "Kwolek, Adam" <adam.kwolek@xxxxxxxxx> wrote: > > -----Original Message----- > > From: NeilBrown [mailto:neilb@xxxxxxx] > > Sent: Wednesday, January 12, 2011 6:43 AM > > To: Kwolek, Adam > > Cc: linux-raid@xxxxxxxxxxxxxxx; Williams, Dan J; Ciechanowski, Ed; > > Neubauer, Wojciech > > Subject: Re: [PATCH 2/9] FIX: Cannot add spare to monitored container > > > > On Tue, 11 Jan 2011 15:04:03 +0100 Adam Kwolek <adam.kwolek@xxxxxxxxx> > > wrote: > > > > > This fix rolls backs some changes introduced by patch: > > > "Add spares to raid0 in mdadm" > > > > > > Signed-off-by: Adam Kwolek <adam.kwolek@xxxxxxxxx> > > > > It would help a lot if you explained *why* you were rolling back these > > changes. > > > > Not applied. > > > > NeilBrown > > > I cannot add spare to i.e. raid5 array. In my unit tests, I'm creating raid5 array and then I'm adding spares for expansion (mdadm --add). > If I've create container with more disks than create array that is using fewer disks, I can test expansion. > Spares can be added to raid0 with no monitor usage without problems. > I've started to observe this problem when patch for adding spares to raid0 was applied. > I've identify code change that makes problem. This causes this code rollback. > > Problem that is visible is that. Monitor cannot write major:minor to new_dev in sysfs /without code roll back/. > I didn't dig this problem deeply. This patch is rather reminder to signalize problem /and not forget about it/. > Fair enough. Having a patch which works around a problem and reminds you to fix it later is a good idea. However you should make sure that the notes at the top of the patch make it clear that is the purpose of the patch - especially when you post it to me. It is just SO important to include completed details in the notes at the top of the patch!!!! NeilBrown > BR > Adam > > > > > > --- > > > > > > Manage.c | 8 ++++---- > > > 1 files changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/Manage.c b/Manage.c > > > index 4e6afb5..fa1b13f 100644 > > > --- a/Manage.c > > > +++ b/Manage.c > > > @@ -800,16 +800,16 @@ int Manage_subdevs(char *devname, int fd, > > > if (dv->writemostly == 1) > > > disc.state |= 1 << MD_DISK_WRITEMOSTLY; > > > dfd = dev_open(dv->devname, O_RDWR | > > O_EXCL|O_DIRECT); > > > - if (tst->ss->external && > > > - mdmon_running(tst->container_dev)) > > > - tst->update_tail = &tst->updates; > > > if (tst->ss->add_to_super(tst, &disc, dfd, > > > dv->devname)) { > > > close(dfd); > > > return 1; > > > } > > > /* write_init_super will close 'dfd' */ > > > - if (tst->ss->write_init_super(tst)) > > > + if (tst->ss->external) > > > + /* mdmon will write the metadata */ > > > + close(dfd); > > > + else if (tst->ss->write_init_super(tst)) > > > return 1; > > > } else if (dv->re_add) { > > > /* this had better be raid1. > > -- > 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 -- 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
