diff --git a/lib/config/config_settings.h b/lib/config/config_settings.h index 1d9603fb5..2b76526e3 100644 --- a/lib/config/config_settings.h +++ b/lib/config/config_settings.h @@ -1221,14 +1221,15 @@ cfg_array(activation_read_only_volume_list_CFG, "read_only_volume_list", activat "read_only_volume_list = [ \"vg1\", \"vg2/lvol1\", \"@tag1\", \"@*\" ]\n" "#\n") -cfg(activation_mirror_region_size_CFG, "mirror_region_size", activation_CFG_SECTION, 0, CFG_TYPE_INT, DEFAULT_RAID_REGION_SIZE, vsn(1, 0, 0), NULL, vsn(2, 2, 99), + cfg(activation_mirror_region_size_CFG, "mirror_region_size", activation_CFG_SECTION, 0, CFG_TYPE_INT, DEFAULT_RAID_REGION_SIZE, vsn(1, 0, 0), NULL, vsn(2, 2, 99), "This has been replaced by the activation/raid_region_size setting.\n", - "Size in KiB of each copy operation when mirroring.\n") + "Size in KiB of each raid or mirror synchronization region.\n") cfg(activation_raid_region_size_CFG, "raid_region_size", activation_CFG_SECTION, 0, CFG_TYPE_INT, DEFAULT_RAID_REGION_SIZE, vsn(2, 2, 99), NULL, 0, NULL, "Size in KiB of each raid or mirror synchronization region.\n" - "For raid or mirror segment types, this is the amount of data that is\n" - "copied at once when initializing, or moved at once by pvmove.\n") + "The clean/dirty state of data is tracked for each region.\n" + "The value is rounded down to a power of two if necessary, and\n" + "is ignored if it is not a multiple of the machine memory page size.\n") cfg(activation_error_when_full_CFG, "error_when_full", activation_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_BOOL, DEFAULT_ERROR_WHEN_FULL, vsn(2, 2, 115), NULL, 0, NULL, "Return errors if a thin pool runs out of space.\n" diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index bc599deb4..d8f869d7b 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -712,6 +712,7 @@ static int _round_down_pow2(int r) int get_default_region_size(struct cmd_context *cmd) { + int pagesize = lvm_getpagesize(); int region_size = _get_default_region_size(cmd); if (!is_power_of_2(region_size)) { @@ -720,6 +721,12 @@ int get_default_region_size(struct cmd_context *cmd) region_size / 2); } + if (region_size % (pagesize >> SECTOR_SHIFT)) { + region_size = DEFAULT_RAID_REGION_SIZE * 2; + log_verbose("Using default region size %u kiB (multiple of page size).", + region_size / 2); + } + return region_size; } diff --git a/tools/args.h b/tools/args.h index a481bc523..b33df9af2 100644 --- a/tools/args.h +++ b/tools/args.h @@ -1212,10 +1212,8 @@ arg(resizefs_ARG, 'r', "resizefs", 0, 0, 0, /* Not used */ arg(reset_ARG, 'R', "reset", 0, 0, 0, NULL) -arg(regionsize_ARG, 'R', "regionsize", sizemb_VAL, 0, 0, - "A mirror is divided into regions of this size, and the mirror log\n" - "uses this granularity to track which regions are in sync.\n" - "(This can be used with the \"mirror\" type, not the \"raid1\" type.)\n") +arg(regionsize_ARG, 'R', "regionsize", regionsize_VAL, 0, 0, + "Size of each raid or mirror synchronization region.\n") arg(physicalextentsize_ARG, 's', "physicalextentsize", sizemb_VAL, 0, 0, "#vgcreate\n" diff --git a/tools/command-lines.in b/tools/command-lines.in index 6743b7989..1a9a796bf 100644 --- a/tools/command-lines.in +++ b/tools/command-lines.in @@ -317,7 +317,7 @@ RULE: all not LV_thinpool LV_cachepool --- OO_LVCONVERT_RAID: --mirrors SNumber, --stripes_long Number, ---stripesize SizeKB, --regionsize SizeMB, --interval Number +--stripesize SizeKB, --regionsize RegionSize, --interval Number OO_LVCONVERT_POOL: --poolmetadata LV, --poolmetadatasize SizeMB, --poolmetadataspare Bool, --readahead Readahead, --chunksize SizeKB @@ -364,7 +364,7 @@ ID: lvconvert_raid_types DESC: Convert LV to raid1 or mirror, or change number of mirror images. RULE: all not lv_is_locked lv_is_pvmove -lvconvert --regionsize SizeMB LV_raid +lvconvert --regionsize RegionSize LV_raid OO: OO_LVCONVERT ID: lvconvert_change_region_size DESC: Change the region size of an LV. @@ -655,7 +655,7 @@ OO_LVCREATE_POOL: --poolmetadatasize SizeMB, --poolmetadataspare Bool, --chunksi OO_LVCREATE_THIN: --discards Discards, --errorwhenfull Bool OO_LVCREATE_RAID: --mirrors SNumber, --stripes Number, --stripesize SizeKB, ---regionsize SizeMB, --minrecoveryrate SizeKB, --maxrecoveryrate SizeKB +--regionsize RegionSize, --minrecoveryrate SizeKB, --maxrecoveryrate SizeKB --- @@ -712,7 +712,7 @@ DESC: Create a striped LV (infers --type striped). --- lvcreate --type mirror --size SizeMB VG -OO: --mirrors SNumber, --mirrorlog MirrorLog, --regionsize SizeMB, --stripes Number, OO_LVCREATE +OO: --mirrors SNumber, --mirrorlog MirrorLog, --regionsize RegionSize, --stripes Number, OO_LVCREATE OP: PV ... ID: lvcreate_mirror DESC: Create a mirror LV (also see --type raid1). diff --git a/tools/command.c b/tools/command.c index 59c9ea007..68566fafc 100644 --- a/tools/command.c +++ b/tools/command.c @@ -75,6 +75,7 @@ static inline int segtype_arg(struct cmd_context *cmd, struct arg_values *av) { static inline int alloc_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; } static inline int locktype_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; } static inline int readahead_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; } +static inline int regionsize_arg(struct cmd_context *cmd, struct arg_values *av) { return 0; } static inline int vgmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; } static inline int pvmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; } static inline int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av) { return 0; } diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 4d6b1c9b1..502d614e7 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -146,8 +146,6 @@ static int _read_conversion_type(struct cmd_context *cmd, static int _read_params(struct cmd_context *cmd, struct lvconvert_params *lp) { const char *vg_name = NULL; - int region_size; - int pagesize = lvm_getpagesize(); if (!_read_conversion_type(cmd, lp)) return_0; @@ -249,45 +247,10 @@ static int _read_params(struct cmd_context *cmd, struct lvconvert_params *lp) return 0; } - /* - * --regionsize is only valid if converting an LV into a mirror. - * Checked when we know the state of the LV being converted. - */ - if (arg_is_set(cmd, regionsize_ARG)) { - if (arg_sign_value(cmd, regionsize_ARG, SIGN_NONE) == - SIGN_MINUS) { - log_error("Negative regionsize is invalid."); - return 0; - } + if (arg_is_set(cmd, regionsize_ARG)) lp->region_size = arg_uint_value(cmd, regionsize_ARG, 0); - } else { - region_size = get_default_region_size(cmd); - if (region_size < 0) { - log_error("Negative regionsize in " - "configuration file is invalid."); - return 0; - } - lp->region_size = region_size; - } - - if (lp->region_size % (pagesize >> SECTOR_SHIFT)) { - log_error("Region size (%" PRIu32 ") must be " - "a multiple of machine memory " - "page size (%d).", - lp->region_size, pagesize >> SECTOR_SHIFT); - return 0; - } - - if (!is_power_of_2(lp->region_size)) { - log_error("Region size (%" PRIu32 - ") must be a power of 2.", lp->region_size); - return 0; - } - - if (!lp->region_size) { - log_error("Non-zero region size must be supplied."); - return 0; - } + else + lp->region_size = get_default_region_size(cmd); /* FIXME man page says in one place that --type and --mirrors can't be mixed */ if (lp->mirrors_supplied && !lp->mirrors) diff --git a/tools/lvcreate.c b/tools/lvcreate.c index 051258535..cd31c9969 100644 --- a/tools/lvcreate.c +++ b/tools/lvcreate.c @@ -545,7 +545,6 @@ static int _read_raid_params(struct cmd_context *cmd, static int _read_mirror_and_raid_params(struct cmd_context *cmd, struct lvcreate_params *lp) { - int pagesize = lvm_getpagesize(); unsigned max_images; if (seg_is_raid(lp)) { @@ -616,19 +615,6 @@ static int _read_mirror_and_raid_params(struct cmd_context *cmd, return 0; } - if (!is_power_of_2(lp->region_size)) { - log_error("Region size (%" PRIu32 ") must be a power of 2", - lp->region_size); - return 0; - } - - if (lp->region_size % (pagesize >> SECTOR_SHIFT)) { - log_error("Region size (%" PRIu32 ") must be a multiple of " - "machine memory page size (%d)", - lp->region_size, pagesize >> SECTOR_SHIFT); - return 0; - } - if (seg_is_mirror(lp) && !_read_mirror_params(cmd, lp)) return_0; diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index 4fda35606..58d1a443c 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -797,6 +797,45 @@ int readahead_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_va return 1; } +int regionsize_arg(struct cmd_context *cmd, struct arg_values *av) +{ + int pagesize = lvm_getpagesize(); + uint32_t num; + + if (!_size_arg(cmd, av, 2048, 0)) + return 0; + + if (av->sign == SIGN_MINUS) { + log_error("Region size may not be negative."); + return 0; + } + + if (av->ui64_value > UINT32_MAX) { + log_error("Region size is too big (max %u).", UINT32_MAX); + return 0; + } + + num = av->ui_value; + + if (!num) { + log_error("Region size may not be zero."); + return 0; + } + + if (num % (pagesize >> SECTOR_SHIFT)) { + log_error("Region size must be a multiple of machine memory page size (%d bytes).", + pagesize); + return 0; + } + + if (!is_power_of_2(num)) { + log_error("Region size must be a power of 2."); + return 0; + } + + return 1; +} + /* * Non-zero, positive integer, "all", or "unmanaged" */ diff --git a/tools/tools.h b/tools/tools.h index 00476d106..b3858a080 100644 --- a/tools/tools.h +++ b/tools/tools.h @@ -200,6 +200,7 @@ int segtype_arg(struct cmd_context *cmd, struct arg_values *av); int alloc_arg(struct cmd_context *cmd, struct arg_values *av); int locktype_arg(struct cmd_context *cmd, struct arg_values *av); int readahead_arg(struct cmd_context *cmd, struct arg_values *av); +int regionsize_arg(struct cmd_context *cmd, struct arg_values *av); int vgmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av); int pvmetadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av); int metadatacopies_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av); diff --git a/tools/vals.h b/tools/vals.h index 12d315f80..38029aadc 100644 --- a/tools/vals.h +++ b/tools/vals.h @@ -92,6 +92,10 @@ * --size and other option args treat upper/lower letters * the same, all as 1024 SI base. For this reason, we * should avoid suggesting the upper case letters. + * + * FIXME: negative numbers should be automatically rejected + * for anything but int_arg_with_sign(), e.g. + * size_mb_arg() should reject a negative number. */ val(none_VAL, NULL, "None", "ERR") /* unused, for enum value 0 */ @@ -113,6 +117,7 @@ val(discards_VAL, discards_arg, "Discards", "passdown|nopassdown|ignore") val(mirrorlog_VAL, mirrorlog_arg, "MirrorLog", "core|disk") val(sizekb_VAL, size_kb_arg, "SizeKB", "Number[k|unit]") val(sizemb_VAL, size_mb_arg, "SizeMB", "Number[m|unit]") +val(regionsize_VAL, regionsize_arg, "RegionSize", "Number[m|unit]") val(numsigned_VAL, int_arg_with_sign, "SNumber", "[+|-]Number") val(numsignedper_VAL, int_arg_with_sign_and_percent, "SNumberP", "[+|-]Number[%VG|%PVS|%FREE]") val(permission_VAL, permission_arg, "Permission", "rw|r")