Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Silence #12

Closed
wants to merge 2 commits into from
Closed

Silence #12

wants to merge 2 commits into from

Conversation

moviuro
Copy link

@moviuro moviuro commented Jan 13, 2016

This PR should be the first in a series of changes aiming at making btrfs(8) more script-friendly.
In here we remove the most annoying and noisy outputs.

@moviuro
cmds-subvolume.c: silence the create, delete and snapshot commands
35e4366 
All standard commands are silent upon success or unless told otherwise: why not
do the same here?
@moviuro
btrfs-subvolume.asciidoc: Reflect the silencing of the subvolume subc…
b7756bf 
…ommands
@kdave
Copy link
Owner

kdave commented Jan 14, 2016

Good idea in general, however this is a user visible change and likely to break scripts. Ideally we should rework the verbose output in one go. I'll look at it after 4.4 release.

@moviuro
Copy link
Author

moviuro commented Jan 14, 2016

My take on the issue was: commands should be silent in general. Hence "changing" the default behavior to a more standard one.
Also, the use of btrfs(8) in script is pretty complex because of some inconsistencies in its outputs (that's for an other PR). We could try to study who uses the verbose output in their projects (e.g. searching on github for people parsing btrfs sub create's output)

On a side note, I am working on https://github.com/moviuro/butter which hit a number of weird things that btrs-progs(8) do, hence my contributions.

Do you have any guidelines as to how I should continue to silence the commands?

@kdave
Copy link
Owner

kdave commented Jan 18, 2016

My idea of the end result, regardless of the transition to the new behaviour:

  • a command is silent unless it would be confusing what it did, ie. a single action like 'subvol create' is silent, 'device add' would print that it does trim (because it delays the whole action)
  • the return code is 0 if ok, other in case of errors
  • all commands get --verbose, and print what it does, repeated options are more verbose
  • all commands get --quiet, disable everything except errors, in the example above 'device add' will not say anything about the trimming

Transition period would be counted in years (1-2), which might sound silly, but deprecation is always counted in years. Warning to users about the unexpected command silence needs to be communicated.

Incrementally we can start adding the --verbose/--quiet options right now.

@moviuro
Copy link
Author

moviuro commented Jan 18, 2016

Okay, so let's add -q/-v everywhere we can so that btrfs(8) doesn't complain about them being unknown options.

I'd also suggest switching to getopts everywhere (remember the -- inconsistency?)

@kdave
Copy link
Owner

kdave commented Jan 19, 2016

Yeah, getopts should be used everywhere. I did not want to do that change just before the release. Please don't forget to send the patches to the mailinglist once you have them ready.

@moviuro
Copy link
Author

moviuro commented Jan 19, 2016

@kdave on a related note: how to I subscribe/register?

@kdave
Copy link
Owner

kdave commented Jan 19, 2016

https://btrfs.wiki.kernel.org/index.php/Btrfs_mailing_list

@kdave kdave closed this Jan 19, 2016
@elliotclee elliotclee mentioned this pull request May 23, 2019
Closed
@kobaz kobaz mentioned this pull request May 21, 2020
Open
@ralisi ralisi mentioned this pull request Sep 6, 2020
Closed
adam900710 added a commit to adam900710/btrfs-progs that referenced this pull request Jul 29, 2024
@adam900710
btrfs-progs: cmd/qgroup: fix memory leak for clear-stale subcommand
9132fa0 
[BUG]
ASAN test fails at misc/055 with the following leak:

Qgroupid    Referenced    Exclusive   Path
--------    ----------    ---------   ----
0/5           16.00KiB     16.00KiB   <toplevel>
0/256         16.00KiB     16.00KiB   <stale>
====== RUN CHECK /home/runner/work/btrfs-progs/btrfs-progs/btrfs qgroup clear-stale /home/runner/work/btrfs-progs/btrfs-progs/tests/mnt

=================================================================
==102571==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7fd1c98fbb37 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x55aa2f8953f8 in btrfs_util_subvolume_path_fd libbtrfsutil/subvolume.c:178
    kdave#2 0x55aa2f8fa2a6 in get_or_add_qgroup cmds/qgroup.c:837
    kdave#3 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    kdave#4 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    kdave#5 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    kdave#6 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    kdave#7 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    kdave#8 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    kdave#9 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    kdave#10 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    kdave#11 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    kdave#12 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    kdave#13 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

Indirect leak of 432 byte(s) in 2 object(s) allocated from:
    #0 0x7fd1c98fb4d0 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x55aa2f8fa1a1 in get_or_add_qgroup cmds/qgroup.c:822
    kdave#2 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    kdave#3 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    kdave#4 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    kdave#5 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    kdave#6 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    kdave#7 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    kdave#8 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    kdave#9 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    kdave#10 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    kdave#11 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    kdave#12 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

[CAUSE]
Above leaks are caused by two btrfs_qgroup structures and one path for
toplevel qgroup.

It's caused by the fact that we called qgroups_search_all() but didn't
do any cleanup.

[FIX]
Call __free_all_qgroups() inside cmd_qgroup_clear_stale() to properly
free the qgroups.

