[stgt] [PATCH] COMPARE_AND_WRITE: If the compare fails we should not continue to write the data

FUJITA Tomonori fujita.tomonori at lab.ntt.co.jp
Mon Sep 10 03:00:58 CEST 2012


On Thu, 30 Aug 2012 09:09:39 -0700
Ronnie Sahlberg <ronniesahlberg at gmail.com> wrote:

> Fix a bug in COMPARE-AND-WRITE. If the compare fails, we have to abort with a sense code immediately and we should NOT continue to over-write the backing store with the mismatching data.
> 
> IF the data is not matching, we also should compute at which offset the first mismatch in the buffer occurs.
> This is to be reported in the INFORMATION field (bytes 3-7) in the sense buffer in a later patch.
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg at gmail.com>
> ---
>  usr/bs_rdwr.c |   25 +++++++++++++++++++++++--
>  1 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/usr/bs_rdwr.c b/usr/bs_rdwr.c
> index a79dd02..c7d72e6 100644
> --- a/usr/bs_rdwr.c
> +++ b/usr/bs_rdwr.c
> @@ -65,6 +65,7 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
>  	int result = SAM_STAT_GOOD;
>  	uint8_t key;
>  	uint16_t asc;
> +	uint32_t info = 0;
>  	char *tmpbuf;
>  	size_t blocksize;
>  	uint64_t offset = cmd->offset;
> @@ -117,12 +118,32 @@ static void bs_rdwr_request(struct scsi_cmd *cmd)
>  
>  		ret = pread64(fd, tmpbuf, length, offset);
>  
> -		if (ret != length)
> +		if (ret != length) {
>  			set_medium_error(&result, &key, &asc);
> -		else if (memcmp(scsi_get_out_buffer(cmd), tmpbuf, length)) {
> +			free(tmpbuf);
> +			break;
> +		}
> +
> +		if (memcmp(scsi_get_out_buffer(cmd), tmpbuf, length)) {
> +			uint32_t pos = 0;
> +			char *spos = scsi_get_out_buffer(cmd);
> +			char *dpos = tmpbuf;
> +
> +			/* data differed, this is assumed to be 'rare' so use a
> +			   much more expensive byte-by-byte comparasion to find
> +			   out at which offset the data differs.
> +			 */

Can you use Linux kernel comment style?

/*
 * comment
 * comment
 */

> +			while (pos < length) {
> +				if (*spos++ != *dpos++)
> +					break;
> +				pos++;
> +			}

How about the simpler code like this?

for (pos = 0; pos < length && *spos++ == *dposs; pos++)
         ;


> +			info = pos;
>  			result = SAM_STAT_CHECK_CONDITION;
>  			key = MISCOMPARE;
>  			asc = ASC_MISCOMPARE_DURING_VERIFY_OPERATION;
> +			free(tmpbuf);
> +			break;
>  		}
>  
>  		if (cmd->scb[1] & 0x10)
> -- 
> 1.7.3.1
> 
> --
> 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
--
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