Possibly missing `-s` to `zfs recv` when using `zrep refresh`

[janvrany]

I'd like to be able to resume (interrupted) recursive ZREP_R=-R ZREP_INC_FLAG=-I ZREP_FORCE=-f zrep refresh.
Reading through the code, it seems to me that invocation of zfs recv in zrep_refresh function is missing -s option
(which has to be passed no matter whether we're resuming or not, IIUC).

If not, how is this supposed to work?

[janvrany]

I was expecting to see something like this in zrep_refresh

From 0a41c5bb50b7082de005a4c672eb48ea28365eb3 Mon Sep 17 00:00:00 2001
From: Jan Vrany <jan@...>
Date: Tue, 20 Sep 2022 13:00:58 +0100
Subject: [PATCH] Fix resuming `zrep refresh`

In order to support resume of interrupted send, `zfs recv` must always
be called with `-s` option. This commit does so for `zrep refresh`.
---
 zrep      | 6 ++++--
 zrep_sync | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/zrep b/zrep
index 2861c02..e5e64ae 100755
--- a/zrep
+++ b/zrep
@@ -2084,6 +2084,7 @@ zrep_sync(){
 zrep_refresh(){
 	typeset srcfs destfs desthost newsnap newseq master
 	typeset force
+	typeset recv_s
 
 	if [[ "$ZREP_FORCE" == "-f" ]] ; then
 		force=-F
@@ -2134,16 +2135,17 @@ zrep_refresh(){
 
 	_debugprint refresh step 2: Pulling $newsnap
 	if [[ "$ZREP_RESUME" != "" ]] ; then
+		recv_s="-s"
 		token=`$ZFSGETVAL receive_resume_token $destfs`
 		if [[ "$token" == "-" ]] ; then token="" ; fi
 	fi
 
 	if [[ "$BBCP" != "" ]] ; then
 		$BBCP -N io "$srchost:$ZREP_PATH _refreshpull $newsnap $token" \
-		  "zfs recv $force $destfs"
+		  "zfs recv $recv_s $force $destfs"
 	else
 		zrep_ssh $srchost "$ZREP_PATH ${ZREP_R} _refreshpull $newsnap $token ${Z_F_OUT}" |
-		  eval ${Z_F_IN} zfs recv $force $destfs
+		  eval ${Z_F_IN} zfs recv $recv_s $force $destfs
 	fi
 	if [[ $? -ne 0 ]] ; then
 		zrep_errquit Unforseen error pulling snapshot $newsnap from $srchost
diff --git a/zrep_sync b/zrep_sync
index a43f8f3..0dc4f75 100644
--- a/zrep_sync
+++ b/zrep_sync
@@ -562,6 +562,7 @@ zrep_sync(){
 zrep_refresh(){
 	typeset srcfs destfs desthost newsnap newseq master
 	typeset force
+	typeset recv_s
 
 	if [[ "$ZREP_FORCE" == "-f" ]] ; then
 		force=-F
@@ -612,16 +613,17 @@ zrep_refresh(){
 
 	_debugprint refresh step 2: Pulling $newsnap
 	if [[ "$ZREP_RESUME" != "" ]] ; then
+		recv_s="-s"
 		token=`$ZFSGETVAL receive_resume_token $destfs`
 		if [[ "$token" == "-" ]] ; then token="" ; fi
 	fi
 
 	if [[ "$BBCP" != "" ]] ; then
 		$BBCP -N io "$srchost:$ZREP_PATH _refreshpull $newsnap $token" \
-		  "zfs recv $force $destfs"
+		  "zfs recv $recv_s $force $destfs"
 	else
 		zrep_ssh $srchost "$ZREP_PATH ${ZREP_R} _refreshpull $newsnap $token ${Z_F_OUT}" |
-		  eval ${Z_F_IN} zfs recv $force $destfs
+		  eval ${Z_F_IN} zfs recv $recv_s $force $destfs
 	fi
 	if [[ $? -ne 0 ]] ; then
 		zrep_errquit Unforseen error pulling snapshot $newsnap from $srchost
-- 
2.35.1

[ppbrown]

I think there was a much earlier ticket, where someone was asking me about recursive sync, and recovery...and after poking around and doing some experiments, we came to the conclusion that recursive sync in zfs is a nasty hairy mess that leads to inconsistency and is best avoided whenever possible :(

…

On Tue, Sep 20, 2022 at 5:12 AM Jan Vraný ***@***.***> wrote: I was expecting to see something like this in zrep_refresh From 0a41c5b Mon Sep 17 00:00:00 2001 From: Jan Vrany ***@***.***> Date: Tue, 20 Sep 2022 13:00:58 +0100 Subject: [PATCH] Fix resuming `zrep refresh` In order to support resume of interrupted send, `zfs recv` must always be called with `-s` option. This commit does so for `zrep refresh`.--- zrep | 6 ++++-- zrep_sync | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/zrep b/zrep index 2861c02..e5e64ae 100755--- a/zrep+++ b/zrep@@ -2084,6 +2084,7 @@ zrep_sync(){ zrep_refresh(){ typeset srcfs destfs desthost newsnap newseq master typeset force+ typeset recv_s if [[ "$ZREP_FORCE" == "-f" ]] ; then force=-F@@ -2134,16 +2135,17 @@ zrep_refresh(){ _debugprint refresh step 2: Pulling $newsnap if [[ "$ZREP_RESUME" != "" ]] ; then+ recv_s="-s" token=`$ZFSGETVAL receive_resume_token $destfs` if [[ "$token" == "-" ]] ; then token="" ; fi fi if [[ "$BBCP" != "" ]] ; then $BBCP -N io "$srchost:$ZREP_PATH _refreshpull $newsnap $token" \- "zfs recv $force $destfs"+ "zfs recv $recv_s $force $destfs" else zrep_ssh $srchost "$ZREP_PATH ${ZREP_R} _refreshpull $newsnap $token ${Z_F_OUT}" |- eval ${Z_F_IN} zfs recv $force $destfs+ eval ${Z_F_IN} zfs recv $recv_s $force $destfs fi if [[ $? -ne 0 ]] ; then zrep_errquit Unforseen error pulling snapshot $newsnap from $srchostdiff --git a/zrep_sync b/zrep_sync index a43f8f3..0dc4f75 100644--- a/zrep_sync+++ b/zrep_sync@@ -562,6 +562,7 @@ zrep_sync(){ zrep_refresh(){ typeset srcfs destfs desthost newsnap newseq master typeset force+ typeset recv_s if [[ "$ZREP_FORCE" == "-f" ]] ; then force=-F@@ -612,16 +613,17 @@ zrep_refresh(){ _debugprint refresh step 2: Pulling $newsnap if [[ "$ZREP_RESUME" != "" ]] ; then+ recv_s="-s" token=`$ZFSGETVAL receive_resume_token $destfs` if [[ "$token" == "-" ]] ; then token="" ; fi fi if [[ "$BBCP" != "" ]] ; then $BBCP -N io "$srchost:$ZREP_PATH _refreshpull $newsnap $token" \- "zfs recv $force $destfs"+ "zfs recv $recv_s $force $destfs" else zrep_ssh $srchost "$ZREP_PATH ${ZREP_R} _refreshpull $newsnap $token ${Z_F_OUT}" |- eval ${Z_F_IN} zfs recv $force $destfs+ eval ${Z_F_IN} zfs recv $recv_s $force $destfs fi if [[ $? -ne 0 ]] ; then zrep_errquit Unforseen error pulling snapshot $newsnap from $srchost-- 2.35.1 — Reply to this email directly, view it on GitHub <#195 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANEV6KDDXSPNJ756TPJVHDV7GS2DANCNFSM6AAAAAAQRAHOKQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[janvrany]

Right, but zfs recv is called without -s regardless of recursive sync or not - at least this is my understanding of the code. ZFS manpage (at least on OpenZFS, do not have Solaris at hand) says:

-s If the receive is interrupted, save the partially received state, rather than delet-
   ing it.  Interruption may be due to...

so it seems to me that -s has to be passed down to zfs recv in order to make resume to work (whether we're resuming or not). And I do not see that in the code - obviously, I may well overlooked it.