All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zdenek Kabelac <zkabelac@redhat.com>
To: lvm-devel@redhat.com
Subject: [PATCH v3 01/18] fsadm: Add new commands create, list, add and remove
Date: Tue, 04 Oct 2011 19:09:57 +0200	[thread overview]
Message-ID: <4E8B3DE5.2080803@redhat.com> (raw)
In-Reply-To: <alpine.LFD.2.00.1110041336440.4206@dhcp-27-109.brq.redhat.com>

Dne 4.10.2011 14:13, Lukas Czerner napsal(a):
> On Tue, 4 Oct 2011, Zdenek Kabelac wrote:
>
>> Dne 27.9.2011 15:42, Lukas Czerner napsal(a):
>>> This commit adds new functionality in the form of new commands. Namely
>>> it is create, list, add and remove. This commit also changes the way how
>>> are commands recognised an executed. The new approach is more suitable
>>> for bigger number of commands.
>>>
>>> Resize command is also significantly reworked. Unlike in the old
>>> approach fsadm will always attempt to resize the logical volume as well,
>>> since this is what it is expected. Leave the --lvresize option for
>>> backwards compatibility, but remove it from the help. If no file system
>>> resides on the logical volume, only the volume will be resized.
>>>
>>> * Create command provides the functionality of creating a new logical
>>>     volumes including defined file system.
>>> * List command provides the functionality of listing useful information
>>>     about the logical volumes, file systems and devices.
>>> * Add command allows to add devices into volume groups (pool).
>>> * Remove command allows to remove the volumes or volume groups from the
>>>     system.
>>>
>>> Signed-off-by: Lukas Czerner<lczerner@redhat.com>
>>
>>> @@ -96,7 +109,6 @@ tool_usage() {
>>>    	echo "    -e | --ext-offline  unmount filesystem before ext2/ext3/ext4
>>> resize"
>>>    	echo "    -f | --force        Bypass sanity checks"
>>>    	echo "    -n | --dry-run      Print commands without running them"
>>> -	echo "    -l | --lvresize     Resize given device (if it is LVM
>>> device)"
>>>    	echo "    -y | --yes          Answer \"yes\" at any prompts"
>>>    	echo
>>>    	echo "  new_size - Absolute number of filesystem blocks to be in the
>>> filesystem,"
>>
>>
>> Again - no way to remove already supported option.
>
> This is only removed from the tool_usage(), as you can see. I am not
> removing it from the code, but only from the documentation since it is
> not needed in the new code.
>

Ok this made it clear that such functional change than needs a separate patch, 
which will demonstrate how it's going to be used and how it is matching 
current behavior.



>>> -	if [ "$DO_LVRESIZE" -eq 2 ]; then
>>> -		# start LVRESIZE with the filesystem modification flag
>>> -		# and allow recursive call of fsadm
>>> -		_FSADM_YES=$YES
>>> -		export _FSADM_YES
>>> -		unset FSADM_RUNNING
>>> -		test -n "$LVM_BINARY"&&   PATH=$_SAVEPATH
>>> -		dry exec "$LVM" lvresize $VERB $FORCE -r -L${NEWSIZE}b
>>> "$VOLUME_ORIG"
>>> -	fi
>>> -
>>
>> Again - please do not remove things you do not understand.
>
> Oh, but I do understand. Please read the new code, before trying to
> insult me. But maybe it would be better to separate resize command
> changes into separate patch.
>
>>
>> And also thing like this needs to be in a separate patch - not bundled in
>> hundreds patch lines.


It's nothing about insulting - this is very tricky change - so unless this is 
extracted to separate patch to be properly reviewed for all combination and 
whether all cases keep all VG locks properly.
(Thus patch needs to explain how it will resize  filesystem and LV when user 
calls fsadm resize and this operation could not be broken with another 
parallel lvm operation in both directions - upsize/downsize while current 
syntax is still usable)


