+++ This bug was initially created as a clone of Bug #2070777 +++ Description of problem: After volume creation, VDO allows for modification of certain properties like thread counts, etc. Currently there is no way to do this through LVM. Version-Release number of selected component (if applicable): How reproducible: Always Steps to Reproduce: 1.Create a VDO volume with 3 physical threads. 2.Try to change that value to 2. 3. Actual results: No way to do so. Expected results: An lvm command to allow this to happen. Additional info: --- Additional comment from Zdenek Kabelac on 2022-04-11 09:09:37 UTC --- With lvchange we should support these modifiable for already existing vdo pool: vdo_max_discard vdo_block_map_period vdo_block_map_cache_size_mb vdo_ack_threads vdo_bio_threads vdo_bio_rotation vdo_cpu_threads vdo_hash_zone_threads vdo_logical_threads vdo_physical_threads To handle this in some standard lvm2 way - we will introduce support for --vdosettings options (just like with --cachesettings) --- Additional comment from Andy Walsh on 2022-04-13 18:59:32 UTC --- I believe some (all?) of these settings require the volume to be completely stopped and started to take effect. A simple suspend/resume is not sufficient to put these in place. Bruce, can you confirm this? --- Additional comment from Zdenek Kabelac on 2022-04-14 08:42:23 UTC --- Ok - this would be good to know if we have some of those that can be applied by just 'suspend/resume' n some that need full deactivation and activation. lvchange can print info message when various options do take effect - but I'd need to futher enhance code - so the internal API can give proper info to the user about when the change will happen for real. So - all changes always will happen with next activation ? Lvm2 can deactivate and activate VDO LV if it is unused - do we want this ? (or just message such like 'Change will apply with next activation...' is what we want) --- Additional comment from Zdenek Kabelac on 2022-05-25 12:38:03 UTC --- So here is the current list of implemented support: === VDOPOOL Lvchange Offline === ack_threads bio_rotation bio_threads block_map_cache_size_mb block_map_era_length block_map_period // alias for block_map_era_length cpu_threads hash_zone_threads logical_threads max_discard physical_threads === VDOPOOL Lvchange Online === use_compression use_deduplication === VDOPOOL NO Lvchange (only lvcreate/lvconvert) === check_point_frequency index_memory_size_mb minimum_io_size slab_size_mb use_metadata_hints use_sparse_index Supported syntax for --vdosettings option lvcreate --vdosettings 'vdo_cpu_threads=1'.... lvcreate --vdosettings 'cputhreads=1'.... Prefixes 'vdo_' & 'vdo_use_' can be skip - as well as any '_' in names. With upstream patch (man & tests included): https://listman.redhat.com/archives/lvm-devel/2022-May/024180.html
Devel finished with https://listman.redhat.com/archives/lvm-devel/2022-May/024180.html backported to RH8.7 branch.
If vdo_block_map_period is an alias for block_map_era_length, then shouldn't it work with lvs to get the value as well? Why have this alias at all when vdo_block_map_era_length is already a valid interface? [root@hayes-01 ~]# lvs -a -o +vdo_block_map_period vdo_sanity/vdo_pool [...] Unrecognised field: vdo_block_map_period [root@hayes-01 ~]# lvs -a -o +vdo_block_map_era_length vdo_sanity/vdo_pool LV VG Attr LSize Pool Origin Data% Meta% Move Log Cpy%Sync Convert VDOBlockMapEraLength vdo_pool vdo_sanity dwi------- 5.00g 60.03 16380
Why does vdo_block_map_cache_size not report an int like all the other properties listed? [root@hayes-01 ~]# lvs -o vdo_block_map_cache_size vdo_sanity/vdo_lv VDOBlockMapCacheSize 128.00m And, if it reports a non int, why doesn't it take a non int for the alteration cmd? [root@hayes-01 ~]# lvchange --vdosettings vdo_block_map_period=256.00m /dev/vdo_sanity/vdo_lv Parse error at byte 29 (line 1): unexpected token [root@hayes-01 ~]# lvchange --vdosettings vdo_block_map_period=256.00 /dev/vdo_sanity/vdo_lv Invalid argument for VDO setting "vdo_block_map_period". [root@hayes-01 ~]# lvchange --vdosettings vdo_block_map_period=256 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. Please post the devel test results for these options.
Doesn't seem to change anything anyways: [root@hayes-01 ~]# lvs -o vdo_block_map_cache_size vdo_sanity/vdo_lv VDOBlockMapCacheSize 128.00m [root@hayes-01 ~]# lvchange --vdosettings vdo_block_map_period=256 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. [root@hayes-01 ~]# lvs -o vdo_block_map_cache_size vdo_sanity/vdo_lv VDOBlockMapCacheSize 128.00m
For this moment - we only support aliasing for 'vdosettings' input as a syntax sugar (maybe user should be notified about proper lvm2 option name). We may eventually add same kind of similar support for 'lvs' (in some future iteration). Related to comment #7 As with 'size input' for some settings - this setting parser is the same as with 'configuration' parser - so input value have same limitation and user can use 'just' plain integer value with all such settings. I can see this to be a candidatre for RFE to improve some parametric input - but ATM all --config, --cachesettings, --vdosettings, --metadataprofile, --profile have simple parser accepting only simple values without any decimal points or SI unit suffixes. As for comment #7 block_map_period is an alias for block_map_era_length - so not related vdo_block_map_cache_size. So it's correct the vdo_block_map_cache_size stays with 128M size. So to print with lvs: -o+vdo_block_map_era_length So ATM all works as it's been designed for current release.
Still, vdo_block_map_cache_size is listed as one of the properties that is now supported to be changed, and it still appears there is not way to do that. [root@hayes-01 ~]# lvs -o vdo_block_map_cache_size vdo_sanity/vdo_lv VDOBlockMapCacheSize 128.00m [root@hayes-01 ~]# lvchange --vdosettings 'vdo_block_map_cache_size=256.00m' /dev/vdo_sanity/vdo_lv Parse error at byte 33 (line 1): unexpected token [root@hayes-01 ~]# lvchange --vdosettings 'vdo_block_map_cache_size=256' /dev/vdo_sanity/vdo_lv Unknown VDO setting "vdo_block_map_cache_size". [root@hayes-01 ~]# lvs -o vdo_block_map_cache_size vdo_sanity/vdo_lv VDOBlockMapCacheSize 128.00m Sure, "Parse error at byte 33 (line 1): unexpected token" is a bug that can be fixed later and not a part of this, but the fact that the lvchange fails with "Unknown VDO setting "vdo_block_map_cache_size"." implies that this doesn't work as advertised.
vdo_block_map_era_length doesn't appear to work either. Now, I realize im not exactly sure what a valid "vdo_block_map_era_length" alteration would be, but having the lvchange cmd report that it's now "corrupted" is another bug in this feature. [root@hayes-01 ~]# lvs -o vdo_block_map_era_length vdo_sanity/vdo_lv VDOBlockMapEraLength 16380 [root@hayes-01 ~]# lvchange --vdosettings vdo_block_map_era_length=16381 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. LV vdo_sanity/vdo_pool, segment 1 invalid: sets unsupported VDO block map era length for vdo-pool segment. Internal error: LV segments corrupted in vdo_pool.
More issues wrt to the online options use_compression, use_deduplication: 1. I assumed this was a typo, and that you meant vdo_compression and vdo_deduplication since lvs and lvchange both fail if you use use_* or vdo_use_*, but then I saw this in the warning "Invalid argument for VDO setting "vdo_use_compression"." So this appears broken as well. [root@hayes-01 ~]# lvchange --vdosettings vdo_compression=enabled /dev/vdo_sanity/vdo_lv Invalid argument for VDO setting "vdo_use_compression". [root@hayes-01 ~]# lvs -o vdo_deduplication vdo_sanity/vdo_lv VDODeduplication [root@hayes-01 ~]# lvs -o vdo_compression vdo_sanity/vdo_lv VDOCompression 2. Why are we adding and supporting another way to change the compression/dedupe when we already have this easier interface? [root@hayes-01 ~]# lvchange --compression n vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. [root@hayes-01 ~]# lvchange --compression y vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. So now we have to either remember "vdo_compression=1" or "--compression y" depending on which interface we're using. [root@hayes-01 ~]# lvchange --vdosettings vdo_compression=1 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. [root@hayes-01 ~]# lvchange --vdosettings vdo_compression=0 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. [root@hayes-01 ~]# lvchange --vdosettings vdo_compression=1 /dev/vdo_sanity/vdo_lv Logical volume vdo_sanity/vdo_lv changed. [root@hayes-01 ~]# lvs -o vdo_compression vdo_sanity/vdo_lv VDOCompression enabled
Another issue: None of the "VDOPOOL NO Lvchange (only lvcreate/lvconvert)" options appear to work either. [root@hayes-02 ~]# lvcreate --yes --type linear -n vdo_pool -L 5G vdo_sanity Logical volume "vdo_pool" created. [root@hayes-02 ~]# lvconvert --yes --type vdo-pool -n vdo_lv --vdosettings 'vdo_index_memory_size=256' -V 20G vdo_sanity/vdo_pool lvconvert: unrecognized option '--vdosettings' Error during parsing of command line. [root@hayes-02 ~]# lvconvert --yes --type vdo-pool -n vdo_lv --vdosettings 'vdo_check_point_frequency=1' -V 20G vdo_sanity/vdo_pool lvconvert: unrecognized option '--vdosettings' Error during parsing of command line. [root@hayes-02 ~]# lvconvert --yes --type vdo-pool -n vdo_lv -V 20G vdo_sanity/vdo_pool WARNING: Converting logical volume vdo_sanity/vdo_pool to VDO pool volume with formating. THIS WILL DESTROY CONTENT OF LOGICAL VOLUME (filesystem etc.) The VDO volume can address 2 GB in 1 data slab. It can grow to address at most 16 TB of physical storage in 8192 slabs. If a larger maximum size might be needed, use bigger slabs. Logical volume "vdo_lv" created. Converted vdo_sanity/vdo_pool to VDO pool volume and created virtual vdo_sanity/vdo_lv VDO volume. [root@hayes-02 ~]# lvconvert --vdosettings 'vdo_check_point_frequency=1' vdo_sanity/vdo_pool lvconvert: unrecognized option '--vdosettings' Error during parsing of command line. [root@hayes-02 ~]# lvconvert --vdosettings 'vdo_index_memory_size=256' vdo_sanity/vdo_pool lvconvert: unrecognized option '--vdosettings' Error during parsing of command line.
Let's go by comments: Comment #12 Correct name of the setting here is: vdo_block_map_cache_size_mb (can be shortened to blockmapcachesizemb - but suffix mb still needs to be there as is not option - but it could be eventually another sort of syntax sugar added later - not sure if it's worth yet..) Comment #13 era length cannot be changed at all (not being listed as changable) - the reported error message is rather ulgy - current upstream does handle this case in nicer way already: # lvchange --vdosettings 'vdo_check_point_frequency=1' vg/vpool0 Cannot change VDO setting "vdo_check_point_frequency" in existing VDO pool. But this can likely wait for next release (assuming it will hit RH9.1) Comment #14 vdo_use_compression is there just for completeness - --vdosettings is just a 'nicer' way to handle rather ugly '--config ...' interface which has lot of internal drawbacks inside lvm2 code whenever user is using this --config option - so since this compression & deduplication can be already said by lvm.conf or any profiles - it's natural to have it as one of '--vdosettings' option. And yes some of those option happen to have it's own dedicated lvm2 option --compression && --deduplication - as these do have potential to be possibly used with some other targets in the future - but with tons of other vdo setting options - it's possibly more easier to handle these changes with '--vdosettings' option (but never say never - if there would be a big user request to add more of those as direct lvm2 option - it may happen - so far I do more believe users will like usage of some vdo profiles more for various workloads - and upstream made some progress in this direction already. Fact the syntax parses for 'config' settings is less advanced and just want 0/1 instead of 'syntax sugar advanced lvm2 option has been already commented - and if we figure out some 'compatible' parsing mechanism - it might be enhanced in future (but's highly non-trivial task) Comment #15 'vdo_check_point_frequency' is problematic - it's 'gone' from current version of vdoformat tooling - so here lvm2 will need to 'ignore' any changes to this setting other then value 0 - so this is current bug in lvm2 handling that needs a fix. vdo_index_memory_size again requires _mb to be three in the option name -> vdo_index_memory_size_mb should work.
Overlooked another point from Comment #13 - which is now also handle with better more understandable error message in upstream code base: # lvchange --vdosettings vdo_block_map_era_length=16381 vg/lvol0 VDO block map era length 16381 is out of range [1..16380]. So error gives a user better understanding why it's failing - as 16380 is max value for this setting. Failing of lvchange in existing tested code base is correct - just the reported error was not ideal. It will be fixed with next release - unsure if the patches enhancing error message will get into this cycle ATM.
Re: Comment #12 and Comment #15: I filed bugs: https://bugzilla.redhat.com/show_bug.cgi?id=2108227 https://bugzilla.redhat.com/show_bug.cgi?id=2108233
Re: Comment #13: I filed bug: https://bugzilla.redhat.com/show_bug.cgi?id=2108239
Re: Comment #14: I filed bug: https://bugzilla.redhat.com/show_bug.cgi?id=2108254i "vdo_use_compression is there just for completeness" The reality though, is that these don't fully work. Only the original way of setting this (vdo_compression) works, in that it can be viewed and set through the same argument. # vdo_use_compression: can NOT be viewed, can NOT be altered [root@hayes-01 ~]# lvs -o vdo_use_compression vdo_sanity/vdo_lv [...] Unrecognised field: vdo_use_compression [root@hayes-01 ~]# lvchange --vdosettings 'vdo_use_compression = 1' vdo_sanity/vdo_pool Unknown VDO setting "vdo_use_compression". # use_compression: can NOT be viewed, CAN be altered [root@hayes-01 ~]# lvs -o use_compression vdo_sanity/vdo_lv [...] Unrecognised field: use_compression [root@hayes-01 ~]# lvchange --vdosettings 'use_compression = 1' vdo_sanity/vdo_pool Logical volume vdo_sanity/vdo_pool changed. # vdo_compression: CAN be viewed, CAN be altered [root@hayes-01 ~]# lvs -o vdo_compression vdo_sanity/vdo_lv VDOCompression enabled [root@hayes-01 ~]# lvchange --vdosettings 'vdo_compression = 1' vdo_sanity/vdo_pool Logical volume vdo_sanity/vdo_pool changed.
None of the many issue above, alone, are enough to pull this feature from 8.7/9.1, but I think all of them together is enough. I think documenting the release notes for this will be difficult given that many of these work only to set, or only to view, and that only a subset work both ways like advertised in comment #0 "So here is the current list of implemented support:" section. That said, I will go with whatever devel decides with the list of provided caveats.
Yep - some logic can be nontrivial to get at first - so filled bugs will be somehow handled to improve the situation. Just to put some 'other perspective' let's just explain this i.e. the property of VDO is really named 'vdo_block_map_cache_size' - and with the suffix _mb it is used for configuration to make it clearly different from sizes in some other units (typical sector 512b) used in those lvm conf files - so the user does not need to recalculate this large size himself and can use the 'MiB' units as more human readable number. 'lvs' does not 'suffer' from this 'config parser' limitation (aka using only plain integer number) and can express/report the size in any configurable --unit number. But for the completeness & user happiness we will add accompanying 'vdo_..._mb' lvs options showing the size as the integer number in MiB - as it might be in fact usable for scripting - so it's a valid RFE. Similar with i.e. 'vdo_use_compression' - which is then reported by 'vdo_compression' reported state - but it's not a big deal to add more 'aliases' - just 'lvs -o help' might need some more thinking how to make this listing more easily readable. Saying that - I do not see it currently as a high prio issue for 8.7 that makes VDO filled BZ reports will not be all eventually fixed for this release - as the VDO got a missing functionality working - just some 'syntax sugar' completeness is lacking (while some of reported BZ are already fixed in upstream) - so we will see how much it will fit into 8.7 build.
Another issue to add to the list (use_sparse_index): https://bugzilla.redhat.com/show_bug.cgi?id=2108326
Current upstream functionality should be matching documentation from comment 4. (lvs needs to follow documented full names of vdo settings). Depends to which level we want to backport this upgraded functionality to RHEL8 branch - may require larger patch series.
The bar for inclusion into rhel8.9 is quite high and I don't think this qualifies for the final rhel8 release. We can fix this in the rhel9 clone (bug 2070777).