linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sandeen <sandeen@redhat.com>
To: Eric Sandeen <sandeen@redhat.com>
Cc: linux-btrfs@vger.kernel.org
Subject: [PATCH 14/14 V2] btrfs-progs: Error handling in scrub_progress_cycle() thread
Date: Mon, 04 Mar 2013 16:45:37 -0600	[thread overview]
Message-ID: <51352411.7080601@redhat.com> (raw)
In-Reply-To: <1362436804-16766-15-git-send-email-sandeen@redhat.com>

consolidate error handling to ensure that peer_fd
is closed on error paths.  Add a couple comments
to the error handling after the thread is complete.

Note that scrub_progress_cycle returns negative
errnos on any error.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: collect positive / pthread errors in perr & flip
on return if needed

diff --git a/cmds-scrub.c b/cmds-scrub.c
index f73b3c6..748a329 100644
--- a/cmds-scrub.c
+++ b/cmds-scrub.c
@@ -840,9 +840,11 @@ static void *progress_one_dev(void *ctx)
 	return NULL;
 }
 
+/* nb: returns a negative errno via ERR_PTR */
 static void *scrub_progress_cycle(void *ctx)
 {
 	int ret;
+	int  perr = 0;	/* positive / pthread error returns */
 	int old;
 	int i;
 	char fsid[37];
@@ -867,9 +869,9 @@ static void *scrub_progress_cycle(void *ctx)
 	struct sockaddr_un peer;
 	socklen_t peer_size = sizeof(peer);
 
-	ret = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &old);
-	if (ret)
-		return ERR_PTR(-ret);
+	perr = pthread_setcanceltype(PTHREAD_CANCEL_ASYNCHRONOUS, &old);
+	if (perr)
+		goto out;
 
 	uuid_unparse(spc->fi->fsid, fsid);
 
@@ -890,8 +892,10 @@ static void *scrub_progress_cycle(void *ctx)
 
 	while (1) {
 		ret = poll(&accept_poll_fd, 1, 5 * 1000);
-		if (ret == -1)
-			return ERR_PTR(-errno);
+		if (ret == -1) {
+			ret = -errno;
+			goto out;
+		}
 		if (ret)
 			peer_fd = accept(spc->prg_fd, (struct sockaddr *)&peer,
 					 &peer_size);
@@ -909,42 +913,46 @@ static void *scrub_progress_cycle(void *ctx)
 			if (!sp->ret)
 				continue;
 			if (sp->ioctl_errno != ENOTCONN &&
-			    sp->ioctl_errno != ENODEV)
-				return ERR_PTR(-sp->ioctl_errno);
+			    sp->ioctl_errno != ENODEV) {
+				ret = -sp->ioctl_errno;
+				goto out;
+			}
 			/*
 			 * scrub finished or device removed, check the
 			 * finished flag. if unset, just use the last
 			 * result we got for the current write and go
 			 * on. flag should be set on next cycle, then.
 			 */
-			ret = pthread_mutex_lock(&sp_shared->progress_mutex);
-			if (ret)
-				return ERR_PTR(-ret);
+			perr = pthread_mutex_lock(&sp_shared->progress_mutex);
+			if (perr)
+				goto out;
 			if (!sp_shared->stats.finished) {
-				ret = pthread_mutex_unlock(
+				perr = pthread_mutex_unlock(
 						&sp_shared->progress_mutex);
-				if (ret)
-					return ERR_PTR(-ret);
+				if (perr)
+					goto out;
 				memcpy(sp, sp_last, sizeof(*sp));
 				continue;
 			}
-			ret = pthread_mutex_unlock(&sp_shared->progress_mutex);
-			if (ret)
-				return ERR_PTR(-ret);
+			perr = pthread_mutex_unlock(&sp_shared->progress_mutex);
+			if (perr)
+				goto out;
 			memcpy(sp, sp_shared, sizeof(*sp));
 			memcpy(sp_last, sp_shared, sizeof(*sp));
 		}
 		if (peer_fd != -1) {
 			write_poll_fd.fd = peer_fd;
 			ret = poll(&write_poll_fd, 1, 0);
-			if (ret == -1)
-				return ERR_PTR(-errno);
+			if (ret == -1) {
+				ret = -errno;
+				goto out;
+			}
 			if (ret) {
 				ret = scrub_write_file(
 					peer_fd, fsid,
 					&spc->progress[this * ndev], ndev);
 				if (ret)
-					return ERR_PTR(ret);
+					goto out;
 			}
 			close(peer_fd);
 			peer_fd = -1;
@@ -954,8 +962,14 @@ static void *scrub_progress_cycle(void *ctx)
 		ret = scrub_write_progress(spc->write_mutex, fsid,
 					   &spc->progress[this * ndev], ndev);
 		if (ret)
-			return ERR_PTR(ret);
+			goto out;
 	}
