send_iterate_snap : doall send without fromsnap

[cedricmaunoury]

doall send without fromsnap were working properly on FreeBSD12. It's not working on FreeBSD13 with OpenZFS.

Motivation and Context

My tool ZFSync (https://github.com/cedricmaunoury/zfsync) is not working on FreeBSD 13 (with OpenZFS because of changes made on libzfs_sendrecv.c, especially on function send_iterate_snap.

Description

 if (sd->doall && sd->fromsnap == NULL && sd->seenfrom == B_FALSE) {
  sd->seenfrom = B_TRUE;
}

send_iterate_snap function is used to create a nvlist with all the snapshots that will be stored in the stream. If you want to send a full stream with all the snapshots of a dataset without its children (so replicate=false), the nvlist built is broken (only the last snapshot appears) which make the recv function fail with an ASSERT (0 == nvlist_lookup_nvlist(lookup, snapname, &snapprops_nvlist)

How Has This Been Tested?

On a FreeBSD13 server with the sources, I applied this patch and then rebuild the ZFS library
root@freebsd13_BETA2:/usr/src/cddl/lib/libzfs # make
root@freebsd13_BETA2:/usr/src/cddl/lib/libzfs # make install
root@freebsd13_BETA2:/usr/src/cddl/lib/libzfs

OS : FreeBSD 13 BETA 2

This patch is here to handle a "race condition" wich is not available from zfs client

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• [x ] I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[cedricmaunoury]

This is my first contribution. It is not perfect at all (sorry for that). I hope this contribution won't be the last one, and I will do better next time (i was not aware of the checklist before posting this PR)

[behlendorf]

Thanks, I'm sure this regressed since we didn't have a test case for it. As part of this change can you also add a simple one to the tests/zfs-tests/tests/functional/rsend/ directory and reference it from the tests/runfiles/common.run file.

nit: please also address the make checkstyle warnings about the comment style.

cc: @problame can you also take a look at this.

[problame]

I added a review comment asking for more assertions to be done in the the .ksh test.

zrepl only uses zfs send -$LZC_FLAGS -i; it re-implements most of the -I and -R functionality itself albeit with live-knowledge about the receiving side. So I'm actually not familiar with the user-space part of send-recv code at all :D

Regardless, at a glance, the fix seems reasonable.

[cedricmaunoury]

Patch pushed. I hope tests are going to work 🤞

I think that it may be interesting to allow users to be able to launch zfs send -I @ @tosnap (with an "empty" fromsnap) to get the same result as the one expected from send_doall in the tests. Feedbacks welcome of course.

[cedricmaunoury]

It seems that I have still a lot to learn 👶
I haven't found my mistake... I've tried to copy what has been done for badsend :(

[cedricmaunoury]

I'm kind of stuck here. @behlendorf @problame : Your previous feedbacks were very helpful :)

I've tried to copy the "badsend" example but something is still going wrong. I didn't find a clue in the logs of the automatic checks to correct my patch.

Thank you in advance

[cedricmaunoury]

/tests-functional-ubuntu (18.04)/

2021-02-19T18:10:26.8188354Z Test: /usr/share/zfs/zfs-tests/tests/functional/rsend/send_invalid (run as root) [00:00] [PASS]
2021-02-19T18:10:28.2080777Z Test: /usr/share/zfs/zfs-tests/tests/functional/rsend/send_doall (run as root) [00:01] [PASS]
2021-02-19T18:10:28.8065106Z Test: /usr/share/zfs/zfs-tests/tests/functional/rsend/cleanup (run as root) [00:00] [PASS]

Everything seems fine on send_doall... I don't know why the functional tests have been cancelled...

[cedricmaunoury]

zfs-tests-functional / tests-functional-ubuntu (20.04) (pull_request) Cancelled after 360m — tests-functional-ubuntu (20.04)
😢

We have never been that close 👍

[cedricmaunoury]

Do you think that my changes make ZFS slower ?

[behlendorf]

Do you think that my changes make ZFS slower?

No I don't think so. But we do seem to have bumped in to the 6 hour test limit for GitHub actions. It's not entirely clear why this test run took longer than usual, but I don't think it needs to hold up this PR.