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 <[email protected]>
> ---
>  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

Attachment: pgpEFdHkQEgiD.pgp
Description: PGP signature

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
Strace-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/strace-devel

Reply via email to