[PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

scsi/scsi.h is exported to userspace, so it should
use __u8 instead of u8 as other userspace-visible headers do.

This fixes the following errors when application includes scsi/scsi.h
generated with make headers_install:
include/scsi/scsi.h:145: error: expected specifier-qualifier-list before ‘u8’
include/scsi/scsi.h: In function ‘scsi_varlen_cdb_length’:
include/scsi/scsi.h:156: error: ‘struct scsi_varlen_cdb_hdr’ has no member named ‘additional_cdb_length’

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/scsi/scsi.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 084478e..dfcfaab 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -142,10 +142,10 @@ struct scsi_cmnd;
 
 /* defined in T10 SCSI Primary Commands-2 (SPC2) */
 struct scsi_varlen_cdb_hdr {
-	u8 opcode;        /* opcode always == VARIABLE_LENGTH_CMD */
-	u8 control;
-	u8 misc[5];
-	u8 additional_cdb_length;         /* total cdb length - 8 */
+	__u8 opcode;        /* opcode always == VARIABLE_LENGTH_CMD */
+	__u8 control;
+	__u8 misc[5];
+	__u8 additional_cdb_length;         /* total cdb length - 8 */
 	__be16 service_action;
 	/* service specific data follows */
 };
-- 
1.6.2.5

[PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

scsi/scsi.h is exported to userspace, so it should
use __u8 instead of u8 as other userspace-visible headers do.

This fixes the following errors when application includes scsi/scsi.h
generated with make headers_install:
include/scsi/scsi.h:145: error: expected specifier-qualifier-list before ‘u8’
include/scsi/scsi.h: In function ‘scsi_varlen_cdb_length’:
include/scsi/scsi.h:156: error: ‘struct scsi_varlen_cdb_hdr’ has no member named ‘additional_cdb_length’

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/scsi/scsi.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index 084478e..dfcfaab 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -142,10 +142,10 @@ struct scsi_cmnd;
 
 /* defined in T10 SCSI Primary Commands-2 (SPC2) */
 struct scsi_varlen_cdb_hdr {
-	u8 opcode;        /* opcode always == VARIABLE_LENGTH_CMD */
-	u8 control;
-	u8 misc[5];
-	u8 additional_cdb_length;         /* total cdb length - 8 */
+	__u8 opcode;        /* opcode always == VARIABLE_LENGTH_CMD */
+	__u8 control;
+	__u8 misc[5];
+	__u8 additional_cdb_length;         /* total cdb length - 8 */
 	__be16 service_action;
 	/* service specific data follows */
 };
-- 
1.6.2.5
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[James Bottomley]

On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
> scsi/scsi.h is exported to userspace, so it should
> use __u8 instead of u8 as other userspace-visible headers do.

Actually, can we just put a hold on this until we decide what we're
doing with exporting include/scsi.

Arguments so far are

     1. Don't export and let glibc supply the headers (as it does now)
     2. Move headers to be exported to include/linux
     3. Take over include/scsi export from glibc: this will necessitate
        comparing our current headers and those of glibc and moving
        stuff around.

James

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Boaz Harrosh]

On 07/29/2009 04:56 PM, James Bottomley wrote:
> On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
>> scsi/scsi.h is exported to userspace, so it should
>> use __u8 instead of u8 as other userspace-visible headers do.
> 
> Actually, can we just put a hold on this until we decide what we're
> doing with exporting include/scsi.
> 
> Arguments so far are
> 
>      1. Don't export and let glibc supply the headers (as it does now)
>      2. Move headers to be exported to include/linux
>      3. Take over include/scsi export from glibc: this will necessitate
>         comparing our current headers and those of glibc and moving
>         stuff around.
> 

I'm all for 3. I think if done we should do it for all include/scsi at once.
No need for prolonged pain.

[]$ ls /usr/include/scsi
scsi.h
scsi_ioctl.h
scsi_tgt_if.h
sg.h

However I'll have to ask my employers if they can spare me, so any other
volunteers?

TODO:
* Fix up Kernel headers by complying to check_headers
* Test drive some example user-mode applications including glibc itself.
* send patches to glibc
* send patches to Kernel
* Notify distributions.

> James
> 

Boaz

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Sam Ravnborg]

