Re: [PATCH v2][linux 2.6.18] scsiback: free resources after error

[Juergen Gross <jgross@suse.com>]

On 07/04/2014 11:40 AM, Jan Beulich wrote:
> From: Juergen Gross <jgross@suse.com>
>
> In case of an error during preparing an I/O the already allocated resources
> should all be freed again and the frontend request should be terminated
> accordingly.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
>
> Some formatting adjustments to the changes above, and some more releasing
> of resources (in the VSCSIIF_ACT_SCSI_RESET and default cases of the switch
> in scsiback_do_cmd_fn()).

For your additional changes:

Reviewed-by: Juergen Gross <jgross@suse.com>

>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/drivers/xen/scsiback/common.h
> +++ b/drivers/xen/scsiback/common.h
> @@ -163,13 +163,13 @@ struct scsi_device *scsiback_do_translat
>   void scsiback_release_translation_entry(struct vscsibk_info *info);
>
>
> -void scsiback_cmd_exec(pending_req_t *pending_req);
> +int scsiback_cmd_exec(pending_req_t *pending_req);
>   void scsiback_do_resp_with_sense(char *sense_buffer, int32_t result,
>   			uint32_t resid, pending_req_t *pending_req);
>   void scsiback_fast_flush_area(pending_req_t *req);
>
>   void scsiback_rsp_emulation(pending_req_t *pending_req);
> -void scsiback_req_emulation_or_cmdexec(pending_req_t *pending_req);
> +int scsiback_req_emulation_or_cmdexec(pending_req_t *pending_req);
>   void scsiback_emulation_init(void);
>
>
> --- a/drivers/xen/scsiback/emulate.c
> +++ b/drivers/xen/scsiback/emulate.c
> @@ -345,16 +345,16 @@ void scsiback_rsp_emulation(pending_req_
>   }
>
>
> -void scsiback_req_emulation_or_cmdexec(pending_req_t *pending_req)
> +int scsiback_req_emulation_or_cmdexec(pending_req_t *pending_req)
>   {
> -	if (__pre_do_emulation(pending_req, NULL)) {
> -		scsiback_cmd_exec(pending_req);
> -	}
> -	else {
> -		scsiback_fast_flush_area(pending_req);
> -		scsiback_do_resp_with_sense(pending_req->sense_buffer,
> -		  pending_req->rslt, pending_req->resid, pending_req);
> -	}
> +	if (__pre_do_emulation(pending_req, NULL))
> +		return scsiback_cmd_exec(pending_req);
> +
> +	scsiback_fast_flush_area(pending_req);
> +	scsiback_do_resp_with_sense(pending_req->sense_buffer,
> +				    pending_req->rslt, pending_req->resid,
> +				    pending_req);
> +	return 0;
>   }
>
>
> --- a/drivers/xen/scsiback/scsiback.c
> +++ b/drivers/xen/scsiback/scsiback.c
> @@ -429,14 +429,14 @@ free_bios:
>   }
>
>
> -void scsiback_cmd_exec(pending_req_t *pending_req)
> +int scsiback_cmd_exec(pending_req_t *pending_req)
>   {
>   	int cmd_len  = (int)pending_req->cmd_len;
>   	int data_dir = (int)pending_req->sc_data_direction;
>   	unsigned int nr_segments = (unsigned int)pending_req->nr_segments;
>   	unsigned int timeout;
>   	struct request *rq;
> -	int write;
> +	int write, err;
>
>   	DPRINTK("%s\n",__FUNCTION__);
>
> @@ -463,17 +463,19 @@ void scsiback_cmd_exec(pending_req_t *pe
>   	rq->end_io_data = pending_req;
>
>   	if (nr_segments) {
> -
> -		if (request_map_sg(rq, pending_req, nr_segments)) {
> -			printk(KERN_ERR "scsiback: SG Request Map Error\n");
> -			return;
> +		err = request_map_sg(rq, pending_req, nr_segments);
> +		if (err) {
> +			printk(KERN_ERR "scsiback: SG Request Map error %d\n",
> +			       err);
> +			blk_put_request(rq);
> +			return err;
>   		}
>   	}
>
>   	scsiback_get(pending_req->info);
>   	blk_execute_rq_nowait(rq->q, NULL, rq, 1, scsiback_cmd_done);
>
> -	return ;
> +	return 0;
>   }
>
>
> @@ -615,17 +617,28 @@ static int scsiback_do_cmd_fn(struct vsc
>   		switch (err ?: pending_req->act) {
>   		case VSCSIIF_ACT_SCSI_CDB:
>   			/* The Host mode is through as for Emulation. */
> -			if (info->feature == VSCSI_TYPE_HOST)
> -				scsiback_cmd_exec(pending_req);
> -			else
> -				scsiback_req_emulation_or_cmdexec(pending_req);
> +			if (info->feature == VSCSI_TYPE_HOST ?
> +			    scsiback_cmd_exec(pending_req) :
> +			    scsiback_req_emulation_or_cmdexec(pending_req)) {
> +				scsiback_fast_flush_area(pending_req);
> +				scsiback_do_resp_with_sense(NULL,
> +							    DRIVER_ERROR << 24,
> +							    0, pending_req);
> +			}
>   			break;
>   		case VSCSIIF_ACT_SCSI_RESET:
> +			/* Just for pointlessly specified segments: */
> +			scsiback_fast_flush_area(pending_req);
>   			scsiback_device_reset_exec(pending_req);
>   			break;
>   		default:
> -			if(!err && printk_ratelimit())
> -				printk(KERN_ERR "scsiback: invalid request\n");
> +			if(!err) {
> +				scsiback_fast_flush_area(pending_req);
> +				if (printk_ratelimit())
> +					printk(KERN_ERR
> +					       "scsiback: invalid request %#x\n",
> +					       pending_req->act);
> +			}
>   			scsiback_do_resp_with_sense(NULL, DRIVER_ERROR << 24,
>   						    0, pending_req);
>   			break;
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>