[stgt] [PATCH 3/6] mgmt and concat_buf: using concat_buf in mgmt.c

Alexander Nezhinsky alexandern at mellanox.com
Wed Jan 11 14:29:56 CET 2012


Using concat_buf in mgmt.c. 
The changes influence mainly the SHOW commands (those producing some
output). Upon concat_buf finalization the dynamically alloctaed buffer
is stored directly to the mtask response descriptor. No need to worry
about memory re-allocatin anymore, it is taken care of by concat_buf
mechanism.

Using concat_buf api highlights a problem pre-existing in the mgmt code:
a mix of the regular linux error codes (like ENOMEM) and enum tgtadm_errno
(like TGTADM_NOMEM). Sometimes positive values returned correspond to
tgtadm_errno, sometimes to the number of bytes written.
A new function errno2tgtadm() converts regular errno to tgtadm_errno.
A separate patch will address this problem in a more fundamental manner.

Signed-off-by: Alexander Nezhinsky <alexandern at mellanox.com>
---
 usr/mgmt.c |  216 +++++++++++++++++++++++++++++-------------------------------
 1 files changed, 105 insertions(+), 111 deletions(-)

diff --git a/usr/mgmt.c b/usr/mgmt.c
index 4397850..819afe2 100644
--- a/usr/mgmt.c
+++ b/usr/mgmt.c
@@ -68,21 +68,33 @@ char mgmt_path[256];
 static struct mgmt_task *mtask_alloc(void);
 static void mtask_free(struct mgmt_task *mtask);
 
-static void set_show_results(struct tgtadm_rsp *rsp, int *err)
+static void set_mtask_result(struct mgmt_task *mtask, int err)
 {
-	if (*err < 0)
-		rsp->err = -*err;
-	else {
-		rsp->err = 0;
-		rsp->len = *err + sizeof(*rsp);
-		*err = 0;
+	mtask->rsp.len = sizeof(mtask->rsp);
+	if (!err) {
+		mtask->rsp.err = 0;
+		mtask->rsp.len += mtask->rsp_bsize;
 	}
+	else if (err < 0)
+		mtask->rsp.err = -err;
+	else
+		mtask->rsp.err = err;
+}
+
+static enum tgtadm_errno errno2tgtadm(int err)
+{
+	if (err >= 0)
+		return TGTADM_SUCCESS;
+	else if (err == -ENOMEM)
+		return TGTADM_NOMEM;
+	else
+		return TGTADM_UNKNOWN_ERR;
 }
 
 static int target_mgmt(int lld_no, struct mgmt_task *mtask)
 {
 	struct tgtadm_req *req = &mtask->req;
-	struct tgtadm_rsp *rsp = &mtask->rsp;
+	struct concat_buf b;
 	int err = TGTADM_INVALID_REQUEST;
 
 	switch (req->op) {
@@ -144,58 +156,52 @@ static int target_mgmt(int lld_no, struct mgmt_task *mtask)
 		break;
 	}
 	case OP_SHOW:
-		if (req->tid < 0) {
-			retry:
-			err = tgt_target_show_all(mtask->rsp_buf, mtask->rsp_bsize);
-			if (err == mtask->rsp_bsize) {
-				char *p;
-				mtask->rsp_bsize <<= 1;
-				p = realloc(mtask->rsp_buf, mtask->rsp_bsize);
-				if (p) {
-					mtask->rsp_buf = p;
-					goto retry;
-				} else {
-					eprintf("out of memory\n");
-					err = TGTADM_NOMEM;
-				}
-			}
-		} else if (tgt_drivers[lld_no]->show)
+	{
+		concat_buf_init(&b, &mtask->rsp_buf, &mtask->rsp_bsize);
+		if (req->tid < 0)
+			err = tgt_target_show_all(&b);
+		else if (tgt_drivers[lld_no]->show)
 			err = tgt_drivers[lld_no]->show(req->mode,
 							req->tid,
 							req->sid,
 							req->cid, req->lun,
-							mtask->rsp_buf, mtask->rsp_bsize);
+							&b);
+		if (!err) {
+			err = concat_buf_finish(&b);
+			err = errno2tgtadm(err);
+		}
+		else
+			concat_buf_finish(&b);
 		break;
+	}
 	default:
 		break;
 	}
 
-	if (req->op == OP_SHOW)
-		set_show_results(rsp, &err);
-	else {
-		rsp->err = err;
-		rsp->len = sizeof(*rsp);
-	}
+	set_mtask_result(mtask, err);
 	return err;
 }
 
