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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 11 Jan 2012 15:29:56 +0200
Alexander Nezhinsky <alexandern@xxxxxxxxxxxx> wrote:

> 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@xxxxxxxxxxxx>
> ---
>  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);

Can we kill rsp_buf? That is, we can simply write the response to
*FILE?


> +		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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux SCSI]     [Linux RAID]     [Linux Clusters]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]

  Powered by Linux