[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [linux-lvm] [PATCH 11/35] fsadm: Add "remove" command



On Thu, 22 Sep 2011, Zdenek Kabelac wrote:

> Dne 21.9.2011 18:45, Lukas Czerner napsal(a):
> > Remove command allows to remove unused devices from the pool (volume
> > group).
> > 
> > Signed-off-by: Lukas Czerner <lczerner redhat com>
> > ---
> >  scripts/fsadm.sh |   82 ++++++++++++++++++++++++++++++++++++++++-------------
> >  1 files changed, 62 insertions(+), 20 deletions(-)
> > 
> > diff --git a/scripts/fsadm.sh b/scripts/fsadm.sh
> > index 6617de0..4a4f625 100755
> > --- a/scripts/fsadm.sh
> > +++ b/scripts/fsadm.sh
> > @@ -823,11 +823,12 @@ list_filesystems() {
> >  	IFS=$NL
> >  	local c=0
> >  	for line in $(LANG=C $LVM lvs -o lv_path,lv_size,segtype --noheadings --separator ' ' --nosuffix --units k 2> /dev/null); do
> > -		c=$((c+1))
> >  		line=$(echo $line | sed -e 's/^ *\//\//')
> >  		volume=$(echo $line | cut -d' ' -f1)
> > -		volumes[$c]=$volume
> > -		segtype[$c]=$(echo $line | cut -d' ' -f3)
> > +		[ "$volume" == "$last_volume" ] && continue
> > +		c=$((c+1))
> > +		local volumes[$c]=$volume
> > +		local segtype[$c]=$(echo $line | cut -d' ' -f3)
> >  		detect_fs $volume
> >  		detect_mounted
> >  		detect_fs_size
> 
> Could you please update/cleanup the patch set - so it doesn't rework same code
> multiple times over and over ?
> (It's a waste of time to review new code, which gets replaced several times
> during the whole patch set)

Unfortunately that is what I have mentioned in the patch 0. The main
problem is that the fsadm is one file script and while adding other
functionality I was building helpers and yes also changing other parts
of the code.

It was not possible for me make one feature, be done with it and move to
the another feature. Some of them actually supports each other and not
having to rebase with every change made my work significantly easier. So
at this point I am not going to rework the patches, since it would take
more time that actually write that code :).

But, since it is a single file script, and I changed the whole thing
significantly (there is not much left from the original code) I can
create one big patch covering all the new features, so it will be easier
for you to review it. Will that be acceptable for you ?

Thanks!
-Lukas

> 
> Thanks
> 
> Zdenek


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]