Skip to content

libzfs: sendrecv: send_progress_thread: handle SIGINFO/SIGUSR1 #15113

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 8, 2023

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Jul 26, 2023

Motivation and Context

https://101010.pl/@[email protected]/110731819189629373

Current semantics are unchanged, except no-flag "zfs send" can now receive SIGUSR1/SIGINFO to write the progress like with -v.

Description

The thread is re-driven to use timer_create() with SIGUSR1 and pause(). Then it's dead-easy to just not timer_create() if neither of -vV.

How Has This Been Tested?

Manually by ./zfs send filling/store/nabijaczleweli/store.nabijaczleweli.xyz@2023-06-11-pre-bookworm | { sleep 3; wc -c ; } (with varying -v/-V presence) and while :; do pkill -USR1 zfs; done simultaneously.

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.
  • I have updated the documentation accordingly.
  • I have read the contributing document.
  • I have added tests to cover my changes. – there aren't any verbosity tests anyway, so not a new set
  • I have run the ZFS Test Suite with this change applied. – CI take my hand
  • All commit messages are properly formatted and contain Signed-off-by.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 27, 2023
@behlendorf behlendorf requested a review from pcd1193182 July 27, 2023 00:04
@nabijaczleweli nabijaczleweli force-pushed the zfs-sendrecv-SIGINFO branch 2 times, most recently from 96813ab to 1ca595d Compare July 27, 2023 00:15
POSIX timers target the process, not the thread (as does SIGINFO),
so we need to block it in the main thread which will die if interrupted.

Ref: https://101010.pl/@[email protected]/110731819189629373
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Current semantics are preserved, /except/ no-flag "zfs send" can now
receive SIGUSR1/SIGINFO will write the progress.

Closes: https://101010.pl/@[email protected]/110731819189629373
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
@behlendorf behlendorf requested review from lundman and ahrens August 2, 2023 16:00
Copy link
Contributor

@lundman lundman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this, it was a shame openzfs/openzfs#638 got left behind.

@@ -57,7 +57,7 @@ libzfs_la_LIBADD = \
libzutil.la \
libuutil.la

libzfs_la_LIBADD += -lm $(LIBCRYPTO_LIBS) $(ZLIB_LIBS) $(LIBFETCH_LIBS) $(LTLIBINTL)
libzfs_la_LIBADD += -lrt -lm $(LIBCRYPTO_LIBS) $(ZLIB_LIBS) $(LIBFETCH_LIBS) $(LTLIBINTL)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will cause issues on macOS and Windows, but they aren't in upstream yet, so you can skip over it. I will add $(LIBCLOCK_GETTIME) if'en those PRs land.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 7, 2023
@behlendorf behlendorf merged commit 683edb3 into openzfs:master Aug 8, 2023
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Aug 25, 2023
POSIX timers target the process, not the thread (as does SIGINFO),
so we need to block it in the main thread which will die if interrupted.

Ref: https://101010.pl/@[email protected]/110731819189629373
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#15113
behlendorf pushed a commit that referenced this pull request Aug 25, 2023
POSIX timers target the process, not the thread (as does SIGINFO),
so we need to block it in the main thread which will die if interrupted.

Ref: https://101010.pl/@[email protected]/110731819189629373
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #15113
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Dec 12, 2023
POSIX timers target the process, not the thread (as does SIGINFO),
so we need to block it in the main thread which will die if interrupted.

Ref: https://101010.pl/@[email protected]/110731819189629373
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Jorgen Lundman <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes openzfs#15113
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants