Re: [PATCH 2/3] common/rc: specify required device size

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





On 3/7/24 20:46, Zorro Lang wrote:
On Thu, Mar 07, 2024 at 06:20:23PM +0530, Anand Jain wrote:
The current _notrun call states that the scratch device is too small but
does not specify the required size. Simply update the _notrun messages.

Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
---

OK, I think that makes sense to me.

  common/rc | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/rc b/common/rc
index 50dde313b851..5680995b2366 100644
--- a/common/rc
+++ b/common/rc
@@ -1834,7 +1834,8 @@ _require_scratch_size()
_require_scratch
  	local devsize=`_get_device_size $SCRATCH_DEV`
-	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
+	[ $devsize -lt $1 ] && \
+_notrun "scratch device $SCRATCH_DEV should be minimum $1, currently $devsize"


  }
# require a scratch dev of a minimum size (in kb) and should not be checked
@@ -1845,7 +1846,8 @@ _require_scratch_size_nocheck()
_require_scratch_nocheck
  	local devsize=`_get_device_size $SCRATCH_DEV`
-	[ $devsize -lt $1 ] && _notrun "scratch dev too small"
+	[ $devsize -lt $1 ] && \
+_notrun "require scratch device $SCRATCH_DEV atleast $1, currently $devsize"

"at least"

Why don't output same message with above ?


If the error messages are unique, it is easier to search for the line in
the source code when it is required.

And why not have indentation at here?

Is it ok to overshoot 80 char? we move the line left as much required
so that line is possible fit within  80char.
I'm ok to follow any better ways if any.



I think the message is long enough, so the $SCRATCH_DEV maybe not necessary,
how about

	[ $devsize -lt $1 ] && \
		_notrun "scratch device too small, $devsize < $1"


 This is reasonable, I will use it.

(same above)

With this change:
Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx>

As I've given review points to patch 3/3, so I suppose you can change this
patch with the 3rd one together :)

 Yep.

Thanks, Anand


Thanks,
Zorro


  }
# Require scratch fs which supports >16T of filesystem size.
--
2.39.3







[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux