Upstream contributing

From Lustre Wiki
Revision as of 15:52, 11 April 2016 by Adilger (Talk | contribs) (Preparing the patch: remove "<br>" from formatted lines)

(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to: navigation, search

Getting started

The upstream Lustre client code is currently hosted in the staging area of the linux kernel tree. All current efforts for the upstream client are done with the git staging repo and that work then goes into Linus's tree during the merge window shortly after a Linux kernel release is cut. The git staging tree has several branches but the one of interest for contributing is the staging-testing branch. Any patches created against the other branches will likely not even apply correctly so please ensure you are working and testing with the staging-testing branch. Before we can start developing patches we need to obtain the source repo, build and install the Lustre enabled kernel with the proper utilities. The steps to getting started are listed below. In my example I named by git repo top directory lustre-upstream but you can name it whatever you want.

cd your-work-directory
git clone git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git lustre-upstream
cd lustre-upstream
git checkout -b staging-testing origin/staging-testing

Kernel configuration

With the lustre client source tree available the next step will be configuring your kernel to enable Lustre. The easiest way to configure a kernel is to run make menuconfig. If this fails it most likely is due to the missing libncurses library that is required. Running the make menuconfig will present you the top level menu. The section for Lustre is located in the Device Drivers section:

< > Volume Management Device Driver
[*] Networking support --->
Device Drivers --->
Firmware Drivers --->

Once in the Device Drivers menu you want to enable the Staging driver menu as shown here:

Virtio drivers --->
Microsoft Hyper-V guest support ----
[*] Staging drivers --->
[ ] X86 Platform Specific Device Drivers --->

Enable the staging driver allows you to display the options of that menu. Scroll down to the Staging drivers option and hit enter to go to the Staging menu. In the menu you will see:

< > GCT GDM724x LTE support>
< > TTY over Firewire
< > Lustre networking subsystem (LNet)
< > Digi Neo and Classic PCI Products

You want to select Lustre networking subsystem which will in turn enable several other options.

< > GCT GDM724x LTE support
< > TTY over Firewire
<M> Lustre networking subsystem (LNet)
 (1048576) Lustre lnet max transfer payload (default 1MB) <M> Lustre networking self testing
<M> LNET infiniband support
<M> Lustre file system client support
(8192) Lustre obd max ioctl buffer bytes (default 8KB)
[ ] Enable Lustre DEBUG checks
<M> Lustre virtual block device
< > Digi Neo and Classic PCI Products
< > Xilinx FPGA firmware download module

These default values will be enough to get you on your way to building a lustre client. Help sections are provided for all the entries to help you understand your options. Once you have made your selections then the normal build process of make bzImage and make modules on x86 machines will produce the lustre client modules. For other platforms or if you want to create debian or rpm packages please consult freely available documentation about kernel building on the web. Also feel free to ask for help on the lustre-devel mailing list. Once your kernel is ready just install it as you normally would a new kernel and reboot.

Building the Lustre utilities

To use the lustre upstream client properly you will need to install the lustre utilities rpm or debian packages. First you will need to download the latest Lustre OpenSFS/Intel repository. Like the upstream repo checkout you can name it something different than lustre-intel-branch in this example.

cd my-work-space
git clone git://git.hpdd.intel.com/fs/lustre-release lustre-intel-branch
cd lustre-intel-branch
sh ./autogen.sh
./configure --disable-server
make rpm - make debs

Once it completes you will have the debian based packages stored in the debs directory or the rpms in the top level source directory. In either case you only need the utilities package. In the rpm case it is the lustre-'version'*.rpm and the debian package of interest is lustre-utils_*.deb. Besides the utility package the other package that could be of interest is the lustre-tests-* package which contains an test suite used to validate Lustre to avoid potential regressions observed in the past. If you are interested in full fledge testing then I recommend reading the How to test Lustre Code section.

Once all your packages are installed you will need to setup a lustre file system. An excellent link to get the newcomer going is https://wiki.hpdd.intel.com/display/PUB/Create+and+Mount+a+Lustre+Filesystem. As always if you have questions feel free to ask on the lustre-devel mailing and people will gladly help you.

Developing for upstream client

This section goes over how to become actively involved in the upstream client work. The process of creating and sending patches is a very different from what is done for the OpenSFS/Intel branch. We will go over the setup of sending patches as well as meeting the coding standards for acceptance.  

git setup for patch submitting

Now that you are familiar with Lustre setup and would like to contribute the next step is too configure .gitconfig to send patches to Greg and the staging tree. I will show my .gitconfig I use for pushing patches upstream.

[sendemail]
       to = Greg Kroah-Hartman <gregkh@linuxfoundation.org>
       to = devel@driverdev.osuosl.org
       to = Oleg Drokin <oleg.drokin@intel.com>
       to = Andreas Dilger <andreas.dilger@intel.com>
       to = James Simmons <jsimmons@infradead.org>
       cc = Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
       cc = Lustre Development List <lustre-devel@lists.lustre.org>
       confirm = auto
[diff]
       renames = true

Basically the .gitconfig is set to email patches to the correct people. I also add a [diff] field to handle renaming of files for when it does happen. Most users will most like never use that functionality. After you merge your patches the easiest way to prepare to send them by:

git send-email -"# of patches" --compose
Edit your cover letter

Coding Style

All patches submitted must adhered to the linux kernel standard. Details about this coding standard can be read at https://www.kernel.org/doc/Documentation/CodingStyle. For those used to working with the OpenSFS/Intel tree you will find most of the standard match what the linux kernel requires. Some differences do exist which we will go over here to make people aware of them.

Code style changes different from OpenSFS

1) The linux kernel disapproves of if (ptr == NULL) and if (var == 0) type testing. The preferred way is if (ptr) or if (!var).

