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

Allow tmpfile compression to be disabled #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow tmpfile compression to be disabled #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions src/bees.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ using namespace crucible;
using namespace std;

int bees_log_level = 8;
bool bees_tmpfile_compression_disabled = false;

void
do_cmd_help(char *argv[])
Expand All @@ -56,6 +57,7 @@ do_cmd_help(char *argv[])
"\n"
"Workarounds:\n"
" -a, --workaround-btrfs-send Workaround for btrfs send\n"
" -N, --no-tmpfile-compression Disable compression for temporary files\n"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the naming could be better: What it actually wants to do is decompressing shared extents. Nobody cares if this is done through tmpfiles or other means. Options should try not exposing too many details of the internal workings so they can still be true after code redesigns.

"\n"
"Logging options:\n"
" -t, --timestamps Show timestamps in log output (default)\n"
Expand Down Expand Up @@ -513,11 +515,13 @@ BeesTempFile::create()
m_ctx->insert_root_ino(m_fd);

// Set compression attribute
BEESTRACE("Getting FS_COMPR_FL on m_fd " << name_fd(m_fd));
int flags = ioctl_iflags_get(m_fd);
flags |= FS_COMPR_FL;
BEESTRACE("Setting FS_COMPR_FL on m_fd " << name_fd(m_fd) << " flags " << to_hex(flags));
ioctl_iflags_set(m_fd, flags);
if(!bees_tmpfile_compression_disabled) {
BEESTRACE("Getting FS_COMPR_FL on m_fd " << name_fd(m_fd));
int flags = ioctl_iflags_get(m_fd);
flags |= FS_COMPR_FL;
BEESTRACE("Setting FS_COMPR_FL on m_fd " << name_fd(m_fd) << " flags " << to_hex(flags));
ioctl_iflags_set(m_fd, flags);
}
Comment on lines +518 to +524
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is wrong as it won't prevent compression if the parent directory of the tmpfile has the compression attribute set. This would leave you with compression still active even when you said no-tmpfile-compression on the cmdline. That would be quite a surprising effect. Actually, I think it's better to get the updated kernel or force compression off for the whole filesystem if this kernel issue is a concern.


// Always leave first block empty to avoid creating a file with an inline extent
m_end_offset = BLOCK_SIZE_CLONE;
Expand Down Expand Up @@ -779,6 +783,7 @@ bees_main(int argc, char *argv[])
{ "strip-paths", no_argument, NULL, 'P' },
{ "no-timestamps", no_argument, NULL, 'T' },
{ "workaround-btrfs-send", no_argument, NULL, 'a' },
{ "no-tmpfile-compression", no_argument, NULL, 'N' },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation isn't correct

{ "thread-count", required_argument, NULL, 'c' },
{ "loadavg-target", required_argument, NULL, 'g' },
{ "help", no_argument, NULL, 'h' },
Expand Down Expand Up @@ -832,6 +837,9 @@ bees_main(int argc, char *argv[])
case 'a':
workaround_btrfs_send = true;
break;
case 'N':
bees_tmpfile_compression_disabled = true;
break;
case 'c':
thread_count = stoul(optarg);
break;
Expand Down
1 change: 1 addition & 0 deletions src/bees.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,7 @@ class BeesTooLong : public Timer {

// And now, a giant pile of extern declarations
extern int bees_log_level;
extern bool bees_tmpfile_compression_disabled;
extern const char *BEES_VERSION;
string pretty(double d);
void bees_sync(int fd);
Expand Down