Patch for fsadm bz 64146

Patch for fsadm bz 64146

[Zdenek Kabelac]

This patch address problem from  BZ 64146.

Instead of leaving lvresize to fail when running on the mounted fs (user
expects online resize to happen and it's currently possible only with -n
option) - this patch changes 'fsadm check' behavior so it returns success for
mounted  filesystem. This allows lvresize to processed further and run the
online resize even without passing -n option.

Anyone sees this modified behavior of fsadm script as a problem ?
(change success for mounted fs).

Zdenek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fsadm.patch
Type: text/x-patch
Size: 1315 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20101029/5612a977/attachment.bin>

Patch for fsadm bz 64146

[Zdenek Kabelac]

Dne 29.10.2010 21:54, Zdenek Kabelac napsal(a):
> This patch address problem from  BZ 64146.

Correct number is 641461

https://bugzilla.redhat.com/show_bug.cgi?id=641461

> 
> Instead of leaving lvresize to fail when running on the mounted fs (user
> expects online resize to happen and it's currently possible only with -n
> option) - this patch changes 'fsadm check' behavior so it returns success for
> mounted  filesystem. This allows lvresize to processed further and run the
> online resize even without passing -n option.
> 
> Anyone sees this modified behavior of fsadm script as a problem ?
> (change success for mounted fs).
> 
> Zdenek
> 
> 
> 
> --
> lvm-devel mailing list
> lvm-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/lvm-devel

Patch for fsadm bz 64146

[Alasdair G Kergon]

On Fri, Oct 29, 2010 at 09:54:16PM +0200, Zdenek Kabelac wrote:
> Anyone sees this modified behavior of fsadm script as a problem ?
> (change success for mounted fs).
 
This is not my first choice of solution, but in the circumstances
I've no objections to this patch going in.

Ack.

Alasdair

Patch for fsadm bz 64146

[Mike Snitzer]

On Fri, Oct 29 2010 at  3:54pm -0400,
Zdenek Kabelac <zkabelac@redhat.com> wrote:

> This patch address problem from  BZ 64146.
> 
> Instead of leaving lvresize to fail when running on the mounted fs (user
> expects online resize to happen and it's currently possible only with -n
> option) - this patch changes 'fsadm check' behavior so it returns success for
> mounted  filesystem. This allows lvresize to processed further and run the
> online resize even without passing -n option.
> 
> Anyone sees this modified behavior of fsadm script as a problem ?
> (change success for mounted fs).
> 
> Zdenek

> Index: scripts/fsadm.sh
> ===================================================================
> RCS file: /cvs/lvm2/LVM2/scripts/fsadm.sh,v
> retrieving revision 1.20
> diff -u -p -r1.20 fsadm.sh
> --- scripts/fsadm.sh	8 Oct 2010 15:02:05 -0000	1.20
> +++ scripts/fsadm.sh	29 Oct 2010 19:43:33 -0000
> @@ -365,7 +365,10 @@ resize() {
>  ###################
>  check() {
>  	detect_fs "$1"
> -	detect_mounted && error "Cannot fsck device \"$VOLUME\", filesystem is mounted on $MOUNTED"
> +	if detect_mounted ; then
> +		verbose "Skipping fsck device \"$VOLUME\" as filesystem is mounted on $MOUNTED";

I think the verbose message should be:
"Skipping fsck for device \"$VOLUME\" as filesystem is mounted on $MOUNTED"

Mike

Patch for fsadm bz 64146

[Mike Snitzer]

On Fri, Oct 29 2010 at  4:44pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Fri, Oct 29 2010 at  3:54pm -0400,
> Zdenek Kabelac <zkabelac@redhat.com> wrote:
> 
> > -	detect_mounted && error "Cannot fsck device \"$VOLUME\", filesystem is mounted on $MOUNTED"
> > +	if detect_mounted ; then
> > +		verbose "Skipping fsck device \"$VOLUME\" as filesystem is mounted on $MOUNTED";
> 
> I think the verbose message should be:
> "Skipping fsck for device \"$VOLUME\" as filesystem is mounted on $MOUNTED"

"for" could be replaced with "on" or "of" too. ;)

Mike

Patch for fsadm bz 64146

[Alasdair G Kergon]

> > "Skipping fsck for device \"$VOLUME\" as filesystem is mounted on $MOUNTED"
> "for" could be replaced with "on" or "of" too. ;)
 
What's fsck?

Alasdair

Patch for fsadm bz 64146

[Zdenek Kabelac]

Dne 29.10.2010 23:26, Alasdair G Kergon napsal(a):
>>> "Skipping fsck for device \"$VOLUME\" as filesystem is mounted on $MOUNTED"
>> "for" could be replaced with "on" or "of" too. ;)
>  
> What's fsck?
> 

I think the tool should be used by system admins - so they should not need the
long version 'filesystem check' - but if it's preferable to avoid shortcut
I'll switch ?

Do we consider now this as a final solution - or do you rather prefer 'non
zero' return code for this case (check of mounted fs). This would require
exec_cmd modification for status passing we've discussed.

I'd rather avoid switching between success/fail return code from the fsadm
command between releases.

Zdenek