Hi, On Wed, Jan 21, 2015 at 05:04:44PM +0100, Bart Van Assche wrote: > Hello, > > If anyone would like to add SG_IO v4 support in strace, the (very > lightly tested) patch below could be used as a starting point. Please > note that I'm not familiar with the strace code base.
Thanks, see my comments below.
> Best regards,
>
> Bart.
>
> [PATCH] scsi: Add bsg support
>
> 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.
> ---
> scsi.c | 112
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 105 insertions(+), 7 deletions(-)
>
> diff --git a/scsi.c b/scsi.c
> index daf7252..cbc8d7a 100644
> --- a/scsi.c
> +++ b/scsi.c
> @@ -32,6 +32,7 @@
>
> # include <sys/ioctl.h>
> # include <scsi/sg.h>
> +# include <linux/bsg.h>
We usually wrap new linux kernel includes in ifdefs:
#ifdef HAVE_LINUX_BSG_H
# include <linux/bsg.h>
#endif
> #include "xlat/sg_io_dxfer_direction.h"
>
> @@ -59,9 +60,8 @@ print_sg_io_buffer(struct tcb *tcp, unsigned char *addr,
> const unsigned int len)
> }
>
> static void
> -print_sg_io_req(struct tcb *tcp, struct sg_io_hdr *sg_io)
> +print_sg_io_v3_req(struct tcb *tcp, struct sg_io_hdr *sg_io)
> {
> - tprintf("{'%c', ", sg_io->interface_id);
> printxval(sg_io_dxfer_direction, sg_io->dxfer_direction,
> "SG_DXFER_???");
> tprintf(", cmd[%u]=[", sg_io->cmd_len);
> @@ -82,7 +82,7 @@ print_sg_io_req(struct tcb *tcp, struct sg_io_hdr *sg_io)
> }
>
> static void
> -print_sg_io_res(struct tcb *tcp, struct sg_io_hdr *sg_io)
> +print_sg_io_v3_res(struct tcb *tcp, struct sg_io_hdr *sg_io)
> {
> if (sg_io->dxfer_direction == SG_DXFER_FROM_DEV ||
> sg_io->dxfer_direction == SG_DXFER_TO_FROM_DEV) {
> @@ -102,26 +102,124 @@ print_sg_io_res(struct tcb *tcp, struct sg_io_hdr
> *sg_io)
> tprintf("info=%#x}", sg_io->info);
> }
>
> +static void
> +print_sg_io_v4_req(struct tcb *tcp, struct sg_io_v4 *sg_io)
> +{
> + static const struct xlat sgv4_prot[] = {
> + { BSG_PROTOCOL_SCSI, "BSG_PROTOCOL_SCSI" },
> + };
> + static const struct xlat sgv4_subpr[] = {
> + { BSG_SUB_PROTOCOL_SCSI_CMD, "BSG_SUB_PROTOCOL_SCSI_CMD" },
> + { BSG_SUB_PROTOCOL_SCSI_TMF, "BSG_SUB_PROTOCOL_SCSI_TMF" },
> + { BSG_SUB_PROTOCOL_SCSI_TRANSPORT,
> + "BSG_SUB_PROTOCOL_SCSI_TRANSPORT" },
> + };
We no longer add xlat structures directly into .c source files.
Instead, there are xlat/*.in files that are automatically converted in
xlat/*.h files that in turn are included by .c files. You can see
examples in the file you are patching.
> +static void
> +print_sg_io_req(struct tcb *tcp, struct sg_io_hdr *sg_io)
> +{
> + tprintf("{'%c', ", sg_io->interface_id);
> + switch (sg_io->interface_id) {
> + case 'S':
> + print_sg_io_v3_req(tcp, sg_io);
> + break;
> + case 'Q':
> + print_sg_io_v4_req(tcp, (void *)sg_io);
> + break;
> + }
> +}
> +
> +static void
> +print_sg_io_res(struct tcb *tcp, struct sg_io_hdr *sg_io)
> +{
> + switch (sg_io->interface_id) {
> + case 'S':
> + print_sg_io_v3_res(tcp, sg_io);
> + break;
> + case 'Q':
> + print_sg_io_v4_res(tcp, (void *)sg_io);
> + break;
> + }
> +}
It's usually better to pass a pointer to a union rather than to pass
a pointer to a struct and then cast it to another struct.
> +
> int
> scsi_ioctl(struct tcb *tcp, const unsigned int code, long arg)
> {
> switch (code) {
> case SG_IO:
> if (entering(tcp)) {
> - struct sg_io_hdr sg_io;
> + union {
> + struct sg_io_hdr sg_io_v3;
> + struct sg_io_v4 sg_io_v4;
> + } sg_io;
>
> if (umove(tcp, arg, &sg_io) < 0)
> tprintf(", %#lx", arg);
This code is unconditionally trying to fetch sg_io_v4 (which is noticeably
larger than sg_io_v3), so if the structure passed to this syscall is
sg_io_v3, there is a change that this umove call would fail.
I'd probably start with fetching only sg_io_v3, check whether it's
actually sg_io_v3 or sg_io_v4, and fetch remaining bytes if it happened
to be sg_io_v4.
--
ldv
pgpNqOyM9nBnh.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