-static int portal_mgmt(int lld_no, struct mgmt_task *mtask,
-		       struct tgtadm_req *req,
-		       struct tgtadm_rsp *rsp)
+static int portal_mgmt(int lld_no, struct mgmt_task *mtask)
 {
+	struct tgtadm_req *req = &mtask->req;
+	struct concat_buf b;
 	int err = TGTADM_INVALID_REQUEST;
 
 	switch (req->op) {
 	case OP_SHOW:
 		if (tgt_drivers[lld_no]->show) {
+			concat_buf_init(&b, &mtask->rsp_buf, &mtask->rsp_bsize);
 			err = tgt_drivers[lld_no]->show(req->mode,
 							req->tid, req->sid,
 							req->cid, req->lun,
-							mtask->rsp_buf,
-							mtask->rsp_bsize);
-
-			set_show_results(rsp, &err);
-			return err;
+							&b);
+			if (!err) {
+				err = concat_buf_finish(&b);
+				err = errno2tgtadm(err);
+			}
+			else
+				concat_buf_finish(&b);
 		}
 		break;
 	case OP_NEW:
@@ -208,19 +214,19 @@ static int portal_mgmt(int lld_no, struct mgmt_task *mtask,
 		break;
 	}
 
-	rsp->err = err;
-	rsp->len = sizeof(*rsp);
-
+	set_mtask_result(mtask, err);
 	return err;
 }
 
-static int device_mgmt(int lld_no, struct tgtadm_req *req, char *params,
-		       struct tgtadm_rsp *rsp, int *rlen)
+static int device_mgmt(int lld_no, struct mgmt_task *mtask)
 {
+	struct tgtadm_req *req = &mtask->req;
+	char *params = mtask->req_buf;
 	int err = TGTADM_UNSUPPORTED_OPERATION;
 
 	switch (req->op) {
 	case OP_NEW:
+		eprintf("sz:%d params:%s\n",mtask->req_bsize,params);
 		err = tgt_device_create(req->tid, req->device_type, req->lun,
 					params, 1);
 		break;
@@ -234,18 +240,16 @@ static int device_mgmt(int lld_no, struct tgtadm_req *req, char *params,
 		break;
 	}
 
-	rsp->err = err;
-	rsp->len = sizeof(*rsp);
-
+	set_mtask_result(mtask, err);
 	return err;
 }
 
 static int account_mgmt(int lld_no,  struct mgmt_task *mtask)
 {
 	struct tgtadm_req *req = &mtask->req;
-	struct tgtadm_rsp *rsp = &mtask->rsp;
-	int err = TGTADM_UNSUPPORTED_OPERATION;
 	char *user, *password;
+	struct concat_buf b;
+	int err = TGTADM_UNSUPPORTED_OPERATION;
 
 	switch (req->op) {
 	case OP_NEW:
@@ -275,36 +279,27 @@ static int account_mgmt(int lld_no,  struct mgmt_task *mtask)
 		}
 		break;
 	case OP_SHOW:
-	retry:
-		err = account_show(mtask->rsp_buf, mtask->rsp_bsize);
-		if (err == mtask->rsp_bsize) {
-			char *p;
-			mtask->rsp_bsize <<= 1;
-			p = realloc(mtask->rsp_buf, mtask->rsp_bsize);
-			if (p) {
-				mtask->rsp_buf = p;
-				goto retry;
-			} else
-				err = TGTADM_NOMEM;
+		concat_buf_init(&b, &mtask->rsp_buf, &mtask->rsp_bsize);
+		err = account_show(&b);
+		if (!err) {
+			err = concat_buf_finish(&b);
+			err = errno2tgtadm(err);
 		}
+		else
+			concat_buf_finish(&b);
 		break;
 	default:
 		break;
 	}
 out:
-	if (req->op == OP_SHOW)
-		set_show_results(rsp, &err);
-	else {
-		rsp->err = err;
-		rsp->len = sizeof(*rsp);
-	}
+	set_mtask_result(mtask, err);
 	return err;
 }
 
 static int sys_mgmt(int lld_no, struct mgmt_task *mtask)
 {
 	struct tgtadm_req *req = &mtask->req;
-	struct tgtadm_rsp *rsp = &mtask->rsp;
+	struct concat_buf b;
 	int err = TGTADM_INVALID_REQUEST;
 
 	switch (req->op) {
@@ -325,50 +320,56 @@ static int sys_mgmt(int lld_no, struct mgmt_task *mtask)
 							  req->sid, req->lun,
 							  req->cid, mtask->req_buf);
 
-		rsp->err = err;
-		rsp->len = sizeof(*rsp);
 		break;
 	case OP_SHOW:
-		err = system_show(req->mode, mtask->rsp_buf, mtask->rsp_bsize);
-		if (err >= 0 && tgt_drivers[lld_no]->show) {
-			err += tgt_drivers[lld_no]->show(req->mode,
-							 req->tid, req->sid,
-							 req->cid, req->lun,
-							 mtask->rsp_buf + err,
-							 mtask->rsp_bsize - err);
+		concat_buf_init(&b, &mtask->rsp_buf, &mtask->rsp_bsize);
+		err = system_show(req->mode, &b);
+		if (!err && tgt_drivers[lld_no]->show) {
+			err = tgt_drivers[lld_no]->show(req->mode,
+							req->tid, req->sid,
+							req->cid, req->lun,
+							&b);
 		}
-		set_show_results(rsp, &err);
+		if (!err) {
+			err = concat_buf_finish(&b);
+			err = errno2tgtadm(err);
+		}
+		else
+			concat_buf_finish(&b);
 		break;
 	case OP_DELETE:
 		if (is_system_inactive())
 			err = 0;
-
-		rsp->err = err;
-		rsp->len = sizeof(*rsp);
 		break;
 	default:
 		break;
 	}
 
+	set_mtask_result(mtask, err);
 	return err;
 }
 
-static int connection_mgmt(int lld_no, struct mgmt_task *mtask,
-			   struct tgtadm_req *req,
-			   struct tgtadm_rsp *rsp)
+static int connection_mgmt(int lld_no, struct mgmt_task *mtask)
 {
+	struct tgtadm_req *req = &mtask->req;
+	struct concat_buf b;
 	int err = TGTADM_INVALID_REQUEST;
 
 	switch (req->op) {
 	case OP_SHOW:
 		if (tgt_drivers[lld_no]->show) {
+			concat_buf_init(&b, &mtask->rsp_buf, &mtask->rsp_bsize);
 			err = tgt_drivers[lld_no]->show(req->mode,
 							req->tid, req->sid,
 							req->cid, req->lun,
-							mtask->rsp_buf,
-							mtask->rsp_bsize);
-			set_show_results(rsp, &err);
-			return err;
+							&b);
+			if (!err) {
+				err = concat_buf_finish(&b);
+				err = errno2tgtadm(err);
+			}
+			else
+				concat_buf_finish(&b);
+			break;
 		}
 		break;
 	default:
@@ -377,20 +378,19 @@ static int connection_mgmt(int lld_no, struct mgmt_task *mtask,
 							  req->tid,
 							  req->sid, req->lun,
 							  req->cid, mtask->req_buf);
-		rsp->err = err;
-		rsp->len = sizeof(*rsp);
 		break;
 	}
 
+	set_mtask_result(mtask, err);
 	return err;
 }
 
 static int mtask_execute(struct mgmt_task *mtask)
 {
 	struct tgtadm_req *req = &mtask->req;
-	struct tgtadm_rsp *rsp = &mtask->rsp;
-	int lld_no, err = TGTADM_INVALID_REQUEST;
-	int len;
+	struct concat_buf b;
+	int lld_no;
+	int err = TGTADM_INVALID_REQUEST;
 
 	if (!strlen(req->lld))
 		lld_no = 0;
@@ -402,8 +402,7 @@ static int mtask_execute(struct mgmt_task *mtask)
 			else
 				eprintf("driver %s is in state: %s\n",
 					req->lld, driver_state_name(tgt_drivers[lld_no]));
-			rsp->err = TGTADM_NO_DRIVER;
-			rsp->len = sizeof(*rsp);
+			set_mtask_result(mtask, TGTADM_NO_DRIVER);
 			return 0;
 		}
 	}
