| Message ID | 20210914090558.79411-1-nborisov@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Implement progs support for removing received uuid on RW vols | expand |
On 2021/9/14 下午5:05, Nikolay Borisov wrote: > Here's V2 which takes into account Qu's suggestions, namely: > > - Add a helper which contains the common code to clear receive-related data > - Now there is a single patch which impements the check/clear for both orig and > lowmem modes > - Added Reviewed-by from Qu. > > > Nikolay Borisov (8): > btrfs-progs: Add btrfs_is_empty_uuid > btrfs-progs: Remove root argument from btrfs_fixup_low_keys > btrfs-progs: Remove fs_info argument from leaf_data_end > btrfs-progs: Remove root argument from btrfs_truncate_item > btrfs-progs: Add btrfs_uuid_tree_remove > btrfs-progs: Implement helper to remove received information of RW > subvol > btrfs-progs: check: Implement removing received data for RW subvols > btrfs-progs: tests: Add test for received information removal > > check/main.c | 21 +++- > check/mode-common.c | 40 ++++++++ > check/mode-common.h | 1 + > check/mode-lowmem.c | 11 ++- > kernel-shared/ctree.c | 62 +++++------- > kernel-shared/ctree.h | 12 ++- > kernel-shared/dir-item.c | 2 +- > kernel-shared/extent-tree.c | 2 +- > kernel-shared/file-item.c | 4 +- > kernel-shared/inode-item.c | 4 +- > kernel-shared/uuid-tree.c | 93 ++++++++++++++++++ > .../050-subvol-recv-clear/test.raw.xz | Bin 0 -> 493524 bytes The images looks a little large than expected, so that the mailing list is rejected the armored patch. Can we use btrfs-image? As there is no need for special layout which can't be dumped by btrfs-image. Thus using btrfs-image -c9 would save quite some space. Thanks, Qu > 12 files changed, 202 insertions(+), 50 deletions(-) > create mode 100644 tests/fsck-tests/050-subvol-recv-clear/test.raw.xz > > -- > 2.17.1 >
On 2021/9/14 下午5:05, Nikolay Borisov wrote: > Here's V2 which takes into account Qu's suggestions, namely: > > - Add a helper which contains the common code to clear receive-related data > - Now there is a single patch which impements the check/clear for both orig and > lowmem modes > - Added Reviewed-by from Qu. > > > Nikolay Borisov (8): > btrfs-progs: Add btrfs_is_empty_uuid > btrfs-progs: Remove root argument from btrfs_fixup_low_keys > btrfs-progs: Remove fs_info argument from leaf_data_end > btrfs-progs: Remove root argument from btrfs_truncate_item > btrfs-progs: Add btrfs_uuid_tree_remove > btrfs-progs: Implement helper to remove received information of RW > subvol > btrfs-progs: check: Implement removing received data for RW subvols > btrfs-progs: tests: Add test for received information removal > > check/main.c | 21 +++- > check/mode-common.c | 40 ++++++++ > check/mode-common.h | 1 + > check/mode-lowmem.c | 11 ++- > kernel-shared/ctree.c | 62 +++++------- > kernel-shared/ctree.h | 12 ++- > kernel-shared/dir-item.c | 2 +- > kernel-shared/extent-tree.c | 2 +- > kernel-shared/file-item.c | 4 +- > kernel-shared/inode-item.c | 4 +- > kernel-shared/uuid-tree.c | 93 ++++++++++++++++++ > .../050-subvol-recv-clear/test.raw.xz | Bin 0 -> 493524 bytes The image looks a little large than expected, so that the mailing list is rejected the armored patch. Can we use btrfs-image? As there is no need for special layout which can't be dumped by btrfs-image. Thus using btrfs-image -c9 would save quite some space. Thanks, Qu > 12 files changed, 202 insertions(+), 50 deletions(-) > create mode 100644 tests/fsck-tests/050-subvol-recv-clear/test.raw.xz > > -- > 2.17.1 >
On 14.09.21 г. 12:30, Qu Wenruo wrote: > > > On 2021/9/14 下午5:05, Nikolay Borisov wrote: >> Here's V2 which takes into account Qu's suggestions, namely: >> >> - Add a helper which contains the common code to clear receive-related >> data >> - Now there is a single patch which impements the check/clear for both >> orig and >> lowmem modes >> - Added Reviewed-by from Qu. >> >> >> Nikolay Borisov (8): >> btrfs-progs: Add btrfs_is_empty_uuid >> btrfs-progs: Remove root argument from btrfs_fixup_low_keys >> btrfs-progs: Remove fs_info argument from leaf_data_end >> btrfs-progs: Remove root argument from btrfs_truncate_item >> btrfs-progs: Add btrfs_uuid_tree_remove >> btrfs-progs: Implement helper to remove received information of RW >> subvol >> btrfs-progs: check: Implement removing received data for RW subvols >> btrfs-progs: tests: Add test for received information removal >> >> check/main.c | 21 +++- >> check/mode-common.c | 40 ++++++++ >> check/mode-common.h | 1 + >> check/mode-lowmem.c | 11 ++- >> kernel-shared/ctree.c | 62 +++++------- >> kernel-shared/ctree.h | 12 ++- >> kernel-shared/dir-item.c | 2 +- >> kernel-shared/extent-tree.c | 2 +- >> kernel-shared/file-item.c | 4 +- >> kernel-shared/inode-item.c | 4 +- >> kernel-shared/uuid-tree.c | 93 ++++++++++++++++++ >> .../050-subvol-recv-clear/test.raw.xz | Bin 0 -> 493524 bytes > > The image looks a little large than expected, so that the mailing list > is rejected the armored patch. > > Can we use btrfs-image? As there is no need for special layout which > can't be dumped by btrfs-image. > Thus using btrfs-image -c9 would save quite some space. Well I did use a raw image, compressed with xz. But sure, I'll look into this but for the time being will wait to gather more feedback. > > Thanks, > Qu > >> 12 files changed, 202 insertions(+), 50 deletions(-) >> create mode 100644 tests/fsck-tests/050-subvol-recv-clear/test.raw.xz >> >> -- >> 2.17.1 >> >
On Tue, Sep 14, 2021 at 12:05:50PM +0300, Nikolay Borisov wrote: > Here's V2 which takes into account Qu's suggestions, namely: > > - Add a helper which contains the common code to clear receive-related data > - Now there is a single patch which impements the check/clear for both orig and > lowmem modes > - Added Reviewed-by from Qu. > > > Nikolay Borisov (8): > btrfs-progs: Add btrfs_is_empty_uuid > btrfs-progs: Remove root argument from btrfs_fixup_low_keys > btrfs-progs: Remove fs_info argument from leaf_data_end > btrfs-progs: Remove root argument from btrfs_truncate_item > btrfs-progs: Add btrfs_uuid_tree_remove > btrfs-progs: Implement helper to remove received information of RW > subvol > btrfs-progs: check: Implement removing received data for RW subvols > btrfs-progs: tests: Add test for received information removal Patches 2-5 added to devel as they're preparatory and otherwise OK as independent cleanups. Regarding the rw and received_uuid, it's choosing between these options: 1) ro->rw resets received_uuid unconditionally Pros: - safe as it does not lead to unexpected results after incremental send Cons: - unconditional, so it's too easy to break the incremental send usecase, which is the main usecase, if that' for backups breaking the continuity is IMNSHO serious usability problem -- and main reason why I'm personally looking for other options Issuing a warning when the ro status is changed by btrfs-progs is only partially fixing that as the raw ioctl or it's wrapper in libbtrfsutil will happily destroy the received_uuid. There's no log or other information that would make it possible to restore it. Note that received_uuid can be set to any value using the BTRFS_IOC_SET_RECEIVED_SUBVOL ioctl, as long as the subvolume is writable. 2) don't allow ro->rw if received_uuid is set, it must be cleared first As mentioned above, the received_uuid can be changed or reset (setting all zeros), but there's still the condition that the subvolume must be writable. After 'receive' the subvolume is snapshotted, updated from stream, set received_uuid and set rw->ro. Reusing the SET_RECEIVED_SUBVOL can't be used as-is, the subvol would have to be rw first. Which is a chicken-egg problem. The safe steps would be: - (after receive, subvolume is RO) - set it to RW - clear received_uuid - either keep it RW or set RO again This would be the single "clear received_uuid manually" and it would be up to the user to knowingly destroy the continuity of the incremental send. The fix here is that the above steps would have to be atomic from user perspective, inside SET_RECEIVED_SUBVOL, eg. based on flag. And we'd have to add a btrfs-progs command somewhere.
On 21/09/2021 19:51, David Sterba wrote: > On Tue, Sep 14, 2021 at 12:05:50PM +0300, Nikolay Borisov wrote: >> Here's V2 which takes into account Qu's suggestions, namely: >> >> - Add a helper which contains the common code to clear receive-related data >> - Now there is a single patch which impements the check/clear for both orig and >> lowmem modes >> - Added Reviewed-by from Qu. >> >> >> Nikolay Borisov (8): >> btrfs-progs: Add btrfs_is_empty_uuid >> btrfs-progs: Remove root argument from btrfs_fixup_low_keys >> btrfs-progs: Remove fs_info argument from leaf_data_end >> btrfs-progs: Remove root argument from btrfs_truncate_item >> btrfs-progs: Add btrfs_uuid_tree_remove >> btrfs-progs: Implement helper to remove received information of RW >> subvol >> btrfs-progs: check: Implement removing received data for RW subvols >> btrfs-progs: tests: Add test for received information removal > > Patches 2-5 added to devel as they're preparatory and otherwise OK as > independent cleanups. > > Regarding the rw and received_uuid, it's choosing between these options: > > 1) ro->rw resets received_uuid unconditionally > > Pros: > - safe as it does not lead to unexpected results after incremental send > > Cons: > - unconditional, so it's too easy to break the incremental send usecase, > which is the main usecase, if that' for backups breaking the > continuity is IMNSHO serious usability problem -- and main reason why > I'm personally looking for other options > > Issuing a warning when the ro status is changed by btrfs-progs is only > partially fixing that as the raw ioctl or it's wrapper in libbtrfsutil > will happily destroy the received_uuid. There's no log or other > information that would make it possible to restore it. > > Note that received_uuid can be set to any value using the > BTRFS_IOC_SET_RECEIVED_SUBVOL ioctl, as long as the subvolume is > writable. How about... Add a --force option. ro->rw with received_uuid non-zero fails without --force, but displays a message something like: "Subvolume is a received subvolume (received_uuid a0fd481e-ac7f-f14f-8bec-f09e3e096344). Setting the subvolume read-write would remove the received_uuid and prevent the subvolume being referenced in future btrfs receive operations. To avoid this, take a rw snapshot of the subvolume instead of setting it rw. If you are happy to clear the received_uuid, specify the --force option". Don't bother to explain about the BTRFS_IOC_SET_RECEIVED_SUBVOL ioctl but at least the received_uuid has been displayed if the user wants to record it. Maybe even log it in a kernel message - I presume this will be a fairly rare action. > > 2) don't allow ro->rw if received_uuid is set, it must be cleared first > > As mentioned above, the received_uuid can be changed or reset (setting > all zeros), but there's still the condition that the subvolume must be > writable. > > After 'receive' the subvolume is snapshotted, updated from stream, set > received_uuid and set rw->ro. > > Reusing the SET_RECEIVED_SUBVOL can't be used as-is, the subvol would > have to be rw first. Which is a chicken-egg problem. > > The safe steps would be: > > - (after receive, subvolume is RO) > - set it to RW > - clear received_uuid > - either keep it RW or set RO again > > This would be the single "clear received_uuid manually" and it would be > up to the user to knowingly destroy the continuity of the incremental > send. > > The fix here is that the above steps would have to be atomic from user > perspective, inside SET_RECEIVED_SUBVOL, eg. based on flag. And we'd > have to add a btrfs-progs command somewhere. >
Here's V2 which takes into account Qu's suggestions, namely: - Add a helper which contains the common code to clear receive-related data - Now there is a single patch which impements the check/clear for both orig and lowmem modes - Added Reviewed-by from Qu. Nikolay Borisov (8): btrfs-progs: Add btrfs_is_empty_uuid btrfs-progs: Remove root argument from btrfs_fixup_low_keys btrfs-progs: Remove fs_info argument from leaf_data_end btrfs-progs: Remove root argument from btrfs_truncate_item btrfs-progs: Add btrfs_uuid_tree_remove btrfs-progs: Implement helper to remove received information of RW subvol btrfs-progs: check: Implement removing received data for RW subvols btrfs-progs: tests: Add test for received information removal check/main.c | 21 +++- check/mode-common.c | 40 ++++++++ check/mode-common.h | 1 + check/mode-lowmem.c | 11 ++- kernel-shared/ctree.c | 62 +++++------- kernel-shared/ctree.h | 12 ++- kernel-shared/dir-item.c | 2 +- kernel-shared/extent-tree.c | 2 +- kernel-shared/file-item.c | 4 +- kernel-shared/inode-item.c | 4 +- kernel-shared/uuid-tree.c | 93 ++++++++++++++++++ .../050-subvol-recv-clear/test.raw.xz | Bin 0 -> 493524 bytes 12 files changed, 202 insertions(+), 50 deletions(-) create mode 100644 tests/fsck-tests/050-subvol-recv-clear/test.raw.xz -- 2.17.1