[stgt] segfault when stopping the target

[FUJITA Tomonori]

On Tue, 21 Oct 2008 16:23:02 +0200
Tomasz Chmielewski <mangoo at wpkg.org> wrote:

> FUJITA Tomonori schrieb:
> 
> >>>> --- SIGSEGV (Segmentation fault) @ 0 (0) ---
> >>>> +++ killed by SIGSEGV +++
> >>>> Process 5131 detached
> >>> This helps?
> >> At least it does not crash any more ;)
> >> But it loops endlessly.
> > 
> > Ah, thanks.
> > 
> > With this patch, tgtadm should fail properly instead of going into
> > endless loop.
> > 
> > But we need to know why tgtd closed tgtadm's socket. Can you try this
> > patch and let me know if you get error messages in the log.
> 
> It does not seem to crash nor loop any more.

Nice.

> Does it say anything?
> 
> Oct 21 16:21:04 megathecus tgtd: conn_close_force(197) close 1 0
> Oct 21 16:21:04 megathecus tgtd: conn_close(88) connection closed 0x80700b4 4
> Oct 21 16:21:04 megathecus tgtd: tgt_event_modify(159) Cannot find event 7

Can you try this new patch and send messages in the log?

I expect error messages like:

tgtd: mtask_handler(x) hoge
tgtd: mgmt_event_handler(x) hoge


If you can't get such message after trying several times, then it's
fine. At least, tgtadm doesn't crash any more. It's much better than
the current situation though I'm not sure why tgtd closes tgtadm
socket.

Thanks,


diff --git a/usr/mgmt.c b/usr/mgmt.c
index f6141cb..f58f880 100644
--- a/usr/mgmt.c
+++ b/usr/mgmt.c
@@ -413,7 +413,9 @@ static void mtask_handler(int fd, int events, void *data)
 					tgt_mgmt(mtask);
 					mtask->mtask_state =
 						MTASK_STATE_RSP_SEND;
-					tgt_event_modify(fd, EPOLLOUT);
+					if (tgt_event_modify(fd, EPOLLOUT))
+						eprintf("failed to modify\n");
+
 					mtask->done = 0;
 				} else {
 					/* the pdu exists */
@@ -440,7 +442,9 @@ static void mtask_handler(int fd, int events, void *data)
 			if (mtask->done == req->len - (sizeof(*req))) {
 				tgt_mgmt(mtask);
 				mtask->mtask_state = MTASK_STATE_RSP_SEND;
-				tgt_event_modify(fd, EPOLLOUT);
+				if (tgt_event_modify(fd, EPOLLOUT))
+					eprintf("failed to modify\n");
+
 				mtask->done = 0;
 			}
 		} else
@@ -493,16 +497,23 @@ static void mgmt_event_handler(int accept_fd, int events, void *data)
 	struct mgmt_task *mtask;
 
 	fd = ipc_accept(accept_fd);
-	if (fd < 0)
+	if (fd < 0) {
+		eprintf("failed to accept a socket\n");
 		return;
+	} else
+		eprintf("new ipc connection %d\n", fd);
 
 	err = ipc_perm(fd);
-	if (err < 0)
+	if (err < 0) {
+		eprintf("permission error\n");
 		goto out;
+	}
 
 	err = set_non_blocking(fd);
-	if (err)
+	if (err) {
+		eprintf("failed to set a socket non-blocking\n");
 		goto out;
+	}
 
 	mtask = zalloc(sizeof(*mtask));
 	if (!mtask) {
@@ -512,6 +523,7 @@ static void mgmt_event_handler(int accept_fd, int events, void *data)
 
 	mtask->buf = zalloc(BUFSIZE);
 	if (!mtask->buf) {
+		eprintf("can't allocate mtask buffer\n");
 		free(mtask);
 		goto out;
 	}
@@ -520,6 +532,7 @@ static void mgmt_event_handler(int accept_fd, int events, void *data)
 	mtask->mtask_state = MTASK_STATE_HDR_RECV;
 	err = tgt_event_add(fd, EPOLLIN, mtask_handler, mtask);
 	if (err) {
+		eprintf("failed to add a socket to epoll %d\n", fd);
 		free(mtask->buf);
 		free(mtask);
 		goto out;
diff --git a/usr/tgtadm.c b/usr/tgtadm.c
index 23dbc53..9133e92 100644
--- a/usr/tgtadm.c
+++ b/usr/tgtadm.c
@@ -199,10 +199,23 @@ static int ipc_mgmt_rsp(int fd)
 	struct tgtadm_rsp rsp;
 	int err, rest, len;
 
-	err = read(fd, &rsp, sizeof(rsp));
+retry:
+	err = recv(fd, &rsp, sizeof(rsp), MSG_WAITALL);
 	if (err < 0) {
-		eprintf("can't get the response, %m\n");
+		if (errno == EAGAIN)
+			goto retry;
+		else if (errno == EINTR)
+			eprintf("interrupted by a signal\n");
+		else
+			eprintf("can't get the response, %m\n");
+
 		return errno;
+	} else if (err == 0) {
+		eprintf("tgtd closed the socket\n");
+		return 0;
+	} else if (err != sizeof(rsp)) {
+		eprintf("a partial response\n");
+		return 0;
 	}
 
 	if (rsp.err != TGTADM_SUCCESS) {
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html