[PATCH v2] scsi: Add bsg support

[Dmitry V. Levin]

On Fri, Jan 23, 2015 at 04:27:53PM +0100, Bart Van Assche wrote:
> The Linux kernel supports two different versions of the SG_IO API,
> namely v3 and v4. This patch adds support for version 4 of this API.
> At least the sg3_utils package supports version 4 of this API. Version
> 4 of this API is used if /dev/bsg/H:C:I:L is used as device name.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche at sandisk.com>
> ---
>  configure.ac            |   1 +
>  scsi.c                  | 248 ++++++++++++++++++++++++++++++++++++++++--------
>  xlat/bsg_protocol.in    |   1 +
>  xlat/bsg_subprotocol.in |   3 +
>  4 files changed, 211 insertions(+), 42 deletions(-)
>  create mode 100644 xlat/bsg_protocol.in
>  create mode 100644 xlat/bsg_subprotocol.in
> 
> diff --git a/configure.ac b/configure.ac
> index a0b4859..112ea1d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -225,6 +225,7 @@ AC_CHECK_HEADERS(m4_normalize([
>  	elf.h
>  	inttypes.h
>  	ioctls.h
> +	linux/bsg.h
>  	linux/falloc.h
>  	linux/perf_event.h
>  	linux/ptrace.h
> diff --git a/scsi.c b/scsi.c
> index daf7252..bd2bbb2 100644
> --- a/scsi.c
> +++ b/scsi.c
> @@ -35,8 +35,15 @@
>  
>  #include "xlat/sg_io_dxfer_direction.h"
>  
> +# ifdef HAVE_LINUX_BSG_H
> +#  include <linux/bsg.h>
> +#  include <sys/uio.h>
> +#  include "xlat/bsg_protocol.h"
> +#  include "xlat/bsg_subprotocol.h"
> +# endif
> +
>  static void
> -print_sg_io_buffer(struct tcb *tcp, unsigned char *addr, const unsigned int len)
> +print_sg_io_buffer(struct tcb *tcp, unsigned long addr, const unsigned int len)
>  {
>  	unsigned char *buf = NULL;
>  	unsigned int allocated, i;
> @@ -45,8 +52,8 @@ print_sg_io_buffer(struct tcb *tcp, unsigned char *addr, const unsigned int len)
>  		return;
>  	allocated = (len > max_strlen) ? max_strlen : len;
>  	if ((buf = malloc(allocated)) == NULL ||
> -	    umoven(tcp, (unsigned long) addr, allocated, (char *) buf) < 0) {
> -		tprintf("%p", addr);
> +	    umoven(tcp, addr, allocated, (char *) buf) < 0) {
> +		tprintf("%#lx", addr);
>  		free(buf);
>  		return;
>  	}
> @@ -58,70 +65,227 @@ print_sg_io_buffer(struct tcb *tcp, unsigned char *addr, const unsigned int len)
>  		tprints(", ...");
>  }
>  
> +/* Print up to @len bytes of an iovec */
>  static void
> -print_sg_io_req(struct tcb *tcp, struct sg_io_hdr *sg_io)
> +print_sg_iovec(struct tcb *tcp, unsigned long iovec_addr,
> +	       const unsigned iovec_count, const unsigned len)
>  {
> -	tprintf("{'%c', ", sg_io->interface_id);
> -	printxval(sg_io_dxfer_direction, sg_io->dxfer_direction,
> +	struct iovec *iov;
> +	unsigned allocated, i, r, s;
> +
> +	if (len == 0)
> +		return;
> +	allocated = iovec_count * sizeof(*iov);
> +	iov = malloc(allocated);
> +	if (iov == NULL)
> +		return;
> +	if (umoven(tcp, iovec_addr, allocated, (void *)iov) < 0) {
> +		tprintf("%#lx", iovec_addr);
> +		goto free;
> +	}
> +	r = len;
> +	if (r > max_strlen)
> +		r = max_strlen;
> +	for (i = 0; i < iovec_count; i++) {
> +		s = iov[i].iov_len;
> +		if (s > r)
> +			s = r;
> +		print_sg_io_buffer(tcp, (unsigned long)iov[i].iov_base, s);
> +		r -= s;
> +	}
> +	if (len > max_strlen)
> +		tprints(", ...");
> +
> +free:
> +	free(iov);
> +}

Looks like linux kernel doesn't support this feature yet:

$ git grep -E 'd(in|out)_iovec_count'
include/uapi/linux/bsg.h:	__u32 dout_iovec_count;	/* [i] 0 -> "flat" dout transfer else
include/uapi/linux/bsg.h:	__u32 din_iovec_count;	/* [i] 0 -> "flat" din transfer */

If we have to implement this, the code is going to be as complicated
as tprint_iov_upto(), so I'd rather made the printstr() call in
tprint_iov_upto() parametrized instead.

> +
> +static void
> +print_sg_io_v3_req(struct tcb *tcp, long arg)
> +{
> +	struct sg_io_hdr sg_io;
> +
> +	if (umove(tcp, arg, &sg_io) < 0) {
> +		tprintf("%#lx", arg);
> +		return;
> +	}

There is a "{'%c'" printed already, extra "%#lx" wouldn't look great.

[...]
> +#ifdef HAVE_LINUX_BSG_H
> +static void
> +print_sg_io_v4_req(struct tcb *tcp, long arg)
> +{
[...]
> +}
> +
> +static void
> +print_sg_io_v4_res(struct tcb *tcp, long arg)
> +{
[...]
> +}
> +#endif
> +
> +static void
> +print_sg_io_req(struct tcb *tcp, uint32_t iid, long arg)
> +{
> +	tprintf("{'%c'", iid);
> +
> +	switch (iid) {
> +	case 'S':
> +		print_sg_io_v3_req(tcp, arg);
> +		break;
> +	case 'Q':
> +		print_sg_io_v4_req(tcp, arg);
> +		break;
> +	}
> +
> +}
> +
> +static void
> +print_sg_io_res(struct tcb *tcp, uint32_t iid, long arg)
> +{
> +	switch (iid) {
> +	case 'S':
> +		print_sg_io_v3_res(tcp, arg);
> +		break;
> +	case 'Q':
> +		print_sg_io_v4_res(tcp, arg);
> +		break;
> +	}
> +
> +	tprintf("}");
>  }

print_sg_io_v4_req() and print_sg_io_v4_res() are defined only in
HAVE_LINUX_BSG_H case.

>  int
>  scsi_ioctl(struct tcb *tcp, const unsigned int code, long arg)
>  {
> +	uint32_t iid;
> +
>  	switch (code) {
>  	case SG_IO:
>  		if (entering(tcp)) {
> -			struct sg_io_hdr sg_io;
> -
> -			if (umove(tcp, arg, &sg_io) < 0)
> +			if (umove(tcp, arg, &iid) < 0)
>  				tprintf(", %#lx", arg);
>  			else {
>  				tprints(", ");
> -				print_sg_io_req(tcp, &sg_io);
> +				print_sg_io_req(tcp, iid, arg);
>  			}
>  		}
>  		if (exiting(tcp)) {
> -			struct sg_io_hdr sg_io;
> -
> -			if (!syserror(tcp) && umove(tcp, arg, &sg_io) >= 0)
> -				print_sg_io_res(tcp, &sg_io);
> +			if (!syserror(tcp) && umove(tcp, arg, &iid) >= 0)
> +				print_sg_io_res(tcp, iid, arg);
>  			else
>  				tprints("}");
>  		}

The original code is not OK here: if umove() call failed, there was no
'{' printed on entering, so there is no need to print '}' on exiting.


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20150123/7aea98a0/attachment.bin>