pcd1193182 requested changes on this pull request.
I like the idea overall, just a few questions about details of the
implementation.
> */
- if (sdd->progress) {
We could still have this conditional, just `if(sdd->progress || sdd->siginfo)`
Might be best to keep it so the progress thread is only created if it might be
needed?
> (void) fprintf(stderr, "TIME SENT SNAPSHOT\n");
/*
* Print the progress from ZFS_IOC_SEND_PROGRESS every second.
*/
for (;;) {
- (void) sleep(1);
+
+ /*
+ * If we are doing 'send -v' sleep for 1 second, otherwise
+ * sleep forever, until signal or quitting.
+ */
+ if (!pa->pa_progress) {
+ while (send_progress_thread_signal == 0)
+ (void) sleep(1);
Rather than calling sleep(1) over and over until the variable is set, could we
use a semaphore? That'll reduce overhead from switching to, waking, and
sleeping the thread over and over.
> (void) strlcpy(zc.zc_name, zhp->zfs_name, sizeof (zc.zc_name));
- if (!pa->pa_parsable)
+ if (!pa->pa_parsable && pa->pa_progress)
(void) fprintf(stderr, "TIME SENT SNAPSHOT\n");
Should we print this header the first time we get SIGINFO? That way the columns
will be labelled for the user.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/638#pullrequestreview-160376657
------------------------------------------
openzfs: openzfs-developer
Permalink:
https://openzfs.topicbox.com/groups/developer/T000e5d283b7db7e5-M9ab650352eb3317d34dedba2
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription
