On Tue 20 Apr 2021 at 01:08, Boris Burkov <bo...@bur.io> wrote:

On Mon, Apr 19, 2021 at 09:05:49PM +0800, Su Yue wrote:
Resize to nums without sign prefix makes false output:
 btrfs fi resize 1:150g /srv/extra
Resize device id 1 (/dev/sdb1) from 298.09GiB to 0.00B

The resize operation would take effect though.

Fix it by handling the case if mod is 0 in check_resize_args().

Issue: #307
Reported-by: Chris Murphy <li...@colorremedies.com>
Signed-off-by: Su Yue <l...@damenly.su>
---
 cmds/filesystem.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

---
Changelog:
    v2:
        Calculate u64 diff using max() and min().
        Calculate mod by comparing new and old size.
diff --git a/cmds/filesystem.c b/cmds/filesystem.c
index 9e3cce687d6e..54d46470c53f 100644
--- a/cmds/filesystem.c
+++ b/cmds/filesystem.c
@@ -1158,6 +1158,16 @@ static int check_resize_args(const char *amount, const char *path) {
                }
                old_size = di_args[dev_idx].total_bytes;

+ /* For target sizes without '+'/'-' sign prefix(e.g. 1:150g) */
+               if (mod == 0) {
+                       new_size = diff;
+ diff = max(old_size, new_size) - min(old_size, new_size);
+                       if (new_size > old_size)
+                               mod = 1;
+                       else if (new_size < old_size)
+                               mod = -1;
+               }
+
                if (mod < 0) {
                        if (diff > old_size) {
error("current size is %s which is smaller than %s",

This fix seems correct to me, but it feels a tiny bit over-complicated.
Personally, I think it would be cleaner to do something like:

if (mod == 0) {
        new_size = diff;
} else if (mod < 0) {
        // >0 check
        new_size = old_size - diff
} else {
        // overflow check
        new_size = old_size + diff
}

At this point, new_size is set correctly in all cases and we can do the print. I tested this version on some simple cases, and it seems to work
ok as well.

Right. The first fix when I saw the code was same with yours.
Now I relaized that I was over thinking about boundary checks and
falling through checks are meaningless if mod is 0.
Just to clarify that my fix is wrong:

      if (mod == 0) {
           new_size = diff;
diff = max(old_size, new_size) - min(old_size, new_size);
          if (new_size > old_size)
              mod = 1;
          else if (new_size < old_size)
              mod = -1;
      } /* falls through to check diff */

if (mod < 0) { // new_size < old_size, diff = old_size - new_size
                    // so diff < new_size
if (diff > old_size) // can't happen, if mod is 0 before.
                 ...
     } else if (mod > 0) {
       // new_size > old_size so diff = new_size(u64) - old_size,
if (diff > ULLONG_MAX - old_size) // can't happen even new_size is // ULLONG_MAX if mod is 0 before
             ...
     }


Thanks for your suggestion, will send v3.
--
Su

Thanks for the fix,
Boris

--
2.30.1

Reply via email to