>> Well user is always the master - if he is brave to go without it, let him do
>> what he wants (i.e. destroy his fs).
>
> You're missing the point, resize2fs requires that the file system is
> checked with e2fsck -f prior the resize, if on of the conditions happen
>
> 1. file system was not cleanly umounted
> 2. file system contain errors, or is invalid
> 3. it was not checked since the last time it was mounted
>
> It is just to make sure that the file system is consistent and we will
> not make everything worse by messing with metadata further.
>

If the user wants to risk  resize2fs without fsck - it's his decision (and 
current -f with fsadm allows it) - and since we are not like gnome3 - we allow 
users to kill their systems in number of ways already, so no reason to 
eliminate this one.

i.e. mkfs, mount, umount, resize LV, resize2fs -f   works without fsck and 
makes errorless result - so we support.

Zdenek




  reply	other threads:[~2011-10-04 17:09 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-27 13:42 [RFC][PATCH v3 00/16] fsadm update Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 01/18] fsadm: Add new commands create, list, add and remove Lukas Czerner
2011-10-04  9:41   ` Zdenek Kabelac
2011-10-04 12:13     ` Lukas Czerner
2011-10-04 17:09       ` Zdenek Kabelac [this message]
2011-10-05  8:02         ` Lukas Czerner
2011-10-05  9:06           ` Zdenek Kabelac
2011-10-05  9:46             ` Lukas Czerner
2011-10-05 10:27               ` Alasdair G Kergon
2011-10-05 11:21                 ` Lukas Czerner
2011-10-05 11:26                   ` Lukas Czerner
2011-10-05 11:28                   ` Ric Wheeler
2011-10-05 11:49                   ` Alasdair G Kergon
2011-10-05 12:15                     ` Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 02/18] fsadm: Make all internal math in kilobytes Lukas Czerner
2011-09-27 15:41   ` Zdenek Kabelac
2011-10-03 16:13     ` Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 03/18] fsadm: Add simple configuration file Lukas Czerner
2011-09-27 15:39   ` Zdenek Kabelac
2011-10-03 16:44     ` Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 04/18] fsadm: Use DEFAULT_DEVICE_POOL when creating volume group Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 05/18] fsadm: Add LVOL_PREFIX configuration option Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 06/18] fsadm: Fsck extN before resize only if it is not mounted Lukas Czerner
2011-10-04  8:09   ` Zdenek Kabelac
2011-09-27 13:42 ` [PATCH v3 07/18] fsadm: Only use readlink if link is provided Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 08/18] fsadm: Remove unnecessary modification of PATH variable Lukas Czerner
2011-10-04  8:12   ` Zdenek Kabelac
2011-10-04  8:17     ` Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 09/18] fsadm: Allow to specify lv in vg/lv format Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 10/18] fsadm: Umount ext2 file system prior resize Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 11/18] fsadm: Add help for new commands and update man page Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 12/18] lvresize: Specify --resize-fs-only when going to use fsadm resize Lukas Czerner
2011-09-27 15:44   ` Zdenek Kabelac
2011-10-03 16:20     ` Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 13/18] fsadm: remove -y (YES) option Lukas Czerner
2011-09-27 15:38   ` Zdenek Kabelac
2011-10-03 16:39     ` Lukas Czerner
2011-10-03 18:18       ` Zdenek Kabelac
2011-10-04  6:29         ` Lukas Czerner
2011-10-04  8:07           ` Zdenek Kabelac
2011-09-27 13:42 ` [PATCH v3 14/18] test: add helper to compute aligned lv size Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 15/18] test: Add test for fsadm add command Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 16/18] test: Add test for fsadm create command Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 17/18] test: Add test for fsadm resize command Lukas Czerner
2011-09-27 13:42 ` [PATCH v3 18/18] test: Add test for fsadm remove command Lukas Czerner
2011-09-27 13:50 ` [RFC][PATCH v3 00/16] fsadm update Lukas Czerner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4E8B3DE5.2080803@redhat.com \
    --to=zkabelac@redhat.com \
    --cc=lvm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.