From 2bf30ba60d11382f68eee83a0fec1fa442be36d2 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Wed, 22 Jan 2025 20:34:20 +0100 Subject: [PATCH 01/23] btrfs-profs: mkfs: update compression help and documentation Move the option documentation next to --rootdir, reword documentation. Signed-off-by: David Sterba --- Documentation/mkfs.btrfs.rst | 18 +++++++++--------- mkfs/main.c | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Documentation/mkfs.btrfs.rst b/Documentation/mkfs.btrfs.rst index b830f8c5b..d488502b7 100644 --- a/Documentation/mkfs.btrfs.rst +++ b/Documentation/mkfs.btrfs.rst @@ -155,6 +155,15 @@ OPTIONS contain the files from *rootdir*. Since version 4.14.1 the filesystem size is not minimized. Please see option *--shrink* if you need that functionality. +--compress [:] + Try to compress files when using *--rootdir*. Supported values for *algo* are + *no* (the default), *zstd*, *lzo* or *zlib*. The optional value *level* is a + compression level, 1..15 for *zstd*, 1..9 for *zlib*. + + As with the kernel, :command:`mkfs.btrfs` won't write compressed extents when + they would be larger than the uncompressed versions, and will set file attribute + *NOCOMPRESS* if its beginning is found to be incompressible. + -u|--subvol : Specify that *subdir* is to be created as a subvolume rather than a regular directory. The option *--rootdir* must also be specified, and *subdir* must be an @@ -212,15 +221,6 @@ OPTIONS $ mkfs.btrfs -O list-all ---compress [:] - Try to compress files when using *--rootdir*. Supported values for *algo* are - *no* (the default), *zlib*, *lzo*, and *zstd*. The optional value *level* is a - compression level, from 1 to 9 for ZLIB and from 1 to 15 for ZSTD. - - As with the kernel, :command:`mkfs.btrfs` won't write compressed extents when - they would be larger than the uncompressed versions, and will mark a file as - `nocompress` if its beginning is found to be incompressible. - -f|--force Forcibly overwrite the block devices when an existing filesystem is detected. By default, :command:`mkfs.btrfs` will utilize *libblkid* to check for any known diff --git a/mkfs/main.c b/mkfs/main.c index 08d1e1f74..ce438a7fe 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -441,10 +441,10 @@ static const char * const mkfs_usage[] = { "Creation:", OPTLINE("-b|--byte-count SIZE", "set size of each device to SIZE (filesystem size is sum of all device sizes)"), OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory"), + OPTLINE("--compress ALGO[:LEVEL]", "compress files by algorithm and level, ALGO can be 'no' (default), zstd, lzo, zlib"), OPTLINE("-u|--subvol TYPE:SUBDIR", "create SUBDIR as subvolume rather than normal directory, can be specified multiple times"), OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"), OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"), - OPTLINE("--compress ALGO[:LEVEL]", "compression algorithm and level to use; ALGO can be no (default), zlib, lzo, zstd"), OPTLINE("-f|--force", "force overwrite of existing filesystem"), "", "General:", From bf86e9505cf212e0afb71f76a9a452f76a9f4f73 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Wed, 22 Jan 2025 20:41:35 +0100 Subject: [PATCH 02/23] btrfs-progs: mkfs: print which compression algos are compiled in The compression support is optional, eg. also in 'btrfs-restore', so print the support in help text. usage: mkfs.btrfs [options] [] ... --compress ALGO[:LEVEL] compress files by algorithm and level, ALGO can be 'no' (default), zstd, lzo, zlib Built-in: - ZSTD: yes - LZO: yes - ZLIB: yes ... Signed-off-by: David Sterba --- Documentation/mkfs.btrfs.rst | 4 ++++ mkfs/main.c | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/Documentation/mkfs.btrfs.rst b/Documentation/mkfs.btrfs.rst index d488502b7..39b7d51a5 100644 --- a/Documentation/mkfs.btrfs.rst +++ b/Documentation/mkfs.btrfs.rst @@ -164,6 +164,10 @@ OPTIONS they would be larger than the uncompressed versions, and will set file attribute *NOCOMPRESS* if its beginning is found to be incompressible. + .. note:: + The support for ZSTD and LZO is a compile-time option, please check + the output of :command:`mkfs.btrfs --help` for the actual support. + -u|--subvol : Specify that *subdir* is to be created as a subvolume rather than a regular directory. The option *--rootdir* must also be specified, and *subdir* must be an diff --git a/mkfs/main.c b/mkfs/main.c index ce438a7fe..11dbc93a6 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -442,6 +442,18 @@ static const char * const mkfs_usage[] = { OPTLINE("-b|--byte-count SIZE", "set size of each device to SIZE (filesystem size is sum of all device sizes)"), OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory"), OPTLINE("--compress ALGO[:LEVEL]", "compress files by algorithm and level, ALGO can be 'no' (default), zstd, lzo, zlib"), + OPTLINE("", "Built-in:"), +#if COMPRESSION_ZSTD + OPTLINE("", "- ZSTD: yes"), +#else + OPTLINE("", "- ZSTD: no"), +#endif +#if COMPRESSION_LZO + OPTLINE("", "- LZO: yes"), +#else + OPTLINE("", "- LZO: no"), +#endif + OPTLINE("", "- ZLIB: yes"), OPTLINE("-u|--subvol TYPE:SUBDIR", "create SUBDIR as subvolume rather than normal directory, can be specified multiple times"), OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"), OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"), From 83d1c5ce42cd6740b3f71c912f80344d08ed266b Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 00:35:05 +0100 Subject: [PATCH 03/23] btrfs-progs: mkfs: factor out compression option parsing Put option parsing to its own helper to keep the switch/case smaller. Signed-off-by: David Sterba --- mkfs/main.c | 69 ++++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/mkfs/main.c b/mkfs/main.c index 11dbc93a6..610d9cd94 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1031,6 +1031,40 @@ static void *prepare_one_device(void *ctx) return NULL; } +static int parse_compression(const char *str, enum btrfs_compression_type *type, u64 *level) +{ + const char *colon; + size_t type_size; + + *level = 0; + if (strcmp(str, "no") == 0) { + *type = BTRFS_COMPRESS_NONE; + return 0; + } + + colon = strstr(str, ":"); + + if (colon) + type_size = colon - str; + else + type_size = strlen(str); + + if (strncmp(str, "zlib", type_size) == 0) { + *type = BTRFS_COMPRESS_ZLIB; + } else if (strncmp(str, "lzo", type_size) == 0) { + *type = BTRFS_COMPRESS_LZO; + } else if (strncmp(str, "zstd", type_size) == 0) { + *type = BTRFS_COMPRESS_ZSTD; + } else { + return 1; + } + + if (colon) + *level = arg_strtou64(colon + 1); + + return 0; +} + int BOX_MAIN(mkfs)(int argc, char **argv) { char *file; @@ -1293,42 +1327,13 @@ int BOX_MAIN(mkfs)(int argc, char **argv) case 'q': bconf_be_quiet(); break; - case GETOPT_VAL_COMPRESS: { - char *colon; - size_t type_size; - - if (!strcmp(optarg, "no")) { - compression = BTRFS_COMPRESS_NONE; - break; - } - - colon = strstr(optarg, ":"); - - if (colon) - type_size = colon - optarg; - else - type_size = strlen(optarg); - - if (!strncmp(optarg, "zlib", type_size)) { - compression = BTRFS_COMPRESS_ZLIB; - } else if (!strncmp(optarg, "lzo", type_size)) { - compression = BTRFS_COMPRESS_LZO; - } else if (!strncmp(optarg, "zstd", type_size)) { - compression = BTRFS_COMPRESS_ZSTD; - } else { - error("unrecognized compression type %s", - optarg); + case GETOPT_VAL_COMPRESS: + if (parse_compression(optarg, &compression, &compression_level)) { + error("invalid compression specification: %s", optarg); ret = 1; goto error; } - - if (colon) - compression_level = arg_strtou64(colon + 1); - else - compression_level = 0; - break; - } case GETOPT_VAL_DEVICE_UUID: strncpy_null(dev_uuid, optarg, BTRFS_UUID_UNPARSED_SIZE); break; From ecc28c5909ba4f5241a3be955834006c4bbef924 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 00:42:49 +0100 Subject: [PATCH 04/23] btrfs-progs: mkfs: change compression level type to 32bit type The level is a small number, no need to pass it around as u64. Signed-off-by: David Sterba --- mkfs/main.c | 13 +++++++++---- mkfs/rootdir.c | 2 +- mkfs/rootdir.h | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/mkfs/main.c b/mkfs/main.c index 610d9cd94..9c3a019f8 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1031,7 +1031,8 @@ static void *prepare_one_device(void *ctx) return NULL; } -static int parse_compression(const char *str, enum btrfs_compression_type *type, u64 *level) +static int parse_compression(const char *str, enum btrfs_compression_type *type, + unsigned int *level) { const char *colon; size_t type_size; @@ -1059,8 +1060,12 @@ static int parse_compression(const char *str, enum btrfs_compression_type *type, return 1; } - if (colon) - *level = arg_strtou64(colon + 1); + if (colon) { + u64 tmplevel = arg_strtou64(colon + 1); + + if (tmplevel > UINT_MAX) + return 1; + } return 0; } @@ -1109,7 +1114,7 @@ int BOX_MAIN(mkfs)(int argc, char **argv) struct rootdir_subvol *rds; bool has_default_subvol = false; enum btrfs_compression_type compression = BTRFS_COMPRESS_NONE; - u64 compression_level = 0; + unsigned int compression_level = 0; LIST_HEAD(subvols); cpu_detect_flags(); diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c index a3ec75459..e0a7ffcbc 100644 --- a/mkfs/rootdir.c +++ b/mkfs/rootdir.c @@ -1637,7 +1637,7 @@ static int set_default_subvolume(struct btrfs_trans_handle *trans) int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir, struct btrfs_root *root, struct list_head *subvols, enum btrfs_compression_type compression, - u64 compression_level) + unsigned int compression_level) { int ret; struct stat root_st; diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h index f0f231619..7b16de00f 100644 --- a/mkfs/rootdir.h +++ b/mkfs/rootdir.h @@ -42,7 +42,7 @@ struct rootdir_subvol { int btrfs_mkfs_fill_dir(struct btrfs_trans_handle *trans, const char *source_dir, struct btrfs_root *root, struct list_head *subvols, enum btrfs_compression_type compression, - u64 compression_level); + unsigned int compression_level); u64 btrfs_mkfs_size_dir(const char *dir_name, u32 sectorsize, u64 min_dev_size, u64 meta_profile, u64 data_profile); int btrfs_mkfs_shrink_fs(struct btrfs_fs_info *fs_info, u64 *new_size_ret, From a2cac3707c3a6c8ac933dba0449c0e6d517b5552 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 00:56:09 +0100 Subject: [PATCH 05/23] btrfs-progs: mkfs: move compression level definitions to rootdir.h The constants will be used in main() to validate command line options. Signed-off-by: David Sterba --- mkfs/rootdir.c | 6 ------ mkfs/rootdir.h | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/mkfs/rootdir.c b/mkfs/rootdir.c index e0a7ffcbc..735c30d39 100644 --- a/mkfs/rootdir.c +++ b/mkfs/rootdir.c @@ -54,12 +54,6 @@ #include "common/rbtree-utils.h" #include "mkfs/rootdir.h" -#define ZLIB_BTRFS_DEFAULT_LEVEL 3 -#define ZLIB_BTRFS_MAX_LEVEL 9 - -#define ZSTD_BTRFS_DEFAULT_LEVEL 3 -#define ZSTD_BTRFS_MAX_LEVEL 15 - #define LZO_LEN 4 static u32 fs_block_size; diff --git a/mkfs/rootdir.h b/mkfs/rootdir.h index 7b16de00f..b32fda5bf 100644 --- a/mkfs/rootdir.h +++ b/mkfs/rootdir.h @@ -28,6 +28,12 @@ #include "kernel-lib/list.h" #include "kernel-shared/compression.h" +#define ZLIB_BTRFS_DEFAULT_LEVEL 3 +#define ZLIB_BTRFS_MAX_LEVEL 9 + +#define ZSTD_BTRFS_DEFAULT_LEVEL 3 +#define ZSTD_BTRFS_MAX_LEVEL 15 + struct btrfs_fs_info; struct btrfs_root; From 9a1b63f1a2a29811d3cd9941e42cf7327ac10196 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 00:51:38 +0100 Subject: [PATCH 06/23] btrfs-progs: mkfs: validate compression levels while parsing options Report invalid compression specification while parsing the options. Now an ivalid level won't be silently accepted and capped when processing the files. Other checks regarding conditional support of LZO and ZSTD are left in place. Signed-off-by: David Sterba --- mkfs/main.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/mkfs/main.c b/mkfs/main.c index 9c3a019f8..3873c87cf 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1036,6 +1036,7 @@ static int parse_compression(const char *str, enum btrfs_compression_type *type, { const char *colon; size_t type_size; + unsigned int default_level, max_level; *level = 0; if (strcmp(str, "no") == 0) { @@ -1052,10 +1053,26 @@ static int parse_compression(const char *str, enum btrfs_compression_type *type, if (strncmp(str, "zlib", type_size) == 0) { *type = BTRFS_COMPRESS_ZLIB; + max_level = ZLIB_BTRFS_MAX_LEVEL; + default_level = ZLIB_BTRFS_DEFAULT_LEVEL; } else if (strncmp(str, "lzo", type_size) == 0) { +#if COMPRESSION_LZO *type = BTRFS_COMPRESS_LZO; + max_level = 1; + default_level = 1; +#else + error("lzo support not compiled in"); + return 1; +#endif } else if (strncmp(str, "zstd", type_size) == 0) { +#if COMPRESSION_ZSTD *type = BTRFS_COMPRESS_ZSTD; + max_level = ZSTD_BTRFS_MAX_LEVEL; + default_level = ZSTD_BTRFS_DEFAULT_LEVEL; +#else + error("zstd support not compiled in"); + return 1; +#endif } else { return 1; } @@ -1065,6 +1082,16 @@ static int parse_compression(const char *str, enum btrfs_compression_type *type, if (tmplevel > UINT_MAX) return 1; + + if (tmplevel == 0) + *level = default_level; + else if (tmplevel > max_level) { + error("compression level %llu out of range [1..%u]", + tmplevel, max_level); + return 1; + } else { + *level = tmplevel; + } } return 0; @@ -1334,7 +1361,6 @@ int BOX_MAIN(mkfs)(int argc, char **argv) break; case GETOPT_VAL_COMPRESS: if (parse_compression(optarg, &compression, &compression_level)) { - error("invalid compression specification: %s", optarg); ret = 1; goto error; } From b81c8396cff38ad8bee12b5ca52bba71adca65ce Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:08:44 +0100 Subject: [PATCH 07/23] btrfs-progs: mkfs: document level ranges for zlib and zstd in help Signed-off-by: David Sterba --- mkfs/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mkfs/main.c b/mkfs/main.c index 3873c87cf..b2f764b13 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -444,7 +444,7 @@ static const char * const mkfs_usage[] = { OPTLINE("--compress ALGO[:LEVEL]", "compress files by algorithm and level, ALGO can be 'no' (default), zstd, lzo, zlib"), OPTLINE("", "Built-in:"), #if COMPRESSION_ZSTD - OPTLINE("", "- ZSTD: yes"), + OPTLINE("", "- ZSTD: yes (levels 1..15)"), #else OPTLINE("", "- ZSTD: no"), #endif @@ -453,7 +453,7 @@ static const char * const mkfs_usage[] = { #else OPTLINE("", "- LZO: no"), #endif - OPTLINE("", "- ZLIB: yes"), + OPTLINE("", "- ZLIB: yes (levels 1..9)"), OPTLINE("-u|--subvol TYPE:SUBDIR", "create SUBDIR as subvolume rather than normal directory, can be specified multiple times"), OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"), OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"), From 4a5e3fa23cc00da1eb752eaa5582561052530ff8 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:11:29 +0100 Subject: [PATCH 08/23] btrfs-progs: mkfs: add --compress and --rootdir constraints It does not make sense to pass only the compression option when there are no files being added by --rootdir. Signed-off-by: David Sterba --- mkfs/main.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mkfs/main.c b/mkfs/main.c index b2f764b13..d367f2280 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -1437,6 +1437,12 @@ int BOX_MAIN(mkfs)(int argc, char **argv) free(source_dir); source_dir = canonical; + } else { + if (compression != BTRFS_COMPRESS_NONE) { + error("--compression must be used with --rootdir"); + ret = 1; + goto error; + } } list_for_each_entry(rds, &subvols, list) { From dfee6181ba235958fb34c43479cf2ec31d7328ee Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:17:06 +0100 Subject: [PATCH 09/23] btrfs-progs: mkfs: print in summary if compression is used $ mkfs.btrfs --compress zlib:9 --rootdir Documentation img ... Rootdir from: /path/Documentation Compress: zlib:9 Shrink: no ... Signed-off-by: David Sterba --- mkfs/main.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/mkfs/main.c b/mkfs/main.c index d367f2280..4c272167a 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -2051,6 +2051,15 @@ int BOX_MAIN(mkfs)(int argc, char **argv) goto out; } + pr_verbose(LOG_DEFAULT, " Compress: %s%s%s\n", + compression == BTRFS_COMPRESS_ZSTD ? "zstd" : + compression == BTRFS_COMPRESS_LZO ? "lzo" : + compression == BTRFS_COMPRESS_ZLIB ? "zlib" : "no", + compression_level > 0 ? ":" : "", + compression_level > 0 ? + pretty_size_mode(compression_level, UNITS_RAW) : + ""); + ret = btrfs_mkfs_fill_dir(trans, source_dir, root, &subvols, compression, compression_level); From 57ca92422e9a1355488f27a145e9f827c24bd162 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:37:32 +0100 Subject: [PATCH 10/23] btrfs-progs: build: move --disable-lzo option closer to zstd There are two configure options for compression so group them and update the description so it mentions all commands that use compression. Signed-off-by: David Sterba --- configure.ac | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/configure.ac b/configure.ac index 15c8f12b8..d098671e9 100644 --- a/configure.ac +++ b/configure.ac @@ -454,7 +454,7 @@ PKG_CHECK_MODULES(ZLIB, [zlib]) PKG_STATIC(ZLIB_LIBS_STATIC, [zlib]) AC_ARG_ENABLE([zstd], - AS_HELP_STRING([--disable-zstd], [build without zstd support for restore and receive (default: enabled)]), + AS_HELP_STRING([--disable-zstd], [build without zstd compression support (btrfs-restore, btrfs-receive, mkfs.btrfs) (default: enabled)]), [], [enable_zstd=yes] ) @@ -466,6 +466,27 @@ fi AS_IF([test "x$enable_zstd" = xyes], [COMPRESSION_ZSTD=1], [COMPRESSION_ZSTD=0]) AC_SUBST(COMPRESSION_ZSTD) +AC_ARG_ENABLE([lzo], + AS_HELP_STRING([--disable-lzo], [build without lzo compression support (btrfs-restore, btrfs-receive, mkfs.btrfs) (default: enabled)]), + [], [enable_lzo=yes] +) + +if test "x$enable_lzo" = xyes; then + dnl lzo library does not provide pkg-config, use classic way + AC_CHECK_LIB([lzo2], [lzo_version], [ + LZO2_LIBS="-llzo2" + LZO2_CFLAGS="" + LZO2_LIBS_STATIC="-llzo2"],[ + AC_MSG_ERROR([cannot find lzo2 library]) + ]) + AC_SUBST([LZO2_LIBS]) + AC_SUBST([LZO2_LIBS_STATIC]) + AC_SUBST([LZO2_CFLAGS]) +fi + +AS_IF([test "x$enable_lzo" = xyes], [COMPRESSION_LZO=1], [COMPRESSION_LZO=0]) +AC_SUBST(COMPRESSION_LZO) + AC_ARG_ENABLE([libudev], AS_HELP_STRING([--disable-libudev], [build without libudev support (for multipath)]), [], [enable_libudev=yes] @@ -513,27 +534,6 @@ if ${PKG_CONFIG} udev --atleast-version 190; then fi AC_SUBST(UDEVDIR) -AC_ARG_ENABLE([lzo], - AS_HELP_STRING([--disable-lzo], [build without lzo support for restore and receive (default: enabled)]), - [], [enable_lzo=yes] -) - -if test "x$enable_lzo" = xyes; then - dnl lzo library does not provide pkg-config, use classic way - AC_CHECK_LIB([lzo2], [lzo_version], [ - LZO2_LIBS="-llzo2" - LZO2_CFLAGS="" - LZO2_LIBS_STATIC="-llzo2"],[ - AC_MSG_ERROR([cannot find lzo2 library]) - ]) - AC_SUBST([LZO2_LIBS]) - AC_SUBST([LZO2_LIBS_STATIC]) - AC_SUBST([LZO2_CFLAGS]) -fi - -AS_IF([test "x$enable_lzo" = xyes], [COMPRESSION_LZO=1], [COMPRESSION_LZO=0]) -AC_SUBST(COMPRESSION_LZO) - dnl call PKG_INSTALLDIR from pkg.m4 to set pkgconfigdir m4_ifdef([PKG_INSTALLDIR], [PKG_INSTALLDIR], [AC_MSG_ERROR([please install pkgconf])]) From 18d71bb993c89d47642494ac4840f57cd8295d95 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:52:09 +0100 Subject: [PATCH 11/23] btrfs-progs: image: add option --version, update help text For completeness and parity with other tools add the option --version and mention --help. Signed-off-by: David Sterba --- image/main.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/image/main.c b/image/main.c index 51cf80ad2..3d61074a9 100644 --- a/image/main.c +++ b/image/main.c @@ -56,6 +56,10 @@ static const char * const image_usage[] = { OPTLINE("-m", "restore for multiple devices"), OPTLINE("-d", "also dump data, conflicts with -w"), "", + "General:", + OPTLINE("--version", "print the btrfs-image version and exit"), + OPTLINE("--help", "print this help and exit"), + "", "In the dump mode, source is the btrfs device and target is the output file (use '-' for stdout).", "In the restore mode, source is the dumped image and target is the btrfs device/file.", NULL @@ -86,8 +90,10 @@ int BOX_MAIN(image)(int argc, char *argv[]) hash_init_accel(); while (1) { + enum { GETOPT_VAL_VERSION = GETOPT_VAL_FIRST }; static const struct option long_options[] = { { "help", no_argument, NULL, GETOPT_VAL_HELP}, + { "version", no_argument, NULL, GETOPT_VAL_VERSION }, { NULL, 0, NULL, 0 } }; int c = getopt_long(argc, argv, "rc:t:oswmd", long_options, NULL); @@ -133,6 +139,10 @@ int BOX_MAIN(image)(int argc, char *argv[]) btrfs_warn_experimental("Feature: dump image with data"); dump_data = true; break; + case GETOPT_VAL_VERSION: + printf("btrfs-image, part of %s\n", PACKAGE_STRING); + ret = 0; + goto success; case GETOPT_VAL_HELP: default: usage(&image_cmd, c != GETOPT_VAL_HELP); @@ -293,5 +303,6 @@ int BOX_MAIN(image)(int argc, char *argv[]) btrfs_close_all_devices(); +success: return !!ret; } From 5e5764a64dbc0c39920fa0c0476646cb56b17ad9 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:52:09 +0100 Subject: [PATCH 12/23] btrfs-progs: btrfstune: add option --version, update help text For completeness and parity with other tools add the option --version and mention --help. Signed-off-by: David Sterba --- tune/main.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tune/main.c b/tune/main.c index 91e70eeb6..c4cafa290 100644 --- a/tune/main.c +++ b/tune/main.c @@ -123,7 +123,8 @@ static const char * const tune_usage[] = { "", "General:", OPTLINE("-f", "allow dangerous operations, make sure that you are aware of the dangers"), - OPTLINE("--help", "print this help"), + OPTLINE("--version", "print the btrfstune version and exit"), + OPTLINE("--help", "print this help and exit"), #if EXPERIMENTAL "", "EXPERIMENTAL FEATURES:", @@ -212,9 +213,11 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[]) GETOPT_VAL_ENABLE_FREE_SPACE_TREE, GETOPT_VAL_ENABLE_SIMPLE_QUOTA, GETOPT_VAL_REMOVE_SIMPLE_QUOTA, + GETOPT_VAL_VERSION, }; static const struct option long_options[] = { { "help", no_argument, NULL, GETOPT_VAL_HELP}, + { "version", no_argument, NULL, GETOPT_VAL_VERSION }, { "convert-to-block-group-tree", no_argument, NULL, GETOPT_VAL_ENABLE_BLOCK_GROUP_TREE}, { "convert-from-block-group-tree", no_argument, NULL, @@ -305,6 +308,10 @@ int BOX_MAIN(btrfstune)(int argc, char *argv[]) btrfstune_cmd_groups[CSUM_CHANGE] = true; break; #endif + case GETOPT_VAL_VERSION: + printf("btrfstune, part of %s\n", PACKAGE_STRING); + ret = 0; + goto free_out; case GETOPT_VAL_HELP: default: usage(&tune_cmd, c != GETOPT_VAL_HELP); From 6db97057a800650faad68c490e6364393ec51665 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 01:52:09 +0100 Subject: [PATCH 13/23] btrfs-progs: convert: add option --version, update help text For completeness and parity with other tools add the option --version and mention --help. Signed-off-by: David Sterba --- convert/main.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/convert/main.c b/convert/main.c index bf31bb3a9..98b96b5f0 100644 --- a/convert/main.c +++ b/convert/main.c @@ -1868,6 +1868,10 @@ static const char * const convert_usage[] = { OPTLINE("-O|--features LIST", "comma separated list of filesystem features"), OPTLINE("--no-progress", "show only overview, not the detailed progress"), "", + "General:", + OPTLINE("--version", "print the btrfs-convert version and exit"), + OPTLINE("--help", "print this help and exit"), + "", "Supported filesystems:", "\text2/3/4: " #if BTRFSCONVERT_EXT2 @@ -1910,11 +1914,10 @@ int BOX_MAIN(convert)(int argc, char *argv[]) cpu_detect_flags(); hash_init_accel(); btrfs_assert_feature_buf_size(); - printf("btrfs-convert from %s\n\n", PACKAGE_STRING); while(1) { enum { GETOPT_VAL_NO_PROGRESS = GETOPT_VAL_FIRST, GETOPT_VAL_CHECKSUM, - GETOPT_VAL_UUID }; + GETOPT_VAL_UUID, GETOPT_VAL_VERSION }; static const struct option long_options[] = { { "no-progress", no_argument, NULL, GETOPT_VAL_NO_PROGRESS }, @@ -1932,7 +1935,8 @@ int BOX_MAIN(convert)(int argc, char *argv[]) { "copy-label", no_argument, NULL, 'L' }, { "uuid", required_argument, NULL, GETOPT_VAL_UUID }, { "nodesize", required_argument, NULL, 'N' }, - { "help", no_argument, NULL, GETOPT_VAL_HELP}, + { "help", no_argument, NULL, GETOPT_VAL_HELP }, + { "version", no_argument, NULL, GETOPT_VAL_VERSION }, { NULL, 0, NULL, 0 } }; int c = getopt_long(argc, argv, "dinN:rl:LpO:", long_options, NULL); @@ -2024,11 +2028,18 @@ int BOX_MAIN(convert)(int argc, char *argv[]) strncpy_null(fsid, optarg, sizeof(fsid)); } break; + case GETOPT_VAL_VERSION: + printf("btrfs-convert, part of %s\n", PACKAGE_STRING); + ret = 0; + goto success; case GETOPT_VAL_HELP: default: usage(&convert_cmd, c != GETOPT_VAL_HELP); } } + + printf("btrfs-convert from %s\n\n", PACKAGE_STRING); + set_argv0(argv); if (check_argc_exact(argc - optind, 1)) { usage(&convert_cmd, 1); @@ -2072,5 +2083,6 @@ int BOX_MAIN(convert)(int argc, char *argv[]) } if (ret) return 1; +success: return 0; } From f4a72e3dcf1e62e6c22578c3f8cc08f9c632256c Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 02:17:21 +0100 Subject: [PATCH 14/23] btrfs-progs: mkfs: update help text Enhance information in the help text where some interesting information was not missing and would require looking up the documentation. Signed-off-by: David Sterba --- mkfs/main.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/mkfs/main.c b/mkfs/main.c index 4c272167a..e0c419eaf 100644 --- a/mkfs/main.c +++ b/mkfs/main.c @@ -431,16 +431,16 @@ static const char * const mkfs_usage[] = { "Features:", OPTLINE("--csum TYPE", ""), OPTLINE("--checksum TYPE", "checksum algorithm to use, crc32c (default), xxhash, sha256, blake2"), - OPTLINE("-n|--nodesize SIZE", "size of btree nodes"), - OPTLINE("-s|--sectorsize SIZE", "data block size (may not be mountable by current kernel)"), + OPTLINE("-n|--nodesize SIZE", "size of btree nodes (default 16K)"), + OPTLINE("-s|--sectorsize SIZE", "data block size (default 4K, may not be mountable by current kernel if CPU page size is different)"), OPTLINE("-O|--features LIST", "comma separated list of filesystem features (use '-O list-all' to list features)"), - OPTLINE("-L|--label LABEL", "set the filesystem label"), + OPTLINE("-L|--label LABEL", "set the filesystem label (maximum length 255)"), OPTLINE("-U|--uuid UUID", "specify the filesystem UUID (must be unique for a filesystem with multiple devices)"), - OPTLINE("--device-uuid UUID", "Specify the filesystem device UUID (a.k.a sub-uuid) (for single device filesystem only)"), + OPTLINE("--device-uuid UUID", "specify the filesystem device UUID (a.k.a sub-uuid) (for single device filesystem only)"), "", "Creation:", OPTLINE("-b|--byte-count SIZE", "set size of each device to SIZE (filesystem size is sum of all device sizes)"), - OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory"), + OPTLINE("-r|--rootdir DIR", "copy files from DIR to the image root directory, can be combined with --subvol"), OPTLINE("--compress ALGO[:LEVEL]", "compress files by algorithm and level, ALGO can be 'no' (default), zstd, lzo, zlib"), OPTLINE("", "Built-in:"), #if COMPRESSION_ZSTD @@ -455,6 +455,11 @@ static const char * const mkfs_usage[] = { #endif OPTLINE("", "- ZLIB: yes (levels 1..9)"), OPTLINE("-u|--subvol TYPE:SUBDIR", "create SUBDIR as subvolume rather than normal directory, can be specified multiple times"), + OPTLINE("", "TYPE is one of:"), + OPTLINE("", "- rw - (default) create a writeable subvolume in SUBDIR"), + OPTLINE("", "- ro - create the subvolume as read-only"), + OPTLINE("", "- default - the SUBDIR will be a subvolume and also set as default (can be specified only once)"), + OPTLINE("", "- defalut-ro - like 'default' and is created as read-only subvolume (can be specified only once)"), OPTLINE("--shrink", "(with --rootdir) shrink the filled filesystem to minimal size"), OPTLINE("-K|--nodiscard", "do not perform whole device TRIM"), OPTLINE("-f|--force", "force overwrite of existing filesystem"), From 8c809fdb7cb0390e14bc36b09962d8eb80a69079 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 23 Jan 2025 02:20:25 +0100 Subject: [PATCH 15/23] btrfs-progs: docs: mention CONFIG_BTRFS_EXPERIMENTAL if relevant The new option for experimental featues was added in 6.12, mention it where the DEBUG option was previously used for the same purpose. [ci skip] Signed-off-by: David Sterba --- Documentation/dev/Development-notes.rst | 1 + Documentation/mkfs.btrfs.rst | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Documentation/dev/Development-notes.rst b/Documentation/dev/Development-notes.rst index a0e7b2762..76c492cae 100644 --- a/Documentation/dev/Development-notes.rst +++ b/Documentation/dev/Development-notes.rst @@ -413,6 +413,7 @@ Please refer to the option documentation for further details. - **CONFIG_BTRFS_DEBUG** - **CONFIG_BTRFS_ASSERT** + - **CONFIG_BTRFS_EXPERIMENTAL** - **CONFIG_BTRFS_FS_RUN_SANITY_TESTS** -- basic tests on module load - **CONFIG_BTRFS_FS_CHECK_INTEGRITY** -- block integrity checker enabled by mount options diff --git a/Documentation/mkfs.btrfs.rst b/Documentation/mkfs.btrfs.rst index 39b7d51a5..e3bcd4f25 100644 --- a/Documentation/mkfs.btrfs.rst +++ b/Documentation/mkfs.btrfs.rst @@ -393,7 +393,7 @@ block-group-tree .. _mkfs-feature-raid-stripe-tree: raid-stripe-tree - (kernel support since 6.7, CONFIG_BTRFS_DEBUG) + (kernel support since 6.7, CONFIG_BTRFS_DEBUG/CONFIG_BTRFS_EXPERIMENTAL) Separate tree for logical file extent mapping where the physical mapping may not match on multiple devices. This is now used in zoned mode to @@ -404,8 +404,9 @@ raid-stripe-tree .. note:: Due to the status of implementation it is enabled only in - builds with CONFIG_BTRFS_DEBUG. Support by the kernel module - can be found in the sysfs feature list. + builds with CONFIG_BTRFS_DEBUG/CONFIG_BTRFS_EXPERIMENTAL. + Support by the kernel module can be found in the sysfs feature + list. squota (kernel support since 6.7) From dc4111c28ed7859a55e62dcfa4b7c7106a07bbdc Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Sun, 22 Dec 2024 10:15:57 +1030 Subject: [PATCH 16/23] btrfs-progs: docs: extra notes about read-only scrub on read-write fs [BUG] There is a bug report that read-only scrub on a read-write fs still causes writes into the fs, and that will be caught if there is a read-only block device among the storage stack. This will cause a kernel warning on failed transaction commit: BTRFS info (device dm-3): first mount of filesystem e18f0c40-88de-413f-9d7e-dcc8136ad6dd BTRFS info (device dm-3): using crc32c (crc32c-intel) checksum algorithm BTRFS info (device dm-3): using free-space-tree BTRFS info (device dm-3): scrub: started on devid 1 Trying to write to read-only block-device md127 btrfs_dev_stat_inc_and_print: 362 callbacks suppressed BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 1, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 2, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 3, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 4, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 5, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 6, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 7, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 8, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 9, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device dm-3): bdev /dev/mapper/data errs: wr 10, rd 0, flush 0, corrupt 0, gen 0 BTRFS: error (device dm-3) in btrfs_commit_transaction:2523: errno=-5 IO failure (Error while writing out transaction) BTRFS info (device dm-3 state E): forced readonly BTRFS warning (device dm-3 state E): Skipping commit of aborted transaction. BTRFS error (device dm-3 state EA): Transaction aborted (error -5) BTRFS: error (device dm-3 state EA) in cleanup_transaction:2017: errno=-5 IO failure BTRFS warning (device dm-3 state EA): failed setting block group ro: -5 BTRFS info (device dm-3 state EA): scrub: not finished on devid 1 with status: -5 [CAUSE] The root cause is inside btrfs_inc_block_group_ro(), where we need to hold a transaction handle, to prevent the transaction to be committed, until we hold ro_block_group_mutex. This will cause an empty transaction by itself, thus even if we can mark the block group read-only without any extra workload, we still need to commit the new and empty transaction. Unfortunately this means RO scrub on RW filesystem will always cause the fs to be updated. [FIX] The best fix is to make btrfs to avoid empty commit transaction, but even with that done, read-only scrub on rw mount can still cause real metadata updates (e.g. allocate new chunks and update device error statistics). It will be very complex to make read-only scrub to be fully read-only on a read-write btrfs. Thankfully read-only scrub on read-write mount with read-only device in the storage stack is pretty rare, thus a documentation update should be enough. Issue: #934 Pull-request: #935 Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- Documentation/btrfs-scrub.rst | 5 +++++ Documentation/ch-scrub-intro.rst | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/Documentation/btrfs-scrub.rst b/Documentation/btrfs-scrub.rst index 86c39b20c..be106a551 100644 --- a/Documentation/btrfs-scrub.rst +++ b/Documentation/btrfs-scrub.rst @@ -89,6 +89,11 @@ start [-BdrRf] | -r run in read-only mode, do not attempt to correct anything, can be run on a read-only filesystem + + Note that a read-only scrub on a read-write filesystem can + still cause writes into the filesystem due to some internal + limitations. Only a read-only scrub on a read-only filesystem + can avoid writes from scrub. -R raw print mode, print full data instead of summary -f diff --git a/Documentation/ch-scrub-intro.rst b/Documentation/ch-scrub-intro.rst index 2276bc263..c688cfeac 100644 --- a/Documentation/ch-scrub-intro.rst +++ b/Documentation/ch-scrub-intro.rst @@ -46,6 +46,16 @@ read-write mounted filesystem. used, with expert guidance, to rebuild certain corrupted filesystem structures in the absence of any good replica. +.. note:: + Read-only scrub on a read-write filesystem will cause some writes into the + filesystem. + + This is due to the design limitation to prevent race between marking block + group read-only and writing back block group items. + + To avoid any writes from scrub, one has to run read-only scrub on read-only + filesystem. + The user is supposed to run it manually or via a periodic system service. The recommended period is a month but it could be less. The estimated device bandwidth utilization is about 80% on an idle filesystem. From ee4834f0a3da314b161b972ce7b2ba98066697a9 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 28 Jan 2025 14:56:04 +0100 Subject: [PATCH 17/23] btrfs-progs: qgroup clear-stale: check if qgroup enabled first If qgroups are not enabled the 'clear-stale' command unconditionally calls sync on the whole filesystem, which is unnecessary and can slow down the system. Do a check first if qgroups are enabled. Issue: #942 Signed-off-by: David Sterba --- cmds/qgroup.c | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/cmds/qgroup.c b/cmds/qgroup.c index 570528612..a612130c9 100644 --- a/cmds/qgroup.c +++ b/cmds/qgroup.c @@ -1317,6 +1317,26 @@ static bool key_in_range(const struct btrfs_key *key, return true; } +static int quota_enabled(int fd) { + struct btrfs_tree_search_args args = { 0 }; + struct btrfs_ioctl_search_key *sk; + int ret; + + sk = btrfs_tree_search_sk(&args); + sk->tree_id = BTRFS_QUOTA_TREE_OBJECTID; + sk->min_type = 0; + sk->max_type = (u8)-1; + sk->max_objectid = (u64)-1; + sk->max_offset = (u64)-1; + sk->max_transid = (u64)-1; + sk->nr_items = 1; + + ret = btrfs_tree_search_ioctl(fd, &args); + if (ret < 0) + ret = -errno; + return ret; +} + static int __qgroups_search(int fd, struct btrfs_tree_search_args *args, struct qgroup_lookup *qgroup_lookup) { @@ -2209,6 +2229,18 @@ static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char * if (fd < 0) return 1; + /* Do the check first so the sync is not done if quotas are not enabled. */ + ret = quota_enabled(fd); + if (ret == -ENOENT) { + warning("qgroups not enabled"); + ret = 0; + goto out_close; + } else if (ret < 0) { + errno = -ret; + error("cannot check qgroup status: %m"); + goto out_close; + } + /* Sync the fs so that the qgroup numbers are uptodate. */ err = btrfs_util_sync_fd(fd); if (err) @@ -2217,11 +2249,11 @@ static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char * ret = qgroups_search_all(fd, &qgroup_lookup); if (ret == -ENOTTY) { error("can't list qgroups: quotas not enabled"); - goto out; + goto out_close; } else if (ret < 0) { errno = -ret; error("can't list qgroups: %m"); - goto out; + goto out_close; } node = rb_first(&qgroup_lookup.root); @@ -2235,9 +2267,9 @@ static int cmd_qgroup_clear_stale(const struct cmd_struct *cmd, int argc, char * node = rb_next(node); } -out: - close(fd); __free_all_qgroups(&qgroup_lookup); +out_close: + close(fd); return !!ret; } static DEFINE_SIMPLE_COMMAND(qgroup_clear_stale, "clear-stale"); From ea8d082f0135b7a9b008367de4f5ee554526bb40 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 28 Jan 2025 15:24:21 +0100 Subject: [PATCH 18/23] btrfs-progs: help: recognize --help option among other options There's a report that handling of --help in connection with other option is confusing in some cases: Various sub-commands to btrfs claim --help is an unrecognized option if there are any other options on the CLI. Examples of --help being unrecognized. $ btrfs filesystem defragment -v --help btrfs filesystem defragment: unrecognized option '--help' Try 'btrfs filesystem defragment --help' for more information $ btrfs balance start -v --help $ btrfs balabtrfs balance start -v --help btrfs balance start: unrecognized option '--help' Try 'btrfs balance start --help' for more information Alternatively, some sub-commands support --help even if there are extra options on the CLI: $ btrfs filesystem df -v --help usage: btrfs filesystem df [options] Show space usage information for a mount point -b|--raw raw numbers in bytes -h|--human-readable human friendly numbers, base 1024 (default) -H human friendly numbers, base 1000 --iec use 1024 as a base (KiB, MiB, GiB, TiB) --si use 1000 as a base (kB, MB, GB, TB) -k|--kbytes show sizes in KiB, or kB with --si -m|--mbytes show sizes in MiB, or MB with --si -g|--gbytes show sizes in GiB, or GB with --si -t|--tbytes show sizes in TiB, or TB with --si Global options: --format TYPE where TYPE is: text Update clean_args_no_options() to detect unrecognized options properly and do not show full help text (which usued to be done in the past but is confusing and not pointing to the problem). There's also a mixup of global verbosity options and command-specific verbosity options (now deprecated), so in some commands they are recognized. Also handle the --help option for commands without options so the following works: $ btrfs fi df -v --help / btrfs filesystem df: invalid option 'v' Try 'btrfs filesystem df --help' for more information $ btrfs fi df -H --help / usage: btrfs filesystem df [options] Show space usage information for a mount point -b|--raw raw numbers in byte ... Issue: #889 Signed-off-by: David Sterba --- common/help.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/common/help.c b/common/help.c index 0ee00a3a1..a77b47ed5 100644 --- a/common/help.c +++ b/common/help.c @@ -87,13 +87,14 @@ int check_argc_max(int nargs, int expected) /* * Preprocess @argv with getopt_long to reorder options and consume the "--" * option separator. - * Unknown short and long options are reported, optionally the @usage is printed - * before exit. + * Unknown short and long options are reported. Also consume the --help + * option in case it's for a command without any options. */ void clean_args_no_options(const struct cmd_struct *cmd, int argc, char *argv[]) { static const struct option long_options[] = { - {NULL, 0, NULL, 0} + { "help", no_argument, NULL, GETOPT_VAL_HELP }, + { NULL, 0, NULL, 0 } }; while (1) { @@ -103,9 +104,13 @@ void clean_args_no_options(const struct cmd_struct *cmd, int argc, char *argv[]) break; switch (c) { - default: + case GETOPT_VAL_HELP: if (cmd->usagestr) usage(cmd, 1); + break; + default: + if (cmd->usagestr) + usage_unknown_option(cmd, argv); } } } From 100f8e5d186ee3f2c6c3bf8397fbe201674b57e7 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 28 Jan 2025 15:45:16 +0100 Subject: [PATCH 19/23] btrfs-progs: tests: add cases to combine --help with other options Signed-off-by: David Sterba --- tests/cli-tests/001-btrfs/test.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/cli-tests/001-btrfs/test.sh b/tests/cli-tests/001-btrfs/test.sh index 85213fccf..ef828f80f 100755 --- a/tests/cli-tests/001-btrfs/test.sh +++ b/tests/cli-tests/001-btrfs/test.sh @@ -28,3 +28,17 @@ run_check "$TOP/btrfs" -vvvv help run_check "$TOP/btrfs" --log=quiet help run_check "$TOP/btrfs" -q help run_mustfail "invalid log level accepted" "$TOP/btrfs" --log=invalid help + +# Combine help with other options +run_mustfail "unrecognized option accepted" "$TOP/btrfs" filesystem df -v +run_mustfail "unrecognized option accepted" "$TOP/btrfs" filesystem df -v / +run_mustfail "unrecognized option accepted" "$TOP/btrfs" filesystem df -v --help / +if ! run_check_stdout "$TOP/btrfs" filesystem df --help / | grep -q 'usage.*filesystem df'; then + _fail "standalone option --help" +fi +if ! run_check_stdout "$TOP/btrfs" filesystem df -H --help / | grep -q 'usage.*filesystem df'; then + _fail "option --help with valid option (1)" +fi +if ! run_check_stdout "$TOP/btrfs" filesystem df --help -H / | grep -q 'usage.*filesystem df'; then + _fail "option --help with valid option (2)" +fi From 6eb0e9b3bb47fab9bc5625ce587daa43ac4268ac Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 30 Jan 2025 00:51:06 +0100 Subject: [PATCH 20/23] btrfs-progs: docs: move command line guidelines do dev Signed-off-by: David Sterba --- .../{CmdLineConventions => dev/CmdLineConventions.rst} | 3 ++- Documentation/index.rst | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) rename Documentation/{CmdLineConventions => dev/CmdLineConventions.rst} (96%) diff --git a/Documentation/CmdLineConventions b/Documentation/dev/CmdLineConventions.rst similarity index 96% rename from Documentation/CmdLineConventions rename to Documentation/dev/CmdLineConventions.rst index df6bf42f1..6990e5e5b 100644 --- a/Documentation/CmdLineConventions +++ b/Documentation/dev/CmdLineConventions.rst @@ -1,4 +1,5 @@ -# Command line, formatting, UI guidelines +Command line, formatting, UI guidelines +======================================= ## Command line options diff --git a/Documentation/index.rst b/Documentation/index.rst index f55b664f5..c4ab32e5d 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -84,6 +84,7 @@ is in the :doc:`manual pages`. dev/dev-internal-apis dev/ReleaseChecklist dev/GithubReviewWorkflow + dev/CmdLineConventions btrfs-ioctl From 44bd0f67396488761eef7918319bedd5b36dfa1d Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 30 Jan 2025 02:27:43 +0100 Subject: [PATCH 21/23] btrfs-progs: docs: update command line and UI conventions Signed-off-by: David Sterba --- Documentation/dev/CmdLineConventions.rst | 179 ++++++++++++++++------- 1 file changed, 130 insertions(+), 49 deletions(-) diff --git a/Documentation/dev/CmdLineConventions.rst b/Documentation/dev/CmdLineConventions.rst index 6990e5e5b..72ac875ca 100644 --- a/Documentation/dev/CmdLineConventions.rst +++ b/Documentation/dev/CmdLineConventions.rst @@ -1,67 +1,148 @@ Command line, formatting, UI guidelines ======================================= -## Command line options +The guidelines try to follow common principles and build on experience based on +user feedback or practices seen in other projects. There are no strict rules +but rather vague statements or principles that is recommended to follow. +It's recommended to follow them or use them as review checklist. Optimize for +humans. + +- *sane defaults* +- *principle of least surprise* +- *it does what it says* +- *it says what it does* +- *frequently performed actions have shortcuts* +- *easy thing easy, hard things possible* +- *dangerous actions are explicit or need a confirmation* +- *same name means the same thing everywhere* +- *it's hard to change things once they are released* + +Command line options +-------------------- + +Unless there's a precedent for using a well known short option name, using +long option for first implementation is always safe. + +All options parsers use :manref:`getopt(3)`, the behaviour is known and +consistent with most tools and utilities found elsewhere. Options and parameters +are sorted (options first) up to the ``--`` delimiter. Global options follow right +after the main command and are parsed separately from the subcommand, :command:`btrfs`, +following syntax +:command:`btrfs [global options] subcommand [options] `. + +Short options +^^^^^^^^^^^^^ + +Short options are in short supply, ``a-z`` and ``A-Z``. + +* reserved for the most common operations +* should follow some common scheme + + * verbose (-v), recursive (-r, -R), force (-f), output redirection (-o), ... + * same option means the same thing in a group of related options + * mnemonic naming when possible, e.g. first letter of the action like + *-l* for *length* but with growing number of features clashes can and will + happen + +* *upper case* could mean negating or extending meaning of lower case if it + exists, using both upper and lower case for different purposes can be + confusing +* most short options should have an equivalent long option +* rarely done actions do not need short options at all + +Long options +^^^^^^^^^^^^ + +Long options are meant to be descriptive, e.g. when used in scripts, documentation +or change descriptions. + +* brief but descriptive +* long and descriptive can be used in justified cases (e.g. conversion options + in :doc:`btrfstune` because of the single command without subcommands) + +Option parameters +^^^^^^^^^^^^^^^^^ + +.. note:: + **Avoid using optional parameter.** This is a usability misfeature of + *getopt()* because of the mandatory short option syntax where the parameter + *must* be glued to the option name, like *-czstd* so this looks like a group + of short options but it is not. In this example *-c zstd* is not the same, + the parameter will take default value and *zstd* will be understood as + another parameter. Unfortunate examples are :command:`btrfs filesystem + defrag -c` and :command:`btrfs balance start -d`. Both quite common and we + probably cannot fix this without breaking existing scripts. + +Help text +^^^^^^^^^ + +Description in the help text should be long enough to describe what it does, mention default +value and should not be too long or detailed. Referring to documentation is recommended +when it's really wise to read it first before using it. Otherwise it's a reminder +to user who has probably used it in the past. + +* short help for *--help* output: :command:`btrfs subcommand [options] param1 param2` + + * the number of options gets unwieldy for a command with many tunable features + so it's better to write a short description and document properly everything + in the manual page or in the followup text + +* short description after command line: terse but understandable explanation + what the command does, mention dangers + +* long description after the short description + + * explain in more detail what the command does, different use cases, things to notice + * more complex things should be documented in the manual pages and pointed to + (examples) + +Command output, verbosity +------------------------- + +Structured output +^^^^^^^^^^^^^^^^^ + +If the output consists of a lot of information, try to present it in a concise +way and structure related information together using some known formats +or already used ones in this project. -### Short options - -* reserved for the most common operations -* should follow some common scheme - * verbose, recursive, force, output redirection, ... - * same option means the same thing in a group of related options -* most have an equivalent long option - -### Long options - -* short but descriptive -* long-worded long options are acceptable for rare but seemingly unique operations - * example: `btrfs check --clear-space-cache v1` - -### Help text - -* short help for *--help* output: `btrfs subcommand [options] param1 param2` - * the number of options gets unwieldy for many options so it's better to - insert the stub and document properly all of them in the detailed section -* short description after command line: terse but understandable explanation - what the command does -* long description after the short description - * explain in more detail what the command does, different use cases, things to notice - * more complex things should be documented in the manual pages and pointed to - -## Command output, verbosity - -### Structured output - -* `Key: value` -* indentation used for visual separation +* `Key: value`, spacing by tabs or spaces +* use indentation and empty lines for visual separation * value column alignment for quick skimming -* must be parseable by scripts but primary consumer of the output is a human, and greppable for logs +* must be parseable and also visually comprehensible, related information + on one line usually satisfies both (*greppable*) -### Default output +Default output +^^^^^^^^^^^^^^ -* unix commands do one thing and say nothing, we may diverge a bit from that +* UNIX commands do one thing and say nothing, we diverge from that as it does + not work well for a multi-command tools * default output is one line shortly describing the action - * why: running commands from scripts or among many other commands should be - noticeable due to progress tracking or for analysis if something goes wrong - * there's a global option to make the commands silent `btrfs -q subcommand`, - this can be easily scripted e.g. storing the global verbosity option in a - variable, `btrfs $quiet subcommand` and then enabling or disabling as needed -* line length should not exceed 80 columns if possible but this is not a strict - limit and we want to put the relevant information there rather abbreviate too - much -### Formatting + * why: running commands from scripts or among many other commands should be + noticeable due to progress tracking or for analysis if something goes wrong + * there's a global option to make the commands silent :command:`btrfs -q subcommand`, + this can be easily scripted e.g. storing the global verbosity option in a + variable, :command:`btrfs $quiet subcommand` and then enabling or disabling as needed + +* there should be a line length limit so the output visually fits to one line without + wrapping, there's no exact number of columns, assume something around 100, + keep related information on one line (printed) rather then breaking it. + +Formatting +^^^^^^^^^^ * numeric values are not quoted * string values are quoted if they would be confused when parsed word by word (e.g. file paths) - * quoting is by single apostrophe on both ends + * quoting is by single apostrophe on both ends, no fancy backtick+quote * all messages follow a few known and understood schemes -### Verbosity levels +Verbosity levels +^^^^^^^^^^^^^^^^ -* 0 - quiet, only errors to stderr, nothing to stdout +* 0 - quiet, only errors to *stderr*, nothing to *stdout* * 1 - default, one line, see above * 2 - inform about main steps that happen -* 3 - a little bit more details about what happens during level 2 -* 4 - lots of information, for debugging and close inspection what happens +* 3 - a little bit more detailed about what happens during level 2 +* 4 - possibly lots of information, for debugging and close inspection what happens From e162294e63df2a1ddeb179532a4015856ab5e69c Mon Sep 17 00:00:00 2001 From: David Sterba Date: Thu, 30 Jan 2025 12:17:05 +0100 Subject: [PATCH 22/23] btrfs-progs: device add: update --nodiscard spec to take no argument By mistake in the original commit 992fd231807d12 ("btrfs-progs: add nodiscard option to device add") the --nodiscard could take an optional parameter but this was not intended. This is also not documented so there's little chance somebody would actually use it like that. Signed-off-by: David Sterba --- cmds/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmds/device.c b/cmds/device.c index 65b5fc2c4..3e45a7209 100644 --- a/cmds/device.c +++ b/cmds/device.c @@ -73,7 +73,7 @@ static int cmd_device_add(const struct cmd_struct *cmd, int c; enum { GETOPT_VAL_ENQUEUE = GETOPT_VAL_FIRST }; static const struct option long_options[] = { - { "nodiscard", optional_argument, NULL, 'K'}, + { "nodiscard", no_argument, NULL, 'K' }, { "force", no_argument, NULL, 'f'}, { "enqueue", no_argument, NULL, GETOPT_VAL_ENQUEUE}, { NULL, 0, NULL, 0} From 7f876e94762e5e3dbe8bdca92f2a92e5514531b6 Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Mon, 3 Feb 2025 14:28:28 +1030 Subject: [PATCH 23/23] btrfs-progs: balance: add extra delay if converting with a missing device [BUG] There is a reproducer that can trigger btrfs to flips RO: # mkfs.btrfs -f -mraid1 -draid1 /dev/sdd /dev/sde # mount /dev/sdd /mnt/btrfs # echo 1 > /sys/block/sde/device/delete # btrfs balance start -mconvert=dup -dconvert=single /mnt/btrfs ERROR: error during balancing '.': Input/output error There may be more info in syslog - try dmesg | tail Then btrfs will flip read-only with the following errors: btrfs: attempt to access beyond end of device sde: rw=6145, sector=21696, nr_sectors = 32 limit=0 btrfs: attempt to access beyond end of device sde: rw=6145, sector=21728, nr_sectors = 32 limit=0 btrfs: attempt to access beyond end of device sde: rw=6145, sector=21760, nr_sectors = 32 limit=0 BTRFS error (device sdd): bdev /dev/sde errs: wr 1, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 2, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 3, rd 0, flush 0, corrupt 0, gen 0 BTRFS error (device sdd): bdev /dev/sde errs: wr 3, rd 0, flush 1, corrupt 0, gen 0 btrfs: attempt to access beyond end of device sde: rw=145409, sector=128, nr_sectors = 8 limit=0 BTRFS warning (device sdd): lost super block write due to IO error on /dev/sde (-5) BTRFS error (device sdd): bdev /dev/sde errs: wr 4, rd 0, flush 1, corrupt 0, gen 0 btrfs: attempt to access beyond end of device sde: rw=14337, sector=131072, nr_sectors = 8 limit=0 BTRFS warning (device sdd): lost super block write due to IO error on /dev/sde (-5) BTRFS error (device sdd): bdev /dev/sde errs: wr 5, rd 0, flush 1, corrupt 0, gen 0 BTRFS error (device sdd): error writing primary super block to device 2 BTRFS info (device sdd): balance: start -dconvert=single -mconvert=dup -sconvert=dup BTRFS info (device sdd): relocating block group 1372585984 flags data|raid1 BTRFS error (device sdd): bdev /dev/sde errs: wr 5, rd 0, flush 2, corrupt 0, gen 0 BTRFS warning (device sdd): chunk 2446327808 missing 1 devices, max tolerance is 0 for writable mount BTRFS: error (device sdd) in write_all_supers:4044: errno=-5 IO failure (errors while submitting device barriers.) BTRFS info (device sdd state E): forced readonly BTRFS warning (device sdd state E): Skipping commit of aborted transaction. BTRFS error (device sdd state EA): Transaction aborted (error -5) BTRFS: error (device sdd state EA) in cleanup_transaction:2017: errno=-5 IO failure BTRFS info (device sdd state EA): balance: ended with status: -5 [CAUSE] The root cause is that, deleting devices using sysfs interface normally will trigger the shutdown callback for the fs. But btrfs doesn't handle that callback at all, thus it can not really know that device is no longer avaialble, thus btrfs will still try to do usual read/write on that device. This is fine if the user do nothing, as RAID1 can handle it properly. But if we try to convert to SINGLE/DUP, btrfs will still use that device to allocate new data/metadata chunks. And if a new metadata chunk is allocated to the removed device, all the write will be lost, and trigger the super block write/barrier errors above. [USER SPACE ENHANCEMENT] For now, add extra missing devices check at btrfs-balance command. If there is a missing devices, `btrfs balance` will add a 10 seconds delay and warn the possible dangerous. The root fix is to introduce a failing/removed device detection for btrfs, but that will be a pretty big feature and will take quite some time before landing it upstream. Reported-by: Jeff Siddall Link: https://lore.kernel.org/linux-btrfs/2cb1d81e-12a8-4fb1-b3fc-e7e83d31e059@siddall.name/ Signed-off-by: Qu Wenruo --- cmds/balance.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/cmds/balance.c b/cmds/balance.c index 4c73273df..5c17fbc1a 100644 --- a/cmds/balance.c +++ b/cmds/balance.c @@ -376,6 +376,45 @@ static const char * const cmd_balance_start_usage[] = { NULL }; +/* + * Return 0 if no missing device found for the fs at @mnt. + * Return >0 if there is any missing device for the fs at @mnt. + * Return <0 if we hit other errors during the check. + */ +static int check_missing_devices(const char *mnt) +{ + struct btrfs_ioctl_fs_info_args fs_info_arg; + struct btrfs_ioctl_dev_info_args *dev_info_args = NULL; + bool found_missing = false; + int ret; + + ret = get_fs_info(mnt, &fs_info_arg, &dev_info_args); + if (ret < 0) + return ret; + + for (int i = 0; i < fs_info_arg.num_devices; i++) { + struct btrfs_ioctl_dev_info_args *cur_dev_info; + int fd; + + cur_dev_info = (struct btrfs_ioctl_dev_info_args *)&dev_info_args[i]; + + /* + * Kernel will report the device path even if we can no + * longer access it anymore. So we have to manually check it. + */ + fd = open((char *)cur_dev_info->path, O_RDONLY); + if (fd < 0) { + found_missing = true; + break; + } + close(fd); + } + free(dev_info_args); + if (found_missing) + return 1; + return 0; +} + static int cmd_balance_start(const struct cmd_struct *cmd, int argc, char **argv) { @@ -387,6 +426,7 @@ static int cmd_balance_start(const struct cmd_struct *cmd, bool enqueue = false; unsigned start_flags = 0; bool raid56_warned = false; + bool convert_warned = false; int i; memset(&args, 0, sizeof(args)); @@ -481,6 +521,53 @@ static int cmd_balance_start(const struct cmd_struct *cmd, args.flags |= BTRFS_BALANCE_TYPE_MASK; } + /* + * If we are using convert, and there is a missing/failed device at + * runtime (e.g. mounted then remove a device using sysfs interface), + * btrfs has no way to avoid using that failing/removed device. + * + * In that case converting the profile is very dangerous, e.g. + * converting RAID1 to SINGLE/DUP, and new SINGLE/DUP chunks can + * be allocated to that failing/removed device, and cause the + * fs to flip RO due to failed metadata writes. + * + * Meanwhile the fs may work completely fine due to the extra + * duplication (e.g. all RAID1 profiles). + * + * So here warn if one is trying to convert with missing devices. + */ + for (i = 0; ptrs[i]; i++) { + int delay = 10; + int ret; + + if (!(ptrs[i]->flags & BTRFS_BALANCE_ARGS_CONVERT) || convert_warned) + continue; + + ret = check_missing_devices(argv[optind]); + if (ret < 0) { + errno = -ret; + warning("skipping missing devices check due to failure: %m"); + break; + } + if (ret == 0) + continue; + convert_warned = true; + printf("WARNING:\n\n"); + printf("\tConversion with missing device(s) can be dangerous.\n"); + printf("\tPlease use `btrfs replace` or `btrfs device remove` instead.\n"); + if (force) { + printf("\tSafety timeout skipped due to --force\n\n"); + continue; + } + printf("\tThe operation will continue in %d seconds.\n", delay); + printf("\tUse Ctrl-C to stop.\n"); + while (delay) { + printf("%2d", delay--); + fflush(stdout); + sleep(1); + } + } + /* drange makes sense only when devid is set */ for (i = 0; ptrs[i]; i++) { if ((ptrs[i]->flags & BTRFS_BALANCE_ARGS_DRANGE) &&