* [PATCH] FIX: Do not count as backup devices, spare disks used for reshape @ 2011-03-18 10:55 Adam Kwolek 2011-03-20 4:41 ` NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: Adam Kwolek @ 2011-03-18 10:55 UTC (permalink / raw) To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer Problem: Reshape is run without specified backup file when all spares are used for expansion. When spare disks are used for reshape, they should not be counted as backup devices. Md still thinks about them as about spares until reshape will not be started. mdadm should have all it in mind. Signed-off-by: Adam Kwolek <adam.kwolek@intel.com> --- Grow.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/Grow.c b/Grow.c index b639585..17c22fc 100644 --- a/Grow.c +++ b/Grow.c @@ -1660,6 +1660,7 @@ static int reshape_array(char *container, int fd, char *devname, unsigned long long array_size; int done; struct mdinfo *sra = NULL; + int used_spares = 0; /* when reshaping a RAID0, the component_size might be zero. * So try to fix that up. @@ -1793,6 +1794,7 @@ static int reshape_array(char *container, int fd, char *devname, * be part of the array. */ add_disk(fd, st, info2, d); + used_spares++; } } sysfs_free(info2); @@ -1956,7 +1958,7 @@ started: Name ": %s: Cannot grow - need backup-file\n", devname); goto release; - } else if (sra->array.spare_disks == 0) { + } else if (sra->array.spare_disks - used_spares == 0) { fprintf(stderr, Name ": %s: Cannot grow - need a spare or " "backup-file to backup critical section\n", devname); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] FIX: Do not count as backup devices, spare disks used for reshape 2011-03-18 10:55 [PATCH] FIX: Do not count as backup devices, spare disks used for reshape Adam Kwolek @ 2011-03-20 4:41 ` NeilBrown 2011-03-23 7:49 ` Kwolek, Adam 0 siblings, 1 reply; 4+ messages in thread From: NeilBrown @ 2011-03-20 4:41 UTC (permalink / raw) To: Adam Kwolek Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer On Fri, 18 Mar 2011 11:55:12 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote: > Problem: > Reshape is run without specified backup file when all spares are used for expansion. > > When spare disks are used for reshape, they should not be counted as backup devices. > Md still thinks about them as about spares until reshape will not be started. > mdadm should have all it in mind. > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com> > --- > > Grow.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/Grow.c b/Grow.c > index b639585..17c22fc 100644 > --- a/Grow.c > +++ b/Grow.c > @@ -1660,6 +1660,7 @@ static int reshape_array(char *container, int fd, char *devname, > unsigned long long array_size; > int done; > struct mdinfo *sra = NULL; > + int used_spares = 0; > > /* when reshaping a RAID0, the component_size might be zero. > * So try to fix that up. > @@ -1793,6 +1794,7 @@ static int reshape_array(char *container, int fd, char *devname, > * be part of the array. > */ > add_disk(fd, st, info2, d); > + used_spares++; > } > } > sysfs_free(info2); > @@ -1956,7 +1958,7 @@ started: > Name ": %s: Cannot grow - need backup-file\n", > devname); > goto release; > - } else if (sra->array.spare_disks == 0) { > + } else if (sra->array.spare_disks - used_spares == 0) { > fprintf(stderr, Name ": %s: Cannot grow - need a spare or " > "backup-file to backup critical section\n", > devname); This change doesn't make sense to me. The only time when we use a spare device as for storing the backup is (or at least should be) when increasing the number of devices in the array. In that case we only need a backup at the very beginning. So we use the end of a spare for the backup that happens at the every beginning and it all works happily. Why do you now want to count a spare used for backup as a spare to use for growth? NeilBrown ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] FIX: Do not count as backup devices, spare disks used for reshape 2011-03-20 4:41 ` NeilBrown @ 2011-03-23 7:49 ` Kwolek, Adam 2011-03-23 23:24 ` NeilBrown 0 siblings, 1 reply; 4+ messages in thread From: Kwolek, Adam @ 2011-03-23 7:49 UTC (permalink / raw) To: NeilBrown Cc: linux-raid@vger.kernel.org, Williams, Dan J, Ciechanowski, Ed, Neubauer, Wojciech > -----Original Message----- > From: NeilBrown [mailto:neilb@suse.de] > Sent: Sunday, March 20, 2011 5:41 AM > To: Kwolek, Adam > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed; > Neubauer, Wojciech > Subject: Re: [PATCH] FIX: Do not count as backup devices, spare disks > used for reshape > > On Fri, 18 Mar 2011 11:55:12 +0100 Adam Kwolek <adam.kwolek@intel.com> > wrote: > > > Problem: > > Reshape is run without specified backup file when all spares are used > for expansion. > > > > When spare disks are used for reshape, they should not be counted as > backup devices. > > Md still thinks about them as about spares until reshape will not be > started. > > mdadm should have all it in mind. > > > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com> > > --- > > > > Grow.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > diff --git a/Grow.c b/Grow.c > > index b639585..17c22fc 100644 > > --- a/Grow.c > > +++ b/Grow.c > > @@ -1660,6 +1660,7 @@ static int reshape_array(char *container, int > fd, char *devname, > > unsigned long long array_size; > > int done; > > struct mdinfo *sra = NULL; > > + int used_spares = 0; > > > > /* when reshaping a RAID0, the component_size might be zero. > > * So try to fix that up. > > @@ -1793,6 +1794,7 @@ static int reshape_array(char *container, int > fd, char *devname, > > * be part of the array. > > */ > > add_disk(fd, st, info2, d); > > + used_spares++; > > } > > } > > sysfs_free(info2); > > @@ -1956,7 +1958,7 @@ started: > > Name ": %s: Cannot grow - need backup-file\n", > > devname); > > goto release; > > - } else if (sra->array.spare_disks == 0) { > > + } else if (sra->array.spare_disks - used_spares == 0) { > > fprintf(stderr, Name ": %s: Cannot grow - need a spare > or " > > "backup-file to backup critical section\n", > > devname); > > > This change doesn't make sense to me. > > The only time when we use a spare device as for storing the backup is > (or at > least should be) when increasing the number of devices in the array. > In that case we only need a backup at the very beginning. > So we use the end of a spare for the backup that happens at the every > beginning and it all works happily. > > Why do you now want to count a spare used for backup as a spare to use > for > growth? > Because difference in user interface in start and restart reshape /IMHO/. For reshape start spare disk can be used for grow can be used as backup device also, so backup file doesn't have to be specified. For reshape restart spare is used by array already and it is not indicated as spare device, so user have to specified backup file (it is also possible that backup file will be not used during reshape restart). The different behavior /requirement for backup-file command line parameter/ for reshape start and restart can be not obvious for user. Possible we should not require backup file after critical section for OLCE. If you think this is not a problem /or it is not important improvement only/ we can forget about this patch BR Adam > NeilBrown ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] FIX: Do not count as backup devices, spare disks used for reshape 2011-03-23 7:49 ` Kwolek, Adam @ 2011-03-23 23:24 ` NeilBrown 0 siblings, 0 replies; 4+ messages in thread From: NeilBrown @ 2011-03-23 23:24 UTC (permalink / raw) To: Kwolek, Adam Cc: linux-raid@vger.kernel.org, Williams, Dan J, Ciechanowski, Ed, Neubauer, Wojciech On Wed, 23 Mar 2011 07:49:32 +0000 "Kwolek, Adam" <adam.kwolek@intel.com> wrote: > > > > -----Original Message----- > > From: NeilBrown [mailto:neilb@suse.de] > > Sent: Sunday, March 20, 2011 5:41 AM > > To: Kwolek, Adam > > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed; > > Neubauer, Wojciech > > Subject: Re: [PATCH] FIX: Do not count as backup devices, spare disks > > used for reshape > > > > On Fri, 18 Mar 2011 11:55:12 +0100 Adam Kwolek <adam.kwolek@intel.com> > > wrote: > > > > > Problem: > > > Reshape is run without specified backup file when all spares are used > > for expansion. > > > > > > When spare disks are used for reshape, they should not be counted as > > backup devices. > > > Md still thinks about them as about spares until reshape will not be > > started. > > > mdadm should have all it in mind. > > > > > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com> > > > --- > > > > > > Grow.c | 4 +++- > > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > > > diff --git a/Grow.c b/Grow.c > > > index b639585..17c22fc 100644 > > > --- a/Grow.c > > > +++ b/Grow.c > > > @@ -1660,6 +1660,7 @@ static int reshape_array(char *container, int > > fd, char *devname, > > > unsigned long long array_size; > > > int done; > > > struct mdinfo *sra = NULL; > > > + int used_spares = 0; > > > > > > /* when reshaping a RAID0, the component_size might be zero. > > > * So try to fix that up. > > > @@ -1793,6 +1794,7 @@ static int reshape_array(char *container, int > > fd, char *devname, > > > * be part of the array. > > > */ > > > add_disk(fd, st, info2, d); > > > + used_spares++; > > > } > > > } > > > sysfs_free(info2); > > > @@ -1956,7 +1958,7 @@ started: > > > Name ": %s: Cannot grow - need backup-file\n", > > > devname); > > > goto release; > > > - } else if (sra->array.spare_disks == 0) { > > > + } else if (sra->array.spare_disks - used_spares == 0) { > > > fprintf(stderr, Name ": %s: Cannot grow - need a spare > > or " > > > "backup-file to backup critical section\n", > > > devname); > > > > > > This change doesn't make sense to me. > > > > The only time when we use a spare device as for storing the backup is > > (or at > > least should be) when increasing the number of devices in the array. > > In that case we only need a backup at the very beginning. > > So we use the end of a spare for the backup that happens at the every > > beginning and it all works happily. > > > > Why do you now want to count a spare used for backup as a spare to use > > for > > growth? > > > > Because difference in user interface in start and restart reshape /IMHO/. > For reshape start spare disk can be used for grow can be used as backup device also, so backup file doesn't have to be specified. > For reshape restart spare is used by array already and it is not indicated as spare device, so user have to specified backup file > (it is also possible that backup file will be not used during reshape restart). If a spare can be used for backup, it will only be used at the very beginning, for the first few stripes. After that it won't be used any more. So when restarting a reshape that used a spare device for the backup, the backup is only of interest if we crashed during those first few stripes, and mdadm will simply read the backup, write out the stripes, and then continue with no further need for a backup. > > The different behavior /requirement for backup-file command line parameter/ for reshape start and restart can be not obvious for user. I agree that the situation might be confusing - but I don't think your change would help. > Possible we should not require backup file after critical section for OLCE. > IMSM OLCE is a bit awkward. As there might be multiple arrays in the container, you need a backup file for the start of each array. So when restarting the reshape of the first array in the container you don't need a backup file for that array, but you will for the next array (if there is one). > If you think this is not a problem /or it is not important improvement only/ we can forget about this patch There may be issue here that need to be addressed, but I don't think this patch addresses them, so I will drop it. Thanks, NeilBrown ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-23 23:24 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-18 10:55 [PATCH] FIX: Do not count as backup devices, spare disks used for reshape Adam Kwolek 2011-03-20 4:41 ` NeilBrown 2011-03-23 7:49 ` Kwolek, Adam 2011-03-23 23:24 ` NeilBrown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).