kdave / btrfs-progs

Development of userspace BTRFS tools
GNU General Public License v2.0
530 stars 240 forks source link

btrfs-progs: receive: cannot find clone source subvol when receiving in reverse direction #643

Closed ettavolt closed 1 month ago

ettavolt commented 1 year ago

process_clone, unlike process_snapshot, only searched a subvolume matching by received uuid. However, when earlier "receiver" side sends, it mentions received uuid, which is for earlier "send" side (now "receiver" side) is just uuid.

Fixes #606.

ettavolt commented 1 year ago

New test before the change: test-fail.log and after: test-pass.log.

Note, I've collected these with btrfs -v receive, which is not set in what I've pushed.

ettavolt commented 8 months ago

Looks like ubuntu-latest is 22.04, not even 23.04, or dd would have known oseek.

I've changed the test to old parameter name.

ettavolt commented 8 months ago

Why did it decide to fail in 028-superblock-recover now, but not during previous run? :-O

kdave commented 8 months ago

The test 028 randomly fails in the github CI, I don't know why, you don't have to worry about it.

kdave commented 8 months ago

Looks like ubuntu-latest is 22.04, not even 23.04, or dd would have known oseek.

Yeah the CI images are old (kernel and user space), so the tests have to test for new features or not use new functionality.

joanbm commented 7 months ago

The first parameter of search_source_subvol should be changed from a struct subvol_uuid_search * to an int, which I suggest naming mnt_fd, otherwise this generates some integer <->pointer conversion warnings.

(Note that there are two different declarations of subvol_uuid_search in btrfs-progs, which probably caused the confusion with the typings).

(Also posted to LKML before I saw that there was also an open PR, sorry for the spam)

ettavolt commented 7 months ago

Thanks, joanbm. To me that looked like a copy-paste error. ☺

ettavolt commented 6 months ago

Sorry, kdave, do I need to do something else to have this merged?

ettavolt commented 4 months ago

Hi @kdave, sorry for the noise, but can this be merged please? Is there anything I can do to improve the patch?

kdave commented 2 months ago

Sorry, I keep forgetting about this PR. Marked for 6.8.1 or 6.9 at the latest.

kdave commented 1 month ago

Now added to devel, thanks. As said in #606 I don't remember if there was a reason to do it as before, some use cases were not meant to be possible and some of them were with reverse receive. I hope nothing else breaks (we had such cases unfortunatelly).

kdave commented 1 month ago

Minor changes to the test case: indentation and parameter quoting.