Upstream contributing: Difference between revisions

From Lustre Wiki
Jump to navigation Jump to search
(→‎Getting started: remove text about staging branch which is no longer valid)
Tag: Replaced
 
Line 1: Line 1:
Lustre client code targetted for upstream kernel submission is currently being managed as part of the 'master' Lustre development branch, and submitted patches should follow the normal Lustre patch submission process.  Once all issues in the Lustre code that are known to be blocking upstream submission have been addressed, then the client will be resubmitted to the upstream kernel.
Lustre client code targetted for upstream kernel submission is currently being managed as part of the 'master' Lustre development branch, and submitted patches should follow the normal Lustre patch submission process.  Once all issues in the Lustre code that are known to be blocking upstream submission have been addressed, then the client will be resubmitted to the upstream kernel.
== 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 to configure '''.gitconfig''' to send patches to Greg and the staging tree. I will show my '''.gitconfig''' I use for pushing patches upstream.
    [sendemail]
        to = NeilBrown <[email protected]>
        to = Oleg Drokin <[email protected]>
        to = Andreas Dilger <[email protected]>
        to = James Simmons <[email protected]>
        cc = Lustre Development List <[email protected]>
        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 which is very similar to the [[Lustre Coding Style Guidelines]]. 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 the Lustre tree ====
1) The linux kernel disapproves of '''if (ptr == NULL)''', and '''if (rc == 0)''' testing (though the latter is acceptable if checking a numeric field for zero). The preferred way is '''if (ptr)''' or '''if (!rc)'''.
2) No extra spaces between types and variables (this is now also true for Lustre patches).
: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 (which is '''-EPERM'''), use a proper '''-ESOMETHING''' return code.
6) Lastly, the 80 character per line limit can be relaxed in some cases. The kernel maintainer value readable code over the 80 character limit, though it is normally preferred to restructure code rather than have too much indentation. The main exception is 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 [[Mailing_Lists_and_IRC|lustre-devel]] mailing list.
7) Much of the coding style information can be found in lustre-upstream/Documentation/process/*
=== 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.  See [[Commit Comments]] and [[Commit Comments#Patch_porting_examples|Patch Porting Examples]] for details on what the commit comments should contain.
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:
    lustre: ''subsystem'':  my fix to apply
    Details of the fix to apply.
    Signed-off-by: Your Name <my.email@address>
Also accepted is "'''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:
# major differences are the title of the commit message is changes from the "'''LU-XXXX ''subsystem'': my bug fix'''" to "'''lustre: ''subsystem'': my bug fix'''". If the original patch touched multiple subsystems then ''subsystem'' is optional in the commit message.
# Replace ''Change-Id: I6346753...'' line with ''WC-bug-id: https://jira.whamcloud.com/browse/LU-XXXX''
# Remove ''Tested-by: ****'' lines (e.g. Jenkins, Maloo, etc) unless the patch was tested by a real person.
# 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:
    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>
    WC-bug-id: https://jira.whamcloud.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 the kernel '''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 [[Mailing_Lists_and_IRC|lustre-devel]] mailing list.
[[Category:Development]]

Latest revision as of 14:41, 3 April 2024

Lustre client code targetted for upstream kernel submission is currently being managed as part of the 'master' Lustre development branch, and submitted patches should follow the normal Lustre patch submission process. Once all issues in the Lustre code that are known to be blocking upstream submission have been addressed, then the client will be resubmitted to the upstream kernel.