On Thu, Apr 04, 2019 at 12:51:30AM +0800, Anand Jain wrote: > Both mainline and misc-5.2 fails the test case that transaction > generation number must be unaltered if the property validation fails. > > For example: > btrfs in dump-super /dev/sdb | grep ^generation; \ > btrfs prop set /btrfs compression lz; sync; \ > btrfs in dump-super /dev/sdb | grep ^generation > generation 53 > ERROR: failed to set compression for /btrfs: Invalid argument > generation 54 > > Which the patch in the mailing list fixes and based on misc-5.2 > [PATCH 4/4 RESEND] btrfs: fix property validate fail should not increment > > But, now I notice that the reason for which the original code failed > is different from the reason for which misc-5.2 failed. Former failed > because the malformed 'lz' was only abled to be caught by handle->apply(), > and in latter we were calling the start_transaction() too early before the > handle->validation() which few of the patches in misc-5.1 did. My bad. > > As per your misc-5.2 branch please drop the following patches as I have > better fixes as in this patch. > > 584aebf26bbf btrfs: merge btrfs_setxattr and do_setxattr > ab54ee38fff7 btrfs: don't create transaction in btrfs_setxattr > 0cbf560f72e1 btrfs: start transaction in btrfs_xattr_handler_set > db0f220e98eb btrfs: start transaction in btrfs_set_acl > 56e9e992c00a btrfs: start transaction in btrfs_set_prop_trans > 3e2299ea6a8b btrfs: refactor btrfs_set_prop and add btrfs_set_prop_trans > 737e67ce0f3f btrfs: merge _btrfs_set_prop helpers > > And please apply the patches as in this set. > > Anand Jain (4): > btrfs: rename __btrfs_set_prop to btrfs_set_prop_trans > btrfs: rename __btrfs_set_acl to do_set_acl > btrfs: fix zstd compression parameter > btrfs: fix vanished compression property after failed set
I'm going to send the two fixes independently to 5.1 as they affect current code. Regarding the increased transaction, I don't want to drop the patches that move transaction around just yet, as they're 100 patches deep in the devel queue. The validation should happen before the transaction, the code currently does a few nested calls so this might need further cleanups but I'd rather go that way than to deleting what's in misc-next and starting again. If deleting the current misc-next patches appears to be a better option in the end, then I'll do that.