On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
> On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
> > scsi/scsi.h is exported to userspace, so it should
> > use __u8 instead of u8 as other userspace-visible headers do.
> 
> Actually, can we just put a hold on this until we decide what we're
> doing with exporting include/scsi.
> 
> Arguments so far are
> 
>      1. Don't export and let glibc supply the headers (as it does now)
>      2. Move headers to be exported to include/linux
>      3. Take over include/scsi export from glibc: this will necessitate
>         comparing our current headers and those of glibc and moving
>         stuff around.

2 + 3...
Let include/scsi be kernel internal stuff.
And have the exported headers in include/linux.

This is how net/ handle their headers.

I did a quick diff of the glibc provided scsi.h and kernel scsi.h.
>From a quick look it seems that we have more in the kernel version
than the glibc version - and no obvious conflicts.

But agree with Boaz that we should do this in one go.

	Sam

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[James Bottomley]

On Wed, 2009-07-29 at 18:28 +0200, Sam Ravnborg wrote:
> On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
> > On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
> > > scsi/scsi.h is exported to userspace, so it should
> > > use __u8 instead of u8 as other userspace-visible headers do.
> > 
> > Actually, can we just put a hold on this until we decide what we're
> > doing with exporting include/scsi.
> > 
> > Arguments so far are
> > 
> >      1. Don't export and let glibc supply the headers (as it does now)
> >      2. Move headers to be exported to include/linux
> >      3. Take over include/scsi export from glibc: this will necessitate
> >         comparing our current headers and those of glibc and moving
> >         stuff around.
> 
> 2 + 3...
> Let include/scsi be kernel internal stuff.
> And have the exported headers in include/linux.

I don't quite understand what you're saying here.  I think 2 and 3 are
either/or options.  Either we move the exported files to include/linux
or we export from include/scsi.

I have to say I don't like option 2 because the breakage is visible to
user level programs (unless we can persuade glibc people to #include
<linux/scsi.h> in scsi/scsi.h).

> This is how net/ handle their headers.
> 
> I did a quick diff of the glibc provided scsi.h and kernel scsi.h.
> >From a quick look it seems that we have more in the kernel version
> than the glibc version - and no obvious conflicts.
> 
> But agree with Boaz that we should do this in one go.

James

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Sam Ravnborg]

On Wed, Jul 29, 2009 at 04:37:36PM +0000, James Bottomley wrote:
> On Wed, 2009-07-29 at 18:28 +0200, Sam Ravnborg wrote:
> > On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
> > > On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
> > > > scsi/scsi.h is exported to userspace, so it should
> > > > use __u8 instead of u8 as other userspace-visible headers do.
> > > 
> > > Actually, can we just put a hold on this until we decide what we're
> > > doing with exporting include/scsi.
> > > 
> > > Arguments so far are
> > > 
> > >      1. Don't export and let glibc supply the headers (as it does now)
> > >      2. Move headers to be exported to include/linux
> > >      3. Take over include/scsi export from glibc: this will necessitate
> > >         comparing our current headers and those of glibc and moving
> > >         stuff around.
> > 
> > 2 + 3...
> > Let include/scsi be kernel internal stuff.
> > And have the exported headers in include/linux.
> 
> I don't quite understand what you're saying here.  I think 2 and 3 are
> either/or options.  Either we move the exported files to include/linux
> or we export from include/scsi.
That we should in any case look at what glibc have defined in
their respective headers when we prepare the kernel versions for export.

> 
> I have to say I don't like option 2 because the breakage is visible to
> user level programs (unless we can persuade glibc people to #include
> <linux/scsi.h> in scsi/scsi.h).

We could turn this the other way around...
glibc could add a wrapper file when they are ready to rely on the kernel
version.
So userspace continue to work independent of glibc using their own set
of headers for scsi or they use the kernel supplied headers via the
wrapper header file . Just an idea.

It would be bad if userspace had to do something strange to pick
up scsi.h in scsi/ for some glibc versions and from linux/ for
other glibc versions.

	Sam

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Wed, Jul 29, 2009 at 05:09:53PM +0300, Boaz Harrosh wrote:
> TODO:
> * Fix up Kernel headers by complying to check_headers

This is accomplished by applying the following on top
of my previous patch:

Subject: scsi: make scsi/scsi.h headers_check clean