+out:
+	if (peer_fd != -1)
+		close(peer_fd);
+	if (perr)
+		ret = -perr;
+	return ERR_PTR(ret);
 }
 
 static struct scrub_file_record *last_dev_scrub(
@@ -1373,11 +1387,14 @@ static int scrub_start(int argc, char **argv, int resume)
 	ret = pthread_cancel(t_prog);
 	if (!ret)
 		ret = pthread_join(t_prog, &terr);
+
+	/* check for errors from the handling of the progress thread */
 	if (do_print && ret) {
-		fprintf(stderr, "ERROR: progress thead handling failed: %s\n",
+		fprintf(stderr, "ERROR: progress thread handling failed: %s\n",
 			strerror(ret));
 	}
 
+	/* check for errors returned from the progress thread itself */
 	if (do_print && terr && terr != PTHREAD_CANCELED) {
 		fprintf(stderr, "ERROR: recording progress "
 			"failed: %s\n", strerror(-PTR_ERR(terr)));


  parent reply	other threads:[~2013-03-04 22:45 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-04 22:39 [PATCH 00/14] btrfs-progs: more Coverity cleanups Eric Sandeen
2013-03-04 22:02 ` Zach Brown
2013-03-04 22:39 ` [PATCH 01/14] btrfs-progs: close fd on cmd_subvol_list return Eric Sandeen
2013-03-04 22:39 ` [PATCH 02/14] btrfs-progs: close fd on do_convert error returns Eric Sandeen
2013-03-04 22:39 ` [PATCH 03/14] btrfs-progs: free resources on do_rollback " Eric Sandeen
2013-03-04 22:39 ` [PATCH 04/14] btrfs-progs: don't leak fd in get_fs_info Eric Sandeen
2013-03-05 23:41   ` Eric Sandeen
2013-03-08 15:27     ` Eric Sandeen
2013-03-04 22:39 ` [PATCH 05/14] btrfs-progs: free allocated metadump structure on restore failure Eric Sandeen
2013-03-04 22:39 ` [PATCH 06/14] btrfs-progs: check for null string in parse_size Eric Sandeen
2013-03-04 22:39 ` [PATCH 07/14] btrfs-progs: tidy up cmd_snapshot() whitespace & returns Eric Sandeen
2013-03-04 22:39 ` [PATCH 08/14] btrfs-progs: Free resources when returning error from cmd_snapshot() Eric Sandeen
2013-03-04 22:39 ` [PATCH 09/14] btrfs-progs: tidy up cmd_subvol_create() whitespace & returns Eric Sandeen
2013-03-04 22:40 ` [PATCH 10/14] btrfs-progs: Free resources when returning error from cmd_subvol_create() Eric Sandeen
2013-03-04 22:40 ` [PATCH 11/14] btrfs-progs: check return of posix_fadvise Eric Sandeen
2013-03-04 22:40 ` [PATCH 12/14] btrfs-progs: Issue warnings if ioctls fail in sigint handlers Eric Sandeen
2013-03-04 21:57   ` Zach Brown
2013-03-04 22:34     ` Eric Sandeen
2013-03-04 22:35   ` [PATCH 12/14 V2] " Eric Sandeen
2013-03-04 22:40 ` [PATCH 13/14] btrfs-progs: better option/error handling for btrfs-vol Eric Sandeen
2013-03-04 22:40 ` [PATCH 14/14] btrfs-progs: Error handling in scrub_progress_cycle() thread Eric Sandeen
2013-03-04 22:00   ` Zach Brown
2013-03-04 22:34     ` Eric Sandeen
2013-03-04 22:45   ` Eric Sandeen [this message]
2013-03-04 23:12     ` [PATCH 14/14 V2] " Zach Brown
2013-03-04 22:49 ` [PATCH 15/14] btrfs-progs: fix scrub error return from pthread_mutex_lock Eric Sandeen
2013-03-10 15:19 ` [PATCH 00/14] btrfs-progs: more Coverity cleanups David Sterba

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51352411.7080601@redhat.com \
    --to=sandeen@redhat.com \
    --cc=linux-btrfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).