Fixes: 701ab15 ("btrfs-progs: qgroup: new command to delete stale qgroups")
Signed-off-by: Qu Wenruo <wqu@suse.com>
@adam900710 adam900710 mentioned this pull request Jul 29, 2024
Merged
kdave pushed a commit that referenced this pull request Jul 29, 2024
@adam900710 @kdave
btrfs-progs: cmd/qgroup: fix memory leak for clear-stale subcommand
e72cb06 
[BUG]
ASAN test fails at misc/055 with the following leak:

Qgroupid    Referenced    Exclusive   Path
--------    ----------    ---------   ----
0/5           16.00KiB     16.00KiB   <toplevel>
0/256         16.00KiB     16.00KiB   <stale>
====== RUN CHECK /home/runner/work/btrfs-progs/btrfs-progs/btrfs qgroup clear-stale /home/runner/work/btrfs-progs/btrfs-progs/tests/mnt

=================================================================
==102571==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7fd1c98fbb37 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x55aa2f8953f8 in btrfs_util_subvolume_path_fd libbtrfsutil/subvolume.c:178
    #2 0x55aa2f8fa2a6 in get_or_add_qgroup cmds/qgroup.c:837
    #3 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    #4 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    #5 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    #6 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    #7 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #8 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    #9 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #10 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    #11 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #12 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #13 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

Indirect leak of 432 byte(s) in 2 object(s) allocated from:
    #0 0x7fd1c98fb4d0 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x55aa2f8fa1a1 in get_or_add_qgroup cmds/qgroup.c:822
    #2 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    #3 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    #4 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    #5 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    #6 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #7 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    #8 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #9 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    #10 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #11 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #12 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

[CAUSE]
Above leaks are caused by two btrfs_qgroup structures and one path for
toplevel qgroup.

It's caused by the fact that we called qgroups_search_all() but didn't
do any cleanup.

[FIX]
Call __free_all_qgroups() inside cmd_qgroup_clear_stale() to properly
free the qgroups.

Fixes: 701ab15 ("btrfs-progs: qgroup: new command to delete stale qgroups")
Signed-off-by: Qu Wenruo <wqu@suse.com>
kdave pushed a commit that referenced this pull request Jul 30, 2024
@adam900710 @kdave
btrfs-progs: qgroup clear-stale: fix memory leak
5aed7ea 
[BUG]
ASAN test fails at misc/055 with the following leak:

Qgroupid    Referenced    Exclusive   Path
--------    ----------    ---------   ----
0/5           16.00KiB     16.00KiB   <toplevel>
0/256         16.00KiB     16.00KiB   <stale>
====== RUN CHECK /home/runner/work/btrfs-progs/btrfs-progs/btrfs qgroup clear-stale /home/runner/work/btrfs-progs/btrfs-progs/tests/mnt

=================================================================
==102571==ERROR: LeakSanitizer: detected memory leaks

Indirect leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7fd1c98fbb37 in malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 0x55aa2f8953f8 in btrfs_util_subvolume_path_fd libbtrfsutil/subvolume.c:178
    #2 0x55aa2f8fa2a6 in get_or_add_qgroup cmds/qgroup.c:837
    #3 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    #4 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    #5 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    #6 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    #7 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #8 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    #9 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #10 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    #11 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #12 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #13 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

Indirect leak of 432 byte(s) in 2 object(s) allocated from:
    #0 0x7fd1c98fb4d0 in calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:77
    #1 0x55aa2f8fa1a1 in get_or_add_qgroup cmds/qgroup.c:822
    #2 0x55aa2f8fa7e9 in update_qgroup_info cmds/qgroup.c:883
    #3 0x55aa2f8fd912 in __qgroups_search cmds/qgroup.c:1385
    #4 0x55aa2f8fe196 in qgroups_search_all cmds/qgroup.c:1453
    #5 0x55aa2f902a7c in cmd_qgroup_clear_stale cmds/qgroup.c:2281
    #6 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #7 0x55aa2f734bcc in handle_command_group /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:177
    #8 0x55aa2f73425b in cmd_execute cmds/commands.h:126
    #9 0x55aa2f735a96 in main /home/runner/work/btrfs-progs/btrfs-progs/btrfs.c:518
    #10 0x7fd1c942a1c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #11 0x7fd1c942a28a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 08134323d00289185684a4cd177d202f39c2a5f3)
    #12 0x55aa2f734144 in _start (/home/runner/work/btrfs-progs/btrfs-progs/btrfs+0x84144) (BuildId: 56f3dd838e1ae189c142c5d27fac025cd46deddb)

[CAUSE]
Above leaks are caused by two btrfs_qgroup structures and one path for
toplevel qgroup.

It's caused by the fact that we called qgroups_search_all() but didn't
do any cleanup.

[FIX]
Call __free_all_qgroups() inside cmd_qgroup_clear_stale() to properly
free the qgroups.

Fixes: 701ab15 ("btrfs-progs: qgroup: new command to delete stale qgroups")
Signed-off-by: Qu Wenruo <wqu@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@moviuro @kdave