2) No white spacing.

Don't define variables like this:
int                foo;
int                rc;
 
Don't use white space when setting variables
var               = value;
var2              = value2;

3) Don't use generic label names such as "goto out". Make those labels meaningful such as "goto out_obd_device" where the "obd_device" and all prior allocation/setup is cleaned up.

4) Avoid many returns in a function. This is covered in the kernel coding style documents but is worth going over. It is encourage to use goto instead.

5) Check for errors and not success when handling function return codes. When writing code it is preferred to handle the return codes:

       rc = my_function(...)
       if (rc) {
                Handle error condition;
                goto out_cleanup;
       }

5) For kernel code never return -1 for a error, use a proper -ESOMETHING return code.

6) Lastly, the 80 character per line limit can be relaxed. The kernel maintainer value readable code over the 80 character limit. Also in the case of strings do not break them across multiple lines. The reasoning is that if a person encounters an error message in the logs it will be easier to locate a continuous string in the code to help track down an issue. The above covers the most common difference between the style difference between the OpenSFS/Intel branch and the upstream kernel client. Any further questions on this topic can be asked on the lustre-devel mailing list.

Preparing the patch

Once you you have made your changes to the staging tree the next step is preparing the patch. Two standard patch formats exist depending one if it is an original work or a patch being ported from the OpenSFS/Intel branch. The simple case is an original fix that is not present in the OpenSFS/Intel branch. For this case when you create your patch please put in the format:

staging: lustre: subsystem:  my fix to apply

Details of the fix to apply.

Signed-off-by: Your Name <my.email@address>

Also accepted is "staging/lustre/subsystem: my fix to apply" depending on your personal style. Also in the case your patch expands across the source tree using subsystem is not needed.

When porting a patch from the OpenSFS/Intel branch to master requires more details to be added in the patch. Patches that are merged into the OpenSFS tree have a particular format which is mostly preserved when porting to the upstream client. The changes needed to port a OpenSFS/Intel patch to the upstream client are:

1) major differences are the title of the commit message is changes from the "LU-XXXX subsystem: my bug fix" to "staging: lustre:subsystem: my bug fix". If the original patch touched multiple subsystems then subsystem is optional in the commit message.

2) Replace Change-Id: I6346753... line with Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-XXXX

3) Remove Tested-by: **** lines (e.g. Jenkins, Maloo, etc) unless the patch was tested by a real person.

4) Add your own Signed-off-by: line so who ported the patch can be kept track of.

In the end your patch to email should look like the following:

staging: lustre: libcfs: delete linux-mem.h

The header linux-mem.h is no longer needed after all macros were removed.
Delete this file.

Signed-off-by: original patch author <author@address>
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-XXXX
Reviewed-on: http://review.whamcloud.com/YYYYY
Reviewed-by: Reviewer1 <reviewer1@someaddress>
Reviewed-by: Reviewer2 <reviewer2@yetanotheraddress>
Signed-off-by: Your Name <my.email@address>

With the case of porting patches to the upstream client please remember to change the authorship of the patch. The git command to accomplish this is:

git commit --amend --author "original author real name <author@address>"

After your patch is ready, generate it with git format-patch and run the result through scripts/checkpatch.pl to ensure no style issues are introduced. Remember earlier in this documentation style issues were discussed so apply common sense with some of the checkpatch errors. Common checkpatch errors are strings being reported over 80 characters in length which can be safely ignored as a example. Once you have fixed your patch to address any real warnings reported by checkpatch.pl you can run git send-email to publish your patch. If you are unfamiliar with this part of the git process please reference the git man pages or you can freely ask questions on the lustre-devel mailing list.