linux-kernel - Re: [PATCH 04/18] libosd: OSDv1 preliminary implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49119BAF.6040603@panasas.com>
Date:	Wed, 05 Nov 2008 15:12:15 +0200
From:	Boaz Harrosh <bharrosh@...asas.com>
To:	Sam Ravnborg <sam@...nborg.org>
CC:	James Bottomley <James.Bottomley@...senPartnership.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mike Christie <michaelc@...wisc.edu>,
	FUJITA Tomonori <fujita.tomonori@....ntt.co.jp>,
	Jeff Garzik <jeff@...zik.org>,
	open-osd ml <osd-dev@...n-osd.org>,
	linux-scsi <linux-scsi@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Sami.Iren@...gate.com, Pete Wyckoff <pw@...d.com>
Subject: Re: [PATCH 04/18] libosd: OSDv1 preliminary implementation

Sam Ravnborg wrote:

> On Tue, Nov 04, 2008 at 06:44:29PM +0200, Boaz Harrosh wrote:
>> Implementation of the most basic OSD functionality and
>> infrastructure. Mainly Format, Create/Remove Partition,
>> Create/Remove Object, and read/write.
>>
>> - Add Makefile and Kbuild to compile libosd.ko
>> - osd_initiator.c Implementation file for osd_initiator.h
>>   and osd_sec.h APIs
>> - osd_debug.h - Some kprintf macro definitions
> 
> A few comments below.
> 
> 	Sam
> 

Thanks Sam for looking

> 
>> Signed-off-by: Boaz Harrosh <bharrosh@...asas.com>
>> Reviewed-by: Benny Halevy <bhalevy@...asas.com>
>> ---
>>  drivers/scsi/osd/Kbuild          |   26 +++
>>  drivers/scsi/osd/Makefile        |   37 +++
>>  drivers/scsi/osd/osd_debug.h     |   27 +++
>>  drivers/scsi/osd/osd_initiator.c |  450 ++++++++++++++++++++++++++++++++++++++
>>  4 files changed, 540 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/scsi/osd/Kbuild
>>  create mode 100755 drivers/scsi/osd/Makefile
>>  create mode 100644 drivers/scsi/osd/osd_debug.h
>>  create mode 100644 drivers/scsi/osd/osd_initiator.c
>>
>> diff --git a/drivers/scsi/osd/Kbuild b/drivers/scsi/osd/Kbuild
>> new file mode 100644
>> index 0000000..b4678e0
>> --- /dev/null
>> +++ b/drivers/scsi/osd/Kbuild
>> @@ -0,0 +1,26 @@
>> +#
>> +# Kbuild for the OSD modules
>> +#
>> +# Copyright (C) 2008 Panasas Inc.  All rights reserved.
>> +#
>> +# Authors:
>> +#   Boaz Harrosh <bharrosh@...asas.com>
>> +#   Benny Halevy <bhalevy@...asas.com>
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License version 2
>> +#
>> +
>> +ifneq ($(OSD_INC),)
>> +# we are built out-of-tree Kconfigure everything as on
>> +
>> +CONFIG_SCSI_OSD_INITIATOR=m
>> +EXTRA_CFLAGS += -DCONFIG_SCSI_OSD_INITIATOR -DCONFIG_SCSI_OSD_INITIATOR_MODULE
>> +
>> +EXTRA_CFLAGS += -I$(OSD_INC)
>> +# EXTRA_CFLAGS += -DCONFIG_SCSI_OSD_DEBUG
>> +
>> +endif
>> +
>> +libosd-objs := osd_initiator.o
>> +obj-$(CONFIG_SCSI_OSD_INITIATOR) += libosd.o
> 
> When you submit for inclusion please clean this up.
> 1) use ccflags-y as replacement for EXTRA_CFLAGS
> 2) use libosd-y as replacement for libosd-objs
> 

This is a most valuable information thanks. I have copy
pasted these stuff, while learning. I guess from the wrong
example. Thanks it looks much more logical that way.

>> +
>> +#ifdef CONFIG_SCSI_OSD_INITIATOR_MODULE
>> +MODULE_AUTHOR("Boaz Harrosh <bharrosh@...asas.com>");
>> +MODULE_DESCRIPTION("open-osd initiator library libosd.ko");
>> +MODULE_LICENSE("GPL");
>> +#endif
> 
> no ifdef around here.
> 

Grate, thanks. Again the wrong copy-paste.

>> +void osd_dev_init(struct osd_dev *osdd, struct scsi_device *scsi_dev)
>> +{
>> +	memset(osdd, 0, sizeof(*osdd));
>> +	osdd->scsi_dev = scsi_dev;
>> +	osdd->def_timeout = BLK_DEFAULT_SG_TIMEOUT;
>> +	/* TODO: Allocate pools for osd_request attributes ... */
>> +}
>> +EXPORT_SYMBOL(osd_dev_init);
> kernel-doc comments for all exported funtions / variables.
> 

I have some kernel-doc comments of exported functions in the Header
file. I have not yet finished all of them. (Laziness on my part).

Are kernel-doc comments in headers a big NO-NO. I like it this way,
so when I have to learn a new Library all the information
I need to know is in the header. Also the header is a much better place
when you do programing by shopping, that is you don't know what you need
and you look for what's available.

Thanks
Boaz
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/