place extern declarations in scsi/scsi.h within ifdef __KERNEL__
so that make headers_install strips them out.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/scsi/scsi.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/scsi/scsi.h b/include/scsi/scsi.h
index dfcfaab..418465a 100644
--- a/include/scsi/scsi.h
+++ b/include/scsi/scsi.h
@@ -150,6 +150,7 @@ struct scsi_varlen_cdb_hdr {
 	/* service specific data follows */
 };
 
+#ifdef __KERNEL__
 static inline unsigned
 scsi_varlen_cdb_length(const void *hdr)
 {
@@ -165,6 +166,7 @@ scsi_command_size(const unsigned char *cmnd)
 	return (cmnd[0] == VARIABLE_LENGTH_CMD) ?
 		scsi_varlen_cdb_length(cmnd) : COMMAND_SIZE(cmnd[0]);
 }
+#endif
 
 /*
  *  SCSI Architecture Model (SAM) Status codes. Taken from SAM-3 draft
@@ -281,8 +283,10 @@ enum scsi_protocol {
 	SCSI_PROTOCOL_UNSPEC = 0xf, /* No specific protocol */
 };
 
+#ifdef __KERNEL__
 /* Returns a human-readable name for the device */
 extern const char * scsi_device_type(unsigned type);
+#endif
 
 /*
  * standard mode-select header prepended to all mode-select commands
-- 
1.6.2.5

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
>      3. Take over include/scsi export from glibc: this will necessitate
>         comparing our current headers and those of glibc and moving
>         stuff around.

Speaking of which: is there a chance some userspace still depends on
SCSI_IOCTL_TAGGED_{ENABLE,DISABLE} being defined (even though they don't
work) ? glibc headers have them ...

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Boaz Harrosh]

On 07/29/2009 11:56 PM, Michael S. Tsirkin wrote:
> On Wed, Jul 29, 2009 at 05:09:53PM +0300, Boaz Harrosh wrote:
>> TODO:
>> * Fix up Kernel headers by complying to check_headers
> 
> This is accomplished by applying the following on top
> of my previous patch:
> 

This is not enough. The same should be done to the other 3 
include/scsi headers I mentioned and export them.

<snip>

Boaz

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Boaz Harrosh]

On 07/29/2009 07:37 PM, James Bottomley wrote:
> On Wed, 2009-07-29 at 18:28 +0200, Sam Ravnborg wrote:
>> On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
>>> On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
>>>> scsi/scsi.h is exported to userspace, so it should
>>>> use __u8 instead of u8 as other userspace-visible headers do.
>>> Actually, can we just put a hold on this until we decide what we're
>>> doing with exporting include/scsi.
>>>
>>> Arguments so far are
>>>
>>>      1. Don't export and let glibc supply the headers (as it does now)
>>>      2. Move headers to be exported to include/linux
>>>      3. Take over include/scsi export from glibc: this will necessitate
>>>         comparing our current headers and those of glibc and moving
>>>         stuff around.
>> 2 + 3...
>> Let include/scsi be kernel internal stuff.
>> And have the exported headers in include/linux.
> 
> I don't quite understand what you're saying here.  I think 2 and 3 are
> either/or options.  Either we move the exported files to include/linux
> or we export from include/scsi.
> 
> I have to say I don't like option 2 because the breakage is visible to
> user level programs (unless we can persuade glibc people to #include
> <linux/scsi.h> in scsi/scsi.h).
> 

This is, to me, a definition of a mess. Please (very strongly) don't do that.
The file is scsi/scsi.h and that's it.

I don't know for a fact but I can imagine that there was a time that Kernel
did not export any headers and glibc would sample a set every release. At some
point in time a kind soul decided that Kernel Headers should be exported from
the Kernel package and the rest is history. But he forgot that include/scsi/ is
also included. Lets just fix his simple mistake and fix this by taking back
control of Kernel headers.

>> This is how net/ handle their headers.
>>

If it was done so originally, but now these headers are scsi/*.h, it's
to late.

>> I did a quick diff of the glibc provided scsi.h and kernel scsi.h.
>> >From a quick look it seems that we have more in the kernel version
>> than the glibc version - and no obvious conflicts.
>>

exactly it is original Kernel headers from the time they where sampled
last by glibc.

>> But agree with Boaz that we should do this in one go.
> 

> James
> 
> 

Boaz

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Thu, Jul 30, 2009 at 12:23:47PM +0300, Boaz Harrosh wrote:
> >> I did a quick diff of the glibc provided scsi.h and kernel scsi.h.
> >> >From a quick look it seems that we have more in the kernel version
> >> than the glibc version - and no obvious conflicts.
> >>
> 
> exactly it is original Kernel headers from the time they where sampled
> last by glibc.

glibc also added
#include <features.h>
in some of them.
Donnu why.

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Wed, Jul 29, 2009 at 05:09:53PM +0300, Boaz Harrosh wrote:
> []$ ls /usr/include/scsi
> scsi.h
> scsi_ioctl.h
> scsi_tgt_if.h
> sg.h

$wget http://ftp.gnu.org/gnu/glibc/glibc-2.10.1.tar.bz2
$cd glibc-2.10.1
$find . -name scsi
./sysdeps/unix/sysv/linux/scsi
$ls ./sysdeps/unix/sysv/linux/scsi/
scsi.h  scsi_ioctl.h  sg.h

where did you get scsi_tgt_if.h?

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Thu, Jul 30, 2009 at 12:05:32PM +0300, Boaz Harrosh wrote:
> On 07/29/2009 11:56 PM, Michael S. Tsirkin wrote:
> > On Wed, Jul 29, 2009 at 05:09:53PM +0300, Boaz Harrosh wrote:
> >> TODO:
> >> * Fix up Kernel headers by complying to check_headers
> > 
> > This is accomplished by applying the following on top
> > of my previous patch:
> > 
> 
> This is not enough. The same should be done to the other 3 
> include/scsi headers I mentioned and export them.

Right. Well, that's easy enough, isn't it?

Subject: scsi: export scsi_ioctl.h and sg.h headers

glibc uses scsi/sg.h and scsi/scsi_ioctl.h, so we need to
export these headers.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/scsi/Kbuild |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/scsi/Kbuild b/include/scsi/Kbuild
index 33b2750..a61932c 100644
--- a/include/scsi/Kbuild
+++ b/include/scsi/Kbuild
@@ -1,3 +1,5 @@
+header-y += sg.h
+header-y += scsi_ioctl.h
 header-y += scsi.h
 header-y += scsi_netlink.h
 header-y += scsi_netlink_fc.h
-- 
1.6.2.5

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Sam Ravnborg]

On Thu, Jul 30, 2009 at 12:34:03PM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 30, 2009 at 12:23:47PM +0300, Boaz Harrosh wrote:
> > >> I did a quick diff of the glibc provided scsi.h and kernel scsi.h.
> > >> >From a quick look it seems that we have more in the kernel version
> > >> than the glibc version - and no obvious conflicts.
> > >>
> > 
> > exactly it is original Kernel headers from the time they where sampled
> > last by glibc.
> 
> glibc also added
> #include <features.h>
> in some of them.
> Donnu why.

Browsing features.h this looked unused (by scsi.h) to me.

	Sam

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Boaz Harrosh]

On 07/30/2009 12:36 PM, Michael S. Tsirkin wrote:
> On Wed, Jul 29, 2009 at 05:09:53PM +0300, Boaz Harrosh wrote:
>> []$ ls /usr/include/scsi
>> scsi.h
>> scsi_ioctl.h
>> scsi_tgt_if.h
>> sg.h
> 
> $wget http://ftp.gnu.org/gnu/glibc/glibc-2.10.1.tar.bz2
> $cd glibc-2.10.1
> $find . -name scsi
> ./sysdeps/unix/sysv/linux/scsi
> $ls ./sysdeps/unix/sysv/linux/scsi/
> scsi.h  scsi_ioctl.h  sg.h
> 
> where did you get scsi_tgt_if.h?

You're right I have it from the SCSI target framework (tgt)
(http://stgt.berlios.de/)

It is a Kernel header that should be exported, I'll send a
patch to export it.

Thanks
Boaz

Re: [PATCH] scsi: consistent use of __u8 in scsi/scsi.h

[Michael S. Tsirkin]

On Wed, Jul 29, 2009 at 08:56:03AM -0500, James Bottomley wrote:
> On Wed, 2009-07-29 at 14:11 +0300, Michael S. Tsirkin wrote:
> > scsi/scsi.h is exported to userspace, so it should
> > use __u8 instead of u8 as other userspace-visible headers do.
> 
> Actually, can we just put a hold on this until we decide what we're
> doing with exporting include/scsi.

Any update?