Page History
Canonical version of this page is at https://wiki.lustre.org/Commit_Comments
Contents:
| Recent edits:
|
Commit message content
Writing good commit comments is critical to ensuring that changes are easily understood, even years after they were originally written. The commit comment should contain enough information about the change to allow the reader to understand the motivation for the change, what parts of the code it is affecting, and any interesting, unusual, or complex parts of the change to draw attention to.
...
The first line of the commit comment is the commit summary of the change. Changes submitted to the fs/lustre-release
branch require a Lustre Jira ticket number at the beginning of the commit summary. A Lustre Jira ticket is one that begins with LU and is therefore part of the Lustre project within Jira. For patches to other projects, such as documentation, a different JIRA project should be used (e.g. LUDOC
). If the patch is submitted for the fs/lustre-release
repository without a Lustre Jira ID in the first line, then it will automatically receive a -2 review which will prevent the patch from being submitted to a release branch. You would then need to fix the summary line and resubmit the patch.
The commit summary should also have a component:
tag immediately following the Jira ticket number that indicates which Lustre subsystem that the commit is related to. Example Lustre subsystems relate to modules like: llite, lov, osc, mdc, lmv, ldlm, ptlrpc, mds, oss, mdd, osd-ldiskfs, osd-zfs, ldiskfs, lnet, libcfs, socklnd, o2iblnd
; functional components like recovery, quota, grant
; or auxiliary components like build, tests, iokit, docs
. This subsystem list is not exhaustive, but provides a good guideline for consistency.
...
No Format |
---|
LU-nnn component: short description of change under 62 columns The "component:" should be a lower-case single-word subsystem of the Lustre code that best encompasses the change being made. Examples of components include modules: llite, lov, lmv, osc, mdc, ldlm, lnetmds, oss, ptlrpc, mds, ossosd-ldiskfs, osd-ldiskfszfs, ldiskfs, libcfslnet, socklnd, o2iblnd, libcfs; functional subsystems: recovery, quota, grant; and auxilliaryauxiliary areas: build, tests, docs. This list is not exhaustive, but is a guideline. The commit comment should contain a detailed explanation of changes being made. This can be as long as you'd like. Please give details of what problem was solved (including error messages or problems that were seen), a good high-level description of how it was solved, and which parts of the code were changed (including important functions that were changed, if this is useful to understand the patch, and for easier searching). Wrap lines at/under 70 columns. Signed-off-by: Your Real Name <your_email@domain.name> Change-Id: Ixxxx(added automatically if missing)xxxx |
The Signed-off-by: line
The Signed-off-by:
line asserts that you have permission to contribute the code to the project according to the Developer's Certificate of Origin.
The Change-Id: line and Code Style
Gerrit needs to automatically identify updates to existing patches and update the existing change instead of creating a new one for each patch submitted. This preserves the history of patch comments, and allows comparing old and new versions of a patch for more efficient inspections. For this to work properly the changes you create locally need to have a unique commit id in them. The same Change-Id:
line should be used for all versions of a patch.
Please make sure the Change-Id:
line and Signed-off-by:
lines are at the bottom of the comment. This is required by the fs/lustre-release
project and other projects in order for the patch to be approved. If your comments are missing either of these lines the patch will be automatically be rejected at submission time.
The Change-Id:
field is inserted automatically into the commit message by installing the buildcontrib/git-hooks/commit-msg
hook into each Git repository's .git/hooks/
directory. The buildcontrib/git-hooks/prepare-commit-msg
script should be installed as well. From the top-level Lustre tree checkout:
No Format |
---|
ln -sf ../../contrib/buildgit-hooks/commit-msg .git/hooks/ ln -sf ../../contrib/buildgit-hooks/prepare-commit-msg .git/hooks/ |
At git commit
time this will run the build/prepare-commit-msg
script from the currently Lustre tree to check the code changes for style errors using buildcontrib/checkpatch.pl
, and add comments at the end of the commit message with any warnings or errors. After the commit message has been saved, the build/commit-msg
script will verify that the commit message format matches the format specified above, and insert the CommitChange-Id:
field into the commit message, if one isn't already present.
...
Lustre-commit:
and Lustre-change:
are used to reference the original version of a patch that is being ported to another branch or .
Patches pushed to the upstream kernel . The Linux-commit:
tag is used for tracking patches that were originally committed to the upstream Linux kernel.
Test-Parameters:
is used to specify additional testing parameters for this patch, see Changing Test Parameters with Gerrit Commit Messages.
Examples of good commit comments
should include Intel-bug-id:
to reference the JIRA URL for the LU ticket, in the form https://jira.hpdd.intel.com/browse/LU-nnnn
. Patches should also include the Reviewed-on:
permalink URL for the Gerrit patch, in the form https://review.whamcloud.com/nnnnn
. The upstream maintainers do not want to have commit hashes for non-kernel commits, so upstream patches should not include the Lustre-commit:
tag.
Patches ported from the upstream kernel, the patch should keep the original commit summary line, with slight modification to conform to Lustre standards: include the JIRA LU ticket number, and replace "staging" and other pathnames in the summary with a subsystem. The Linux-commit:
tag is used to hold the upstream kernel commit hash. The original author should be kept as the author of the patch using the git --author
option, as well as the original Signed-off-by:
tag(s).
Test-Parameters:
is used to specify additional testing parameters for this patch, see Changing Test Parameters with Gerrit Commit Messages.
Examples of good commit comments
No Format |
---|
LU-477 ldiskfs: allocate s_group_desc/s_group_info by vmalloc()
Add the patch |
No Format |
LU-477 ldiskfs: allocate s_group_desc/s_group_info by vmalloc() Add the patch to the RHEL6 ldiskfs patch series. Large kmalloc() for sbi->s_group_desc and sbi->s_group_info can fail for large filesystems (typically over 16TB), which will cause the "not enough memory" error while mounting. This patch makes allocation fall back to vmalloc() if the kmalloc() failed, as what was done for sbi->s_flex_groups. To avoid colliding with an valid on-disk inode number, EXT4_BAD_INO is used as the number of the buddy cache inode. The patch also incorporates the following upstream kernel fix: Linux-commit: 32a9bb57d7c1fd04ae0f72b8f671501f000a0e9f ext4: fix missing iput() of root inode for some mount error paths https://bugzilla.kernel.org/show_bug.cgi?id=26752 Signed-off-by: Yu Jian <yujian whamcloud<yujian@whamcloud.com> Change-Id: I3950425835ea7f2968ceb2edbc622e3ff3ed8545 |
...
For patches backported from master to a Lustre maintenance branch (e.g. b2_
510
) there are some conventions to follow so that the changes/fixes can more easily be tracked across branches. The simples method for porting a patch from one branch to another is to use the "Cherry Pick"
button on the patch directly in Gerrit, which can be used for patches that the changes/fixes can more easily be tracked across branches. The best method for porting a patch from one branch to another is to use apply cleanly to the specified target branch as long as it is in the same Git repository. Alternately, if this is not possible, use git cherry-pick {commit_hash
} on the branch where you want the patch to land|branch}
from the command line to pull the patch onto the (current) branch where you want the patch to land, and then using the normal patch submission process to push the patch to Gerrit or submit it to the upstream kernel. This will apply the whole patch (as best as is able, and show conflicts where needed), copy the commit message, preserve the original patch author. With luck, there will not be any patch conflicts and no further work is needed. If necessary, the patch conflicts need to be resolved before committing the patch. For the commit message:
Patches ported from master to maintenance branch
For example, porting a patch from master to b2_10
or similar:
No Format |
---|
LU-4725 mdt: child-parent lock ordering in rename Change rename so that it always has parent-child lock ordering, otherwise it may deadlock with other operations. Lustre-commit: 4e308ef74f271ec7e19917e3c0f88586537582c3 Lustre-change: http://review.whamcloud.com/9538 LU-4725 obd: lu_object_find_at hung lu_object_find_at hangs if called two times and object is removed in between. It makes mdt_rename_sanity not workable for rename. Change mdt_rename_sanity to work on existing object. Lustre-commit: b7c72ec1ddeda2cf94ea151f974d3f94e3a7d1ed Lustre-change: http://review.whamcloud.com/10484 Xyratex-bug-id: MRP-1700 Test-Parameters: alwaysuploadlogs \ envdefinitions=SLOW=yes,ENABLE_QUOTA=yes,ONLY=54 \ ossjob=lustre-b2_4 mdsjob=lustre-b2_4 ossbuildno=73 mdsbuildno=73 \ testlist=sanityn Signed-off-by: Vitaly Fertman <vitaly_fertman@xyratex.com> Signed-off-by: Rahul Deshmukh <rahul_deshmukh@xyratex.com> Change-Id: Ic9ce52bfcd8788834347fba155cc8c6be674dcd8 |
...
Patches ported from master to
...
upstream kernel
These are treated similarly as patches ported to maintenance branches (keep all comments and Signed-off-by:
lines from the original patch) add new Signed-off-by:
and comments afterward, but replace the Lustre-commit:
line (which doesn't mean anything in the upstream kernel git) with Intel-bug-id:
{jira_URL}
so that the original bug can still be identified::
and comments afterward, but replace the Lustre-commit:
line (which doesn't mean anything in the upstream kernel git) with Intel-bug-id:
{jira_URL}
so that the original bug can still be identified. When submitting patches upstream, please also follow the Documentation/process/submitting-patches.rst
instructions, and use scripts/get_maintainer.pl
to generate the CC list for the patch. If in doubt, submit the patch only to lustre-devel
first to get feedback from the Lustre maintainers.
No Format |
---|
lustre/llite: simplify dentry revalidate Lustre client dentry validation is protected by LDLM lock, so any time a dentry is found, it's valid and no need to revalidate from MDS, and even it does, there is race that it may be invalidated after revalidation is finished. Signed-off-by: Lai Siyao <lai.siyao@intel.com> Reviewed-on: http://review.whamcloud.com/7475 Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-3544 Signed-off-by: OlegLai DrokinSiyao <oleg<lai.drokin@intelsiyao@intel.com> Signed-off-by: GregOleg Kroah-Hartman <gregkh@linuxfoundation.org> |
...
Drokin <oleg.drokin@intel.com>
|
Author
of the patch should be kept. This should be done automatically when usinggit cherry-pick
but is lost when applying the patch manually, sogit commit --author="Original Author <author@email.com>"
should be usedSigned-off-by:
line of the original author should also be keptSigned-off-by:
line with their name and email following the original oneChange-Id:
line can be kept (newer Gerrit versions can handle the sameChange-Id:
line on different branchesReviewed-by:
lines can be kept, and Gerrit will automatically add them as reviewers to the new patchTested-by: Maloo
andTested-by: Jenkins
lines should be removed from the new commit message, though anyTested-by:
lines from real people can be kept"Reviewed-on: http://review.whamcloud.com/nnnnn"
line should be changed to"Lustre-change: http://review.whamcloud.com/nnnnn"
(please use the "canonicalpermalink" gerrit Gerrit URL format as shown)"cherry picked from commit abcdef1234567890"
line should be changed to"Lustre-commit: abcdef1234567890"
Signed-off-by:
andLustre-commit:
lines before your ownSigned-off-by:
lineSigned-off-by:
andLustre-commit:
andLustre-change:
lines of each commit