Alexey Dobriyan <[email protected]> writes:
> Signed-off-by: Alexey Dobriyan <[email protected]>
Hi. Thanks for the patch. Have you tested it? If you don't have any
ATA over Ethernet hardware, you can using the alpha vblade program for
testing. (Run it on a system in the broadcast domain of the host
running your patched aoe driver, and vblade will export any file,
e.g., /dev/loop0, as block storage.) It's at
http://sf.net/projects/aoetools
I was trying to determine what sparse warnings you see, so I got
sparse from bk://sparse.bkbits.net/sparse and ran it. Your patch cuts
down significantly on the complaints, but there are some that persist.
Maybe you're using an older version of sparse?
ecashin@kokone linux-2.6.11-rc4-bk9$ make C=1
CHK include/linux/version.h
make[1]: `arch/i386/kernel/asm-offsets.s' is up to date.
CHK include/asm-i386/asm_offsets.h
CHK include/linux/compile.h
CHK usr/initramfs_list
CHECK drivers/block/aoe/aoeblk.c
CC [M] drivers/block/aoe/aoeblk.o
CHECK drivers/block/aoe/aoechr.c
drivers/block/aoe/aoechr.c:236:24: warning: symbol 'aoe_fops' was not declared. Should it be static?
This change has been made already, so I'll check whether I've pushed
the change up. I have a couple of things I haven't submitted yet.
CC [M] drivers/block/aoe/aoechr.o
CHECK drivers/block/aoe/aoecmd.c
drivers/block/aoe/aoecmd.c:27:17: warning: incorrect type in assignment (different base types)
drivers/block/aoe/aoecmd.c:27:17: expected unsigned short [unsigned] protocol
drivers/block/aoe/aoecmd.c:27:17: got restricted unsigned short [usertype] [force] <noident>
CC [M] drivers/block/aoe/aoecmd.o
CHECK drivers/block/aoe/aoedev.c
CC [M] drivers/block/aoe/aoedev.o
CHECK drivers/block/aoe/aoemain.c
CC [M] drivers/block/aoe/aoemain.o
CHECK drivers/block/aoe/aoenet.c
drivers/block/aoe/aoenet.c:156:10: warning: incorrect type in initializer (different base types)
drivers/block/aoe/aoenet.c:156:10: expected unsigned short [unsigned] type
drivers/block/aoe/aoenet.c:156:10: got restricted unsigned short [usertype] [force] <noident>
CC [M] drivers/block/aoe/aoenet.o
LD [M] drivers/block/aoe/aoe.o
Kernel: arch/i386/boot/bzImage is ready
Building modules, stage 2.
MODPOST
CC drivers/block/aoe/aoe.mod.o
LD [M] drivers/block/aoe/aoe.ko
ecashin@kokone linux-2.6.11-rc4-bk9$
The "array abuse" is something that I'm not all that enthusiastic
about changing, since it's mostly a style issue, and last time I
changed it the way your patch does, the original author of the patch
changed it back. But you've figured out how to make sparse happy, and
for that I'm grateful! :)
--
Ed L Cashin <[email protected]>
On Thursday 24 February 2005 19:23, Ed L Cashin wrote:
> Have you tested it?
Not yet.
> If you don't have any
> ATA over Ethernet hardware, you can using the alpha vblade program for
> testing.
OK. Will try.
> I was trying to determine what sparse warnings you see, so I got
> sparse from bk://sparse.bkbits.net/sparse and ran it. Your patch cuts
> down significantly on the complaints, but there are some that persist.
> Maybe you're using an older version of sparse?
No. Those three were deliberately left as is because they aren't local to
AOE.
> drivers/block/aoe/aoechr.c:236:24: warning: symbol 'aoe_fops' was not declared. Should it be static?
> drivers/block/aoe/aoecmd.c:27:17: warning: incorrect type in assignment (different base types)
> drivers/block/aoe/aoecmd.c:27:17: expected unsigned short [unsigned] protocol
> drivers/block/aoe/aoecmd.c:27:17: got restricted unsigned short [usertype] [force] <noident>
> drivers/block/aoe/aoenet.c:156:10: warning: incorrect type in initializer (different base types)
> drivers/block/aoe/aoenet.c:156:10: expected unsigned short [unsigned] type
> drivers/block/aoe/aoenet.c:156:10: got restricted unsigned short [usertype] [force] <noident>
> The "array abuse" is something that I'm not all that enthusiastic
> about changing,
I am.
> since it's mostly a style issue,
It isn't.
struct aoe_hdr {
unsigned char tag[4];
};
struct aoe_hdr *h;
u32 net_tag;
net_tag = __cpu_to_be32(n);
memcpy(h->tag, &net_tag, sizeof net_tag);
This code is plain ugly. When AOE was merged there were _plenty_ of examples
of LE and BE fields in structs.
> and last time I
> changed it the way your patch does, the original author of the patch
> changed it back.
Please, show him include/linux/ext2_fs.h::struct ext2_group_desc{} and
fs/ext2/super.c::ext2_check_descriptors(), for example.
> But you've figured out how to make sparse happy, and
> for that I'm grateful! :)
:)
Alexey