Allow tmpfile compression to be disabled#110
Allow tmpfile compression to be disabled#110charles-dyfis-net wants to merge 1 commit intoZygo:masterfrom
Conversation
kakra
left a comment
kakra
left a comment
There was a problem hiding this comment.
I'm not sure if this should be upstreamed at all. Is there any reason why you'd not want compression of data processed by bees besides working around a kernel bug that should be in the distribution in the first place? The only reasons I can think of would make you force compression off globally anyways.
| { "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' }, |
| 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); | ||
| } |
There was a problem hiding this comment.
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.
| "\n" | ||
| "Workarounds:\n" | ||
| " -a, --workaround-btrfs-send Workaround for btrfs send\n" | ||
| " -N, --no-tmpfile-compression Disable compression for temporary files\n" |
There was a problem hiding this comment.
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.
|
Considering #107 (comment), I think this should just be closed. |
|
There is still something to do in this area:
That said, these should be their own issue. |
4 participants
For kernels without #107 applied