On 05/24/2018 09:03 PM, David Sterba wrote:
On Wed, May 23, 2018 at 02:35:07PM +0800, Anand Jain wrote:
Balance arg info is an important information to be reviewed for the
system audit. So this patch adds them to the kernel log.

Example:

->btrfs bal start --full-balance -f -mprofiles=raid1,convert=single,soft 
-dlimit=10..20,usage=50 /btrfs

--full-balance should not be here as it's for the case when no filter is
used.

 Oh yep. I should remove that.

  kernel: BTRFS info (device sdc): balance: start -f -d usage=50,limit=10..20 
-m soft,profiles=raid1,convert=single -s soft,profiles=raid1,convert=single

While this looks good, it would not work as is because of the dual
syntax of -d and -m, the argument is optional and with the space between
it'll fail with "btrfs filesystem balance start: too many arguments" .

 I have dropped the space after -d -m -s and cut and paste works.
 I have it in the version v4.1a.

At least the main parts of the string are copy&paste ready, so the
format can be like

balance: start force D:usage=50,limit=10..20 
M:soft,profiles=raid1,convert=single S:soft,profiles=raid1,convert=single

 This not bad either. But I liked the idea of just plain of cut and
 paste and I am fine with either format. And I have this in the version
 v4.1b. So can you pls pick either v4.1a or v4.1b for the integration.

more below..

or with "DATA:" "METADATA:"


Signed-off-by: Anand Jain <anand.j...@oracle.com>
---
v3->v4: Change log updated with new example.
        Log format is changed to almost match with the cli.
        snprintf drop %s for inline string.
        Print range as =%u..%u instead of min=%umax=%u.
        soft is under the args now.
        force is printed as -f.

v2->v3: Change log updated.
         Make use of describe_block_group() added in 1/4
         Drop using strcat instead use snprintf.
         Logs string format updated as shown in the example above.

v1->v2: Change log update.
         Move adding the logs for balance complete and end to a new patch

  fs/btrfs/volumes.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++--
  1 file changed, 104 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 44f4799bf06f..85f954f7f93e 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3806,6 +3806,108 @@ static inline int validate_convert_profile(struct 
btrfs_balance_args *bctl_arg,
                 (bctl_arg->target & ~allowed)));
  }
+static u32 describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
+                                u32 size_buf)
+{
+       char *bp = buf;
+       u64 flags = bargs->flags;
+       char tmp_buf[128];
+
+       if (!flags)
+               return 0;
+
+       if (flags & BTRFS_BALANCE_ARGS_SOFT)
+               bp += snprintf(bp, buf - bp + size_buf, "soft,");
+
+       if (flags & BTRFS_BALANCE_ARGS_PROFILES) {
+               btrfs_describe_block_groups(bargs->profiles, tmp_buf,
+                                           sizeof(tmp_buf));
+               bp += snprintf(bp, buf - bp + size_buf, "profiles=%s,",
+                              tmp_buf);
+       }
+
+       if (flags & BTRFS_BALANCE_ARGS_USAGE)
+               bp += snprintf(bp, buf - bp + size_buf, "usage=%llu,",
+                              bargs->usage);
+
+       if (flags & BTRFS_BALANCE_ARGS_USAGE_RANGE)
+               bp += snprintf(bp, buf - bp + size_buf, "usage=%u..%u,",
+                              bargs->usage_min, bargs->usage_max);
+
+       if (flags & BTRFS_BALANCE_ARGS_DEVID)
+               bp += snprintf(bp, buf - bp + size_buf, "devid=%llu,",
+                              bargs->devid);
+
+       if (flags & BTRFS_BALANCE_ARGS_DRANGE)
+               bp += snprintf(bp, buf - bp + size_buf, "drange=%llu..%llu,",
+                              bargs->pstart, bargs->pend);
+
+       if (flags & BTRFS_BALANCE_ARGS_VRANGE)
+               bp += snprintf(bp, buf - bp + size_buf, "vrange%llu..%llu,",
+                              bargs->vstart, bargs->vend);
+
+       if (flags & BTRFS_BALANCE_ARGS_LIMIT)
+               bp += snprintf(bp, buf - bp + size_buf, "limit=%llu,",
+                              bargs->limit);
+
+       if (flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE)
+               bp += snprintf(bp, buf - bp + size_buf, "limit=%u..%u,",
+                              bargs->limit_min, bargs->limit_max);
+
+       if (flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE)
+               bp += snprintf(bp, buf - bp + size_buf, "stripes=%u..%u,",
+                              bargs->stripes_min, bargs->stripes_max);
+
+       if (flags & BTRFS_BALANCE_ARGS_CONVERT) {
+               int index = btrfs_bg_flags_to_raid_index(bargs->target);
+
+               bp += snprintf(bp, buf - bp + size_buf, "convert=%s,",
+                              get_raid_name(index));
+       }
+
+       buf[bp - buf - 1] = '\0'; /* remove last , */
+       return bp - buf - 1;
+}
+
+static void describe_balance_start_or_resume(struct btrfs_fs_info *fs_info)
+{
+       u32 sz = 1024;

 I have taken this opportunity to rename this to size_buf.

+       char tmp_buf[64];

This is too short, the profiles= can contain all profile names that
itself has 50 bytes, plus any filter, this will not fit the buffer.

 oh yes. Theoretically it should be more than the buffer in the
 describe_balance_args() + btrfs_describe_block_groups (128). Given
 that the user might specify all the options with the lengthy stripe
 range in describe_balance_args(), I have made it 192 bytes.

Other than that, it looks good and I'll add it to for-next. We may still
want tweak the exact output format and buffer sizes, there's a bit of time.

 I have both types sent (v4.1a: no space v4.1b: cap abbreviates) pls
 pick one. I would go by the method which ever is more easier to cut
 and paste.

Thanks, Anand

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to