@@ -420,30 +419,32 @@ static int mtask_execute(struct mgmt_task *mtask)
 		err = target_mgmt(lld_no, mtask);
 		break;
 	case MODE_PORTAL:
-		err = portal_mgmt(lld_no, mtask, req, rsp);
+		err = portal_mgmt(lld_no, mtask);
 		break;
 	case MODE_DEVICE:
-		err = device_mgmt(lld_no, req, mtask->req_buf, rsp, &len);
+		err = device_mgmt(lld_no, mtask);
 		break;
 	case MODE_ACCOUNT:
 		err = account_mgmt(lld_no, mtask);
 		break;
 	case MODE_CONNECTION:
-		err = connection_mgmt(lld_no, mtask, req, rsp);
+		err = connection_mgmt(lld_no, mtask);
 		break;
 	default:
 		if (req->op == OP_SHOW && tgt_drivers[lld_no]->show) {
+			concat_buf_init(&b, &mtask->req_buf, &mtask->req_bsize);
 			err = tgt_drivers[lld_no]->show(req->mode,
 							req->tid, req->sid,
 							req->cid, req->lun,
-							mtask->rsp_buf,
-							mtask->rsp_bsize);
-
-			set_show_results(rsp, &err);
-		} else {
-			rsp->err = err;
-			rsp->len = sizeof(*rsp);
+							&b);
+			if (!err) {
+				err = concat_buf_finish(&b);
+				err = errno2tgtadm(err);
+			}
+			else
+				concat_buf_finish(&b);
 		}
+		set_mtask_result(mtask, err);
 		break;
 	}
 
@@ -512,13 +513,6 @@ static int mtask_received(struct mgmt_task *mtask, int fd)
 {
 	int err;
 
-	mtask->rsp_buf = zalloc(MAX_MGT_BUFSIZE);
-	if (!mtask->rsp_buf) {
-		eprintf("failed to allocate response data buffer\n");
-		return -ENOMEM;
-	}
-	mtask->rsp_bsize = MAX_MGT_BUFSIZE;
-
 	err = mtask_execute(mtask);
 	if (err) {
 		eprintf("mgmt task processing failed\n");
-- 
1.7.3.2

--
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



More information about the stgt mailing list