Difference between revisions of "Submitting Xen Project Patches"
Edwintorok (talk | contribs) m (→What is in a patch and a patch series?) |
|||
(119 intermediate revisions by 13 users not shown) | |||
Line 1: | Line 1: | ||
+ | This document assumes that you have cloned xen.git (or a similar repository) and that you have some development patches ready to submit. |
||
− | == How To Generate and Submit a Patch to Xen == |
||
+ | If you are not familiar with git and Xen Development branches, read the following documents |
||
+ | * [[Xen Project Repositories]] |
||
+ | * [[Managing Xen Patches with Git]] |
||
+ | * [[Managing Xen Patches with StGit]] |
||
+ | * [[Managing Xen Patches with_Git-series]] |
||
+ | This document outlines the code submission process primarily to the xen-devel@ mailing list. The process does also apply to other mailing list based subprojects: specific information related to these subprojects can be found at the bottom of this document. |
||
− | Xen uses [http://git-scm.com Git] for its source code management. The current Xen development tree is at http://xenbits.xen.org/xen.git; "master" is the branch containing the latest development changesets to pass our automated build and test system. |
||
− | + | = What is in a patch and a patch series? = |
|
− | ---- |
||
− | Here's a recommendation on how to create patches using git, as suggested by the Xen maintainers. If you want to use mercurial, please see [[Submitting_Xen_Patches_-_mercurial]]. |
||
+ | Changes to the Xen Project are submitted to a mailing list either as an individual patch or as a patch series. A patch series is a series of patches that are related and broken into a logical series of individual patches. |
||
− | The instructions on this page use the most common git workflow of branches. Many people find this difficult to work with; one alternative is to use stgit instead; you can find instructions at [[Submitting_Xen_Patches_with_StGit]]. |
||
+ | Patches, if accepted, will turn into commits in the git source tree. However, frequently multiple versions of patches will have to be posted, before the patch is accepted. |
||
− | Begin by cloning the git repo from xen.org: |
||
− | <pre> |
||
− | $ git clone git://xenbits.xen.org/xen.git xen.git |
||
− | </pre> |
||
+ | There are three groups of people to think about when writing the patch and the commit message: |
||
− | At this point you should have <code>xenbits</code> set up as the remote repostiory called "origin": |
||
+ | * Reviewers on the mailing list, who are trying to understand what your patch does, if it's correct, and what side effects it will have. |
||
− | <pre> |
||
+ | * People skimming through changesets deciding if a patch is important for them to look at for backporting, review, implications on out-of-tree patches, &c. |
||
− | $ git branch -a |
||
+ | * Someone in the perhaps distant future, who is trying to understand why the code is the way it is. |
||
− | * master |
||
− | remotes/origin/HEAD -> origin/master |
||
− | remotes/origin/master |
||
− | remotes/origin/stable-4.0 |
||
− | remotes/origin/stable-4.1 |
||
− | remotes/origin/stable-4.2 |
||
− | remotes/origin/staging |
||
− | remotes/origin/staging-4.0 |
||
− | remotes/origin/staging-4.1 |
||
− | remotes/origin/staging-4.2 |
||
− | </pre> |
||
+ | Try to make your patches with all of these people in mind. |
||
− | This process automatically creates a local branch called <code>master</code> that will track the xen.org branch called <code>master</code>. |
||
+ | == What is in a patch? == |
||
− | The branch called <code>staging</code> is the bleeding-edge of commits; this is tested regularly with the <code>xen.org</code> build and regression testing system, and when it pases, is pushed to the <code>master</code> branch. It is suggested to develop patches based on the <code>master</code> branch. |
||
+ | A patch is an e-mail, which is generated from a single commit in your git tree and augmented by using a tool (e.g. git-format, add_maintainers.pl, git-send-email) which may prompt you to edit the final e-mail manually. |
||
+ | It is important to note, that your patch will inherit the following information from the git commit message on which it is based, assuming you recorded the information in the commit message at commit time |
||
− | When you want to introduce a change, start by making a new branch based on the most recent change in the <code>master</code> branch: |
||
+ | * Title and description |
||
+ | * Signed-off-by |
||
+ | * Other *-by tags such as Reported-by |
||
+ | * CC lists of reviewers - these can be added automatically later |
||
+ | * Change log and other information that is useful for the reviewer |
||
+ | {{WarningLeft|More documentation can be found at https://xenbits.xen.org/docs/unstable/process/sending-patches.html including the format of Fixes: lines}} |
||
− | <pre> |
||
− | $ git checkout -b out/trondle-calls master |
||
− | Switched to a new branch 'out/trondle-calls' |
||
− | </pre> |
||
+ | === Title and description of the patch === |
||
− | Then edit the source files you want to change. You can see which files have changed by using <code>git status</code>. |
||
+ | * The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches. It typically contains a prefix, such as '''xen/x86:''', '''xen/arm:''', '''tools:''', etc. Note that maintainers may propose a different prefix or change it at commit-time if it isn't correct |
||
+ | * The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as <code>Code cleanup -- no functional changes</code>, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing. |
||
− | When you're done editing, use <code>git add</code> to specify which file changes you want included in the changeset, and then use <code>git commit</code> to make a commit. You will be prompted to make a changset description: |
||
+ | The description should be wrapped to a maximum of 80 columns, unless it includes an error message which is longer than 80 characters. |
||
− | |||
− | <pre> |
||
− | $ git status |
||
− | # On branch out/trondle-calls |
||
− | # Changes not staged for commit: |
||
− | # (use "git add <file>..." to update what will be committed) |
||
− | # (use "git checkout -- <file>..." to discard changes in working directory) |
||
− | # |
||
− | # modified: foobar/zot.c |
||
− | # modified: foobar/zot.h |
||
− | # |
||
− | no changes added to commit (use "git add" and/or "git commit -a") |
||
− | $ git add foobar/zot.c foobar/zot.h |
||
− | $ git commit |
||
− | </pre> |
||
+ | === Signed-off-by === |
||
− | Alternatively, you can commit all changes using "git commit -a": |
||
+ | All patches to the Xen Project code base must include the line '''Signed-off-by: your_name <your_email>''' at the end of the change description. |
||
− | <pre> |
||
+ | You can use '''-s''' or '''--signoff''' when committing a patch, if you change your default settings as outlined [[#Settings_that_help_save_you_time|here]]. |
||
− | $ git commit -a |
||
− | < |
+ | <br> |
+ | <br> |
||
− | |||
+ | {{WarningLeft|It is important that the '''e-mail address reflects the copyright holder of the code'''. |
||
− | <pre> |
||
− | foobar: Add a new trondle calls |
||
+ | For example, if you work for a company called FOOBAR and developed the code on work-time, your employer owns the copyright for the change. Thus you must use an email address that includes @FOOBAR. If the code was developed on your own time, you can use a private email address.}} |
||
− | Add a some new trondle calls to the foobar interface to support |
||
− | the new zot feature. |
||
+ | This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states: |
||
− | Signed-off-by: Joe Smith <joe.smith@citrix.com> |
||
− | </pre> |
||
− | |||
− | Make as many commits on your new branch as are necessary. |
||
+ | <syntaxhighlight> |
||
− | |||
− | If you want to make a local branch based on <code>staging</code> rather than <code>master</code> (for example, to fix a changeset which has caused the xen.org nightly testing to fail), you can do the following: |
||
− | <pre> |
||
− | $ git checkout -b staging remotes/origin/staging |
||
− | Branch staging set up to track remote branch staging from origin. |
||
− | Switched to a new branch 'staging' |
||
− | </pre> |
||
− | Then in the commands above, you would use "staging" instead of "master" as the base branch. |
||
− | ... |
||
− | === Signing off a patch === |
||
− | ---- |
||
− | All patches to the Xen code base must include the the line "Signed-off-by: your_name <your_email>" at the end of the change description. This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states: |
||
− | |||
− | |||
− | <pre> |
||
Developer's Certificate of Origin 1.1 |
Developer's Certificate of Origin 1.1 |
||
Line 116: | Line 77: | ||
maintained indefinitely and may be redistributed consistent with |
maintained indefinitely and may be redistributed consistent with |
||
this project or the open source license(s) involved. |
this project or the open source license(s) involved. |
||
+ | </syntaxhighlight> |
||
− | </pre> |
||
− | === |
+ | === Other *-by tags === |
+ | You may add the following git tags before or after the signed-off-by tag: |
||
− | ---- |
||
+ | * '''Reported-by:''' is used to credit someone who found the bug that the patch attempts to fix |
||
− | Patches, if accepted, will turn into commits in the git source tree. There are three people to think about when writing the patch and the comment: |
||
+ | * '''Tested-by:''' is used to indicate that the listed person applied the patch and found it to have the desired effect. |
||
− | * Reviewers on the mailing list, who are trying to understand what your patch does, if it's correct, and what side effects it will have. |
||
+ | |||
− | * People skimming through changesets deciding if a patch is important for them to look at for backporting, review, implications on out-of-tree patches, &c. |
||
+ | Later in the review process, your patch or patch series may accumulate tags, such as |
||
− | * Someone in the perhaps distant future, who is trying to understand why the code is the way it is. |
||
+ | * '''Acked-by:''' a maintainer of the patch reviewed the patch and found it is good to go in. |
||
− | Try to make your patches with all of these people in mind. To do this: |
||
+ | * '''Reviewed-by:''' another community member, a designated reviewer or a maintainer of another component reviewed the patch |
||
− | ==== Break down your patches ==== |
||
+ | |||
− | * Each patch should preferably do one logical thing (or one related set of things). The goal is for reviewers to understand the change that each patch is making with a minimum of effort. |
||
+ | Note that if there are several revisions of a patch, you ought to copy tags that have accumulated during the review. For example, if person A and person B added a Reviewed-by: tags to v1 of your patch, include it into v2 of your patch. If you make substantial changes after certain tags were already applied, you will want to consider which ones are no longer applicable (and may require re-providing). |
||
+ | |||
+ | === CC lists of the maintainers or reviewers for the patch === |
||
+ | * Always send a copy of the patch to (via CC) the maintainer of the code you are modifying. The maintainers and reviewers are listed in the [http://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=MAINTAINERS MAINTAINERS] file at the top of the Xen tree. If no maintainer is listed then the maintainers listed under THE REST in the MAINTAINERS file |
||
+ | ** Note that if you follow the steps described in this document, the add_maintainers.pl/get_maintainer.pl scripts will add the correct people automatically |
||
+ | * If any people who were not on the CC list for an earlier review, but have reviewed your patch, you may want to manually add them (via CC) to the CC list. |
||
+ | |||
+ | {{Anchor|extra-info}} |
||
+ | === Change log (for patch revisions >= 2) === |
||
+ | Changes to previous versions of a patch are recorded in the patch commit message after the CC lists. These are required to be very detailed and will help the reviewer to check whether changes have been addressed. An example of a change log can be found below (the actual patch for this example is [https://lists.xenproject.org/archives/html/xen-devel/2019-08/msg00548.html here]). |
||
+ | |||
+ | <syntaxhighlight> |
||
+ | Changes in v4: |
||
+ | - ARM -> Arm |
||
+ | - libxl__memory_policy_to_xc -> libxl__arch_memory_policy_to_xc |
||
+ | - keep Arm policies together |
||
+ | |||
+ | Changes in v3: |
||
+ | - s/nGRE/nGnRE/g |
||
+ | - s/LIBXL_MEMORY_POLICY_ARM_DEV_NGRE/LIBXL_MEMORY_POLICY_ARM_DEV_NGNRE/g |
||
+ | - s/arm_devmem/arm_dev_nGnRE/g |
||
+ | - s/arm_memory/arm_mem_WB/g |
||
+ | - improve commit message |
||
+ | - improve man page |
||
+ | - s/MEMORY_POLICY_X86_UC/MEMORY_POLICY_X86_UC_MINUS/g |
||
+ | - s/x86_uc/x86_UC_minus/g |
||
+ | - move security support clarification to a separate patch |
||
+ | |||
+ | Changes in v2: |
||
+ | - add #define LIBXL_HAVE_MEMORY_POLICY |
||
+ | - ability to part the memory policy parameter even if gfn is not passed |
||
+ | - rename cache_policy to memory policy |
||
+ | - rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE |
||
+ | - rename MEMORY_POLICY_MEMORY to MEMORY_POLICY_ARM_MEM_WB |
||
+ | - rename memory to arm_memory and devmem to arm_devmem |
||
+ | - expand the non-security support status to non device passthrough iomem |
||
+ | configurations |
||
+ | - rename iomem options |
||
+ | - add x86 specific iomem option |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | === Information that is useful for the reviewer, which you do not want in the final commit messages === |
||
+ | You can manually add additional information that you do not want to appear in the commit message, after the maintainers list. An example can be found in [https://xen.markmail.org/thread/vpkla55fyywp7xe7 this patch]. |
||
+ | |||
+ | === A version number and subject prefix === |
||
+ | Each patch contains a revision number, or a subject prefix. For example, a patch email may contain '''[RFC PATCH v2]''' or '''[PATCH v3 4/11]''' indicating that the patch is part of a patch series. |
||
+ | |||
+ | === Example patch === |
||
+ | The example patch [https://xen.markmail.org/thread/6lirsu5bias755qr here] that resulted in the following [http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=3d2010f9ffeacc8836811420460e15f2c1233695 commit]. |
||
+ | |||
+ | <syntaxhighlight lang="diff" highlight="1,17,18,20,21" line> |
||
+ | vif-common.sh: Have iptables wait for the xtables lock permalink |
||
+ | |||
+ | iptables has a system-wide lock on the xtables. Strangely though, in |
||
+ | the case of two concurrent invocations, the default is for the |
||
+ | instance not grabbing the lock to exit out rather than waiting for it. |
||
+ | This means that when starting a large number of guests in parallel, |
||
+ | many will fail out with messages like this: |
||
+ | |||
+ | 2017-05-10 11:45:40 UTC libxl: error: libxl_exec.c:118: libxl_report_child_exitstatus: /etc/xen/scripts/vif-bridge remove [18767] exited with error status 4 |
||
+ | 2017-05-10 11:50:52 UTC libxl: error: libxl_exec.c:118: libxl_report_child_exitstatus: /etc/xen/scripts/vif-bridge offline [1554] exited with error status 4 |
||
+ | |||
+ | In order to instruct iptables to wait for the lock, you have to |
||
+ | specify '-w'. Unfortunately, not all versions of iptables have the |
||
+ | '-w' option, so on first invocation check to see if it accepts the -w |
||
+ | command. |
||
+ | |||
+ | Reported-by: Antony Saba <aws...@gmail.com> |
||
+ | Signed-off-by: George Dunlap <geor...@citrix.com> |
||
+ | --- |
||
+ | Cc: Ian Jackson <ian....@citrix.com> |
||
+ | Cc: Wei Liu <wei....@citrix.com> |
||
+ | --- |
||
+ | tools/hotplug/Linux/vif-common.sh | 38 +++++++++++++++++++++++++++++++++++--- |
||
+ | 1 file changed, 35 insertions(+), 3 deletions(-) |
||
+ | |||
+ | diff --git a/tools/hotplug/Linux/vif-common.sh |
||
+ | b/tools/hotplug/Linux/vif-common.sh |
||
+ | index 6e8d584..29cd8dd 100644 |
||
+ | --- a/tools/hotplug/Linux/vif-common.sh |
||
+ | ... |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | * '''Lines 1 to 15:''' A good patch description answers the following questions |
||
+ | ** What is the situation |
||
+ | ** Why is the current situation a problem |
||
+ | ** How does this patch fix the problem |
||
+ | ** Anything else the reviewers need to know to understand the patch |
||
+ | * '''Lines 17 to 18:''' Contains the *by tags including the mandatory '''Signed-off-by''' tag |
||
+ | * '''Lines 20 to 21:''' Contains CC statements of the maintainers. Note that these can be added manually, but if you follow the steps below these will be added automatically |
||
+ | * '''Before Line 22:''' added in lines before the <code>---</code> |
||
+ | ** Change log of the patch (see [[#extra-info|here]] for an example) |
||
+ | ** Any extra information as outlined [[#extra-info|here]] |
||
+ | * '''After Line 22:''' This is the actual code patch |
||
+ | |||
+ | <br> |
||
+ | |||
+ | == What is in a patch series? == |
||
+ | A patch series is a sequence of threaded emails, which are generated from multiple git commits (typically the last '''N''' ones) in your git tree and augmented by using a tool (e.g. <code>git-format</code>, <code>add_maintainers.pl</code>, <code>git-send-email</code>) which may prompt you to edit the final email manually. |
||
+ | |||
+ | Patch series can be anything from bug fixes or small improvements that are too big for an individual patch or encompass different components (e.g. hypervisor, toolstack change and a docs change) to new features with 10s of patches spanning multiple components. |
||
+ | |||
+ | In addition to the individual patches, which describe the patches making up the series, a patch series '''must contain a cover letter''' as the first email in the series. If you want any of the automated tooling which we use on xen-devel@ to work, the '''patch series must be threaded''', with the cover letter the parent and each individual patch being a child of the cover letter. |
||
+ | |||
+ | === Break down your patches appropriately === |
||
+ | * Each patch in a patch series should preferably do one logical thing (or one related set of things). The goal is for reviewers to understand the change that each patch is making with a minimum of effort. |
||
* No patch should ever break existing functionality. |
* No patch should ever break existing functionality. |
||
* Never fix bugs in one of your patches by changing it in a later patch; go back and change the patch with the bug in it. |
* Never fix bugs in one of your patches by changing it in a later patch; go back and change the patch with the bug in it. |
||
− | * Don't mix clean-up patches (which make things look prettier or |
+ | * Don't mix clean-up patches (which make things look prettier or refactor but don't change functionality) with code-change patches. However, e.g. style corrections on lines changed anyway are generally accepted (and often welcomed). Clean-up patches should be clearly marked as having no functional changes. |
− | ==== Write a good description for each patch ==== |
||
− | * The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches |
||
− | * The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as <code>Code cleanup -- no functional changes</code>, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing. |
||
− | * The description should be wrapped to a maximum of 80 columns. |
||
+ | === Cover letter === |
||
− | ==== Cc the maintainer of the code you are modifying ==== |
||
+ | Cover letters are an important part of a patch series. The Xen Project has high standards when it comes to large and complex series. The cover letter should cover the following information |
||
+ | * It should contain an explanation of what you are trying to achieve |
||
+ | * It should explain as to why the series is needed (if it is not obvious) |
||
+ | * How the series achieves your goal |
||
+ | * It should list any pre-conditions and major assumptions you have made |
||
+ | * It should refer to any specifications, design documents, previous discussion on mailing lists and other relevant information |
||
+ | |||
+ | The amount of detail required depends on the complexity and size of the patch series. The larger and more complex the series, the more information is required. The goal of the information is to make it easier for the code reviewer to understand your series. The more time you invest in preparing the series’ cover letter, the easier it will be for a reviewer to understand your code. |
||
+ | However, '''cover letters do not get committed to the git tree''': if the cover letter contains information that should be in the git tree, you may want to create a new document under [http://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=docs/designs docs/designs], [http://xenbits.xenproject.org/gitweb/?p=xen.git;a=tree;f=docs/features docs/features] or [http://xenbits.xenprojectorg/gitweb/?p=xen.git;a=tree;f=docs/specs docs/specs] instead as part of the series and refer to it from the series. |
||
− | * Please always send a copy of the patch to the maintainer of the code you are modifying. |
||
− | * The maintainers are listed in the MAINTAINERS file at the top of the Xen source tree. If no maintainer is listed then there is no need for a CC (if you are modifying code with no maintainer then you might like to consider becoming the maintainer for that piece of code!) |
||
− | * In addition to CCing the maintainer you should always send patches to the xen-devel@lists.xen.org mailing list as well. |
||
− | * To add a CC when sending the mail you can use the ''--cc'' option to the ''git send-email'' command, or you can do it manually in your MUA. |
||
− | === |
+ | === Change log (for patch series revisions >= 2) === |
+ | The end of the cover letter is also the location where you would typically track changes between different versions of the patch series. The format is identical to change logs of individual patches (see [[#extra-info|here]]), but a series change log is much less detailed and more high-level than for a patch. The key purpose is to point reviewers to areas of the series which need to be revisited. |
||
− | ---- |
||
+ | |||
+ | === Providing a git branch === |
||
+ | |||
+ | If you send a large patch series, it is recommended to include a branch in your public git repository in your cover letter, so that reviewers can pull directly from that branch. |
||
+ | |||
+ | If you do not have a public repository, you can fork the Xen Project’s GitHub or GitLab mirrors at https://github.com/xen-project/xen and https://gitlab.com/xen-project/xen into your own project space and push your branch to your own fork and provide a link to that branch. If you are a regular contributor to the Xen Project, you can request to host personal repositories at xenbits by sending an email to community dot manager at xenproject dot org. |
||
+ | |||
+ | {{Anchor|send}} |
||
+ | |||
+ | = Sending the patches to the list = |
||
The xen-devel mailing list is moderated for non-subscribers. It is not mandatory to subscribe but it can help avoid this delay. It is possible to subscribe and then disable delivery in the mailman options so as to avoid moderation but not receive list traffic if that is what you would prefer. |
The xen-devel mailing list is moderated for non-subscribers. It is not mandatory to subscribe but it can help avoid this delay. It is possible to subscribe and then disable delivery in the mailman options so as to avoid moderation but not receive list traffic if that is what you would prefer. |
||
+ | == Setting up git send-email == |
||
− | ==== Git Patchbomb Extension ==== |
||
The most robust way to send files is by using the [https://www.kernel.org/pub/software/scm/git/docs/git-send-email.html git send-email command]. (If you're using mercurial, please see our [[Submitting_Xen_Patches_-_mercurial]]). |
The most robust way to send files is by using the [https://www.kernel.org/pub/software/scm/git/docs/git-send-email.html git send-email command]. (If you're using mercurial, please see our [[Submitting_Xen_Patches_-_mercurial]]). |
||
To do this, first set configure your information: |
To do this, first set configure your information: |
||
+ | <syntaxhighlight lang="sh" highlight="1,2,3,5"> |
||
− | <pre> |
||
git config --global sendemail.from "YOUR NAME <user@example.org>" |
git config --global sendemail.from "YOUR NAME <user@example.org>" |
||
git config --global sendemail.smtpserver imap.example.org |
git config --global sendemail.smtpserver imap.example.org |
||
Line 157: | Line 235: | ||
# depending on your config you may also set: |
# depending on your config you may also set: |
||
git config --global sendemail.smtpencryption tls |
git config --global sendemail.smtpencryption tls |
||
+ | </syntaxhighlight> |
||
− | </pre> |
||
If you don't want to enter password for your SMTP server all the time: |
If you don't want to enter password for your SMTP server all the time: |
||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
− | <pre> |
||
git config --global sendemail.smtppass = PASS |
git config --global sendemail.smtppass = PASS |
||
+ | </syntaxhighlight> |
||
− | </pre> |
||
+ | == Settings that help save you time == |
||
− | Then check to make sure you have the right patches to be submitted by using <code>git </code>: |
||
+ | There are a number of settings you may want to set globally or per repository, such as |
||
− | |||
+ | <syntaxhighlight lang="sh" highlight="2,5,6"> |
||
− | <pre> |
||
+ | # Allows you to drop --thread from git format-patch ... |
||
− | $ git log master.. |
||
+ | git config --global format.thread true |
||
− | commit 347175ecaba64c324853f004e4860a941b07e5a9 |
||
− | Author: Joe Smith <joe.smith@citrix.com> |
||
+ | # Allows you to use -s or --signoff when committing a patch |
||
− | foobar: Add new trondle calls |
||
+ | git config --global user.name "YOUR NAME" |
||
+ | git config --global user.email "<user@example.org>" |
||
+ | </syntaxhighlight> |
||
+ | == Subject Prefix == |
||
+ | |||
+ | Patches and patch series are posted to a mailing list: when using the <code>git format-patch</code> command a subject prefix is added to a patch or patch series, which helps disambiguate normal discussions from posted patches and code reviews. |
||
+ | |||
+ | <code>git format-patch</code> creates mails with subject lines such as |
||
+ | <syntaxhighlight lang="sh" highlight="2, 4,5,6,7"> |
||
+ | # Single patch |
||
+ | [<SUBJECT-PREFIX>] <Title of patch in git> |
||
+ | # Patch series |
||
+ | [<SUBJECT-PREFIX> 0/n] <Title of cover letter/patch series> |
||
+ | [<SUBJECT-PREFIX> 1/n] <Title of 1st patch in git> |
||
... |
... |
||
+ | [<SUBJECT-PREFIX> n/n] <Title of nth patch in git> |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | === Canonical Subject Prefix === |
||
+ | As patches for different git repositories can be posted (or CC'ed) to different mailing lists, some conventions have emerged over time which help code reviewers identify which patch is intended for which git tree. We refer to these as '''canonical subject prefix'''. The table below shows recommended default Subject Prefixes, which either |
||
+ | * need to be passed using the <code>--subject-prefix=<Subject-Prefix></code> option when calling <code>[[#Step_1:_Create_patches_using_git_format-patch|git format-patch]]</code>, |
||
+ | * or can be set as defaults per git repository as described [[#Changing_git_defaults_to_use_the_Canonical_Subject_Prefix|here]] |
||
+ | |||
+ | The following table lists common '''canonical subject prefix'''es used in the project |
||
+ | {| class="wikitable" |
||
+ | ! repo |
||
+ | ! Subject Prefix |
||
+ | ! List |
||
+ | |- |
||
+ | | git default |
||
+ | | <code>PATCH</code> |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=xen.git;a=summary xen.git] |
||
+ | | <code>PATCH</code><br>or <code>XEN PATCH</code> (recommended) |
||
+ | | xen-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=osstest.git;a=summary osstest.git] |
||
+ | | <code>OSSTEST PATCH</code> |
||
+ | | xen-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=xtf.git;a=summary xtf.git] |
||
+ | | <code>XTF PATCH</code> |
||
+ | | xen-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=xenalyze.git;a=summary xenalyze.git] |
||
+ | | <code>XENALYZE PATCH</code> |
||
+ | | xen-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=livepatch-build-tools.git;a=summary livepatch-build-tools.git] |
||
+ | | <code>LIVEPATCH-BUILD-TOOLS PATCH</code> |
||
+ | | xen-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=mini-os.git;a=summary mini-os.git] |
||
+ | | <code>PATCH</code><br>or <code>MINI-OS PATCH</code> (recommended) |
||
+ | | minios-devel@ but sometimes xen-devel@ is CC'ed |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?p=unikraft/unikraft.git;a=summary unikraft/unikraft.git] |
||
+ | | <code>UNIKRAFT PATCH</code> |
||
+ | | minios-devel@ |
||
+ | |- |
||
+ | | [http://xenbits.xen.org/gitweb/?a=project_list;pf=unikraft unikraft/.../<REPO>.git] |
||
+ | | <code>UNIKRAFT/<REPO> PATCH</code> |
||
+ | | minios-devel@ |
||
+ | |} |
||
+ | |||
+ | <span style="color: red">'''IMPORTANT''': If you do not use a '''canonical subject prefix''' some existing and future automated tooling may not work correctly:</span> consider for example a situation in which a patch for livepatch-build-tools.git is sent to xen-devel@ and that this triggers a build test in different environments. In this case, the build test would try and apply the patch to xen.git and might fail in unexpected ways. |
||
+ | <br> |
||
+ | |||
+ | |||
+ | === Common modifications to the Subject Prefix === |
||
+ | In addition, the following modifications to Subject Prefixes are commonly used and can also be used in combination |
||
+ | {| class="wikitable" |
||
+ | ! Subject Prefix Modification |
||
+ | ! Example |
||
+ | ! When to use |
||
+ | |- |
||
+ | | <code>'''RFC''' ...</code> |
||
+ | | <code>'''RFC''' XTF PATCH</code> |
||
+ | | RFC means "Request For Comments"; use this when sending an experimental patch for discussion rather than application. |
||
+ | |- |
||
+ | | <code>... '''RESEND'''</code> |
||
+ | | <code>XTF PATCH '''RESEND'''</code> |
||
+ | | Typically used if your patch has not been reviewed for some time and you want to remind people of the patch |
||
+ | |- |
||
+ | | <code>... '''for-<release>'''</code> |
||
+ | | <code>XEN PATCH '''for-4.13'''</code> |
||
+ | | Typically used '''after the master xen.git branch has been feature frozen''' and you want to highlight a patch that is destined for the release which is currently being developed |
||
+ | |- |
||
+ | | <code>... '''for-next'''</code> |
||
+ | | <code>XEN PATCH '''for-next'''</code> |
||
+ | | Typically used '''towards the end of a development cycle''' when you want to highlight a patch that is destined for the '''next''' release |
||
+ | |} |
||
+ | |||
+ | <code>[[#Step_1:_Create_patches_using_git_format-patch|git format-patch]]</code> contains options to help with these |
||
+ | * <code>--rfc</code> will mark a patch as RFC, but will '''always''' lead to <code>RFC PATCH</code> |
||
+ | * <code>--subject-prefix=<Subject-Prefix></code> allows you to set a subject prefix, but will overwrite '''any defaults''' you may have set |
||
+ | |||
+ | <br> |
||
+ | |||
+ | === Changing git defaults to use the Canonical Subject Prefix === |
||
+ | To change the setting perform |
||
+ | <syntaxhighlight lang="sh" highlight="2"> |
||
+ | cd <local-git-tree> |
||
+ | git config --local format.subjectPrefix "<canonical subject prefix>" |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | == Sending a Patch Series == |
||
+ | |||
+ | === '''Step 1''': Create patches using '''git format-patch''' === |
||
+ | |||
+ | [https://git-scm.com/docs/git-format-patch Git format patch] allows you to create and save formatted patches in a directory. By default it will create them in the root of your git directory, but you probably want to direct this into a <code>../patches/feature-version</code> directory (in the examples below <code>../patches/feature-v2</code> would contain only v2 of that series). It is also possible to store several versions of a patch, e.g. v1, v2, etc in the same <code>../patches/feature</code> directory). Let's say the last two commits of your head are part of your series you want to send out. In this case, the command line would look like |
||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ git format-patch --reroll-count=2 --thread --cover-letter -o ../patches/feature-v2 -2 |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | This will create 3 files, such as |
||
+ | <pre> |
||
+ | v2-0000-cover-letter.patch |
||
+ | v2-0002-Patch-to-do-bar.patch |
||
+ | v2-0001-Patch-to-do-foo.patch |
||
</pre> |
</pre> |
||
+ | Notes |
||
− | Next you need to figure out who (if anyone) you should copy the patches to. To do this you should consult the <code>MAINTAINERS</code> file at the top of the Xen source tree. If there is a specific maintainer listed for the area of the code you are modifying then it is best to copy the patches to them. You should always send patches to the xen-devel@lists.xen.org mailing list as well. |
||
+ | * You will need to edit the subject and body of <code>v2-0000-cover-letter.patch</code> |
||
+ | * You must always use the '''--thread''' and '''--cover-letter''' options. If you omit '''--thread''', any automatic tooling and patch checking that is triggered by sending a mail to one of our mailing lists may not work without using this option. You can set '''--thread''' as a default, as outlined [[#Settings_that_help_save_you_time|here]]. |
||
+ | Other useful options include: |
||
− | Once you're sure you have the proper series of patches and know who you are sending it to then you're ready to use <code>git send-email</code>: |
||
+ | * <code>--rfc</code> and <code>--subject-prefix=Subject-Prefix</code>: please read [[#Subject_Prefix|this document]] before using these options |
||
− | |||
+ | |||
+ | <br> |
||
+ | |||
+ | === '''Step 2''': Use '''[http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=scripts/add_maintainers.pl add_maintainers.pl]''' (or '''get_maintainer.pl''') === |
||
+ | |||
+ | '''Option 1''': use '''[http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=scripts/add_maintainers.pl add_maintainers.pl]''' |
||
+ | |||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ ./scripts/add_maintainers.pl -d ../patches/feature-v2 |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | {{Warning|If you are preparing a security patch it is best not to use this script to avoid accidentally sending security patches to xen-devel if you forget --suppress-cc from git-send-email}} |
||
+ | |||
+ | Then follow the instructions (which are essentially '''Step 3''', with correct command line options based on what you pass to <code>add_maintainers.pl</code>). Note that the <code>add_maintainers.pl</code> script works around a limitation of <code>git send-email ... --cc-cmd="./scripts/get_maintainer.pl" ...</code>, which does not automatically update the CC list of your cover letter. |
||
+ | |||
+ | Other useful options include: |
||
+ | * <code>--reroll-count|-v</code> (e.g. <code>-v 2</code>): If you store your patches in '''one directory''' with different versions in it generated by <code>--reroll-count=2</code> you want to use this option |
||
+ | * <code>--patchcc|-p LOCATION</code>: Inserts CC's into a specific location (see <code>--help</code> ) to *.patch files |
||
+ | * <code>--covercc|-c LOCATION</code>: Inserts CC's into a specific location (see <code>--help</code> ) to the cover letter |
||
+ | * <code>--tags|-t</code>: Adds people who have *-by tags to the mail header (<code>git send-email</code> does not do this itself) |
||
+ | * <code>--tagscc</code>: Adds people who have *-by tags to the CC list |
||
+ | * <code>--arg|-a</code> (e.g. <code>-a "<argument1 with space>" -a <argument2> ...</code> for arguments you want to pass to <code>./scripts/get_maintainer.pl</code>) |
||
+ | Common LOCATION combinations include: |
||
+ | * <code>-p commit</code>: copy CC blocks into the *.patch body (CC's will become part of the commit message) |
||
+ | * <code>-p none -c header</code>: copy CC blocks from *.patch files in the cover letter without modifying *.patch files. This is useful in rerolled patches, where you do not want to modify the CC blocks. <code>-p ccbody</code> behaves similarly, only that any new CCs that may come from an updated MAINTAINERS file or from new files that have been added to *.patch files will be added to *.patch files. |
||
+ | * <code>-p comment -c end</code>: copy CC blocks after the *.patch body (CC's will not be committed) and into the body of the cover letter |
||
+ | |||
+ | <br> |
||
+ | '''Option 2''': use '''get_maintainer.pl''' manually |
||
<pre> |
<pre> |
||
+ | Foreach <v2 patchfile> in ../patches/feature (with XXXX > 0000): |
||
− | $ git send-email --to xen-devel@lists.xen.org --cc the.maintainer@example.com master.. |
||
+ | - Run: ./scripts/get_maintainer.pl <v2 patchfile> |
||
+ | - Review the e-mail list and prefix each line with Cc: |
||
+ | - Copy the e-mail CC block into patchfile |
||
+ | - Do anything else you want to do manually |
||
+ | |||
+ | For v2-0000-cover-letter.patch take the superset of all the files generated previously and merge, then copy the e-mail CC block into 0000-cover-letter.patch |
||
</pre> |
</pre> |
||
+ | === '''Step 3''': Send patches using '''git send-email''' === |
||
− | <code>--to</code> and <code>--cc</code> say who to send the mail to. <code>master..</code> will tell it to send all changesets on the current branch *after* (not including) the one marked by <code>master</code>. |
||
+ | Send the patches using |
||
− | NB, emacs users: if you're running this from an emacs shell, this can get tricky, since all of the editors it wants to offer you require a terminal. The best option is probably to enable server mode and then set [http://www.emacswiki.org/emacs/EmacsClient emacsclient] as the default editor. |
||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ git send-email --to xen-devel@lists.xenproject.org ../patches/feature-v2/*.patch |
||
+ | </syntaxhighlight> |
||
+ | Other useful options include: |
||
− | Tada! You should shortly see your patch series appear in your inbox, and on the xen-devel mailing list (depending on how you've configured it.) |
||
+ | * <code>--dry-run</code> |
||
+ | * <code>--cc</code> if you need to CC additional reviewers (e.g. from within your team) |
||
+ | |||
+ | <br> |
||
+ | |||
+ | {{Warning|Be careful if you are sending security patches then DO NOT send them to xen-devel, but to the security team directly, |
||
+ | and avoid using the above add maintainers script. |
||
+ | |||
+ | A safer way to send security patches is: |
||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ git send-email --to security@xenproject.org --suppress-cc=all ../patches/feature-v2/*.patch |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | }} |
||
+ | |||
+ | === Submitting patches against point releases, such as Xen 4.10 === |
||
+ | When you develop against older Xen versions you are likely going to use an outdated version of the MAINTAINERS file. Typically only the files on staging or master are up-to-date. To work around this, use the following slightly modified workflow |
||
+ | <syntaxhighlight lang="sh" highlight="2,4"> |
||
+ | $ git format-patch ... -o <patchdir> ... |
||
+ | $ checkout master |
||
+ | $ ./scripts/add_maintainers.pl -d <patchdir> |
||
+ | $ checkout <original branch> |
||
+ | $ git send-email ... <patchdir> |
||
+ | </syntaxhighlight> |
||
+ | This ensures that you use the latest set of tools and the latest MAINTAINERS file |
||
+ | |||
+ | == Sending an individual patch == |
||
+ | |||
+ | You can use the same workflow as outlined in the previous section, without generating a cover letter. In this case, the <code>git-format-patch</code> command line in step 1 would look like |
||
+ | |||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ git format-patch --thread -o ../patches/bugfix-v2 -1 |
||
+ | </syntaxhighlight> |
||
+ | |||
+ | This is followed by steps 2 and 3. However, for single patches without a cover letter, using <code>git-send-email</code> alone, is quite a reasonable option. In this case, you can use the following command line, which will get the CC list from the <code>./scripts/get_maintainer.pl</code> script, which allows you to fold all 3 steps into 1. |
||
+ | |||
+ | <syntaxhighlight lang="sh" highlight="1"> |
||
+ | $ git send-email --to xen-devel@lists.xenproject.org --cc-cmd="./scripts/get_maintainer.pl" -1 |
||
+ | </syntaxhighlight> |
||
Other useful options include: |
Other useful options include: |
||
* <code>--dry-run</code> will go through all the motions of preparing the patchbomb, but instead of sending a mail, will just output the mails it would have sent. Useful for testing. |
* <code>--dry-run</code> will go through all the motions of preparing the patchbomb, but instead of sending a mail, will just output the mails it would have sent. Useful for testing. |
||
+ | * <code>--cc</code> if you need to CC additional reviewers (e.g. from within your team) |
||
− | * <code>--subject-prefix</code> will allow you to change the prefix from <code>[PATCH]</code> to something else. Examples may include <code>--subject-prefx="PATCH RFC"</code> if you just want to get feedback on a patch series (Request For Comments), or <code>--subject-prefix="PATCH v3"</code> for the 3rd version of your patch series. |
||
+ | * <code>--reroll-count=N</code> allows you to change the revision of the patch |
||
+ | * <code>--rfc</code> and <code>--subject-prefix=Subject-Prefix</code> allow you to change the subject prefix: please read [[#Subject_Prefix|this document]] before using these options |
||
+ | <br> |
||
+ | == Using '''[http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=scripts/add_maintainers.pl add_maintainers.pl]''' (or '''get_maintainer.pl''') from outside of xen.git == |
||
− | ==== Sending Patches Manually ==== |
||
+ | You can use <code>add_maintainers.pl</code> or <code>get_maintainer.pl</code> on any Xen Project git repository with a compatible <code>MAINTAINERS</code> file in the root of its tree. An example is <code>livepatch-build-tools.git</code>. In this case simply replace <code>./scripts/add_maintainers.pl</code> or <code>./scripts/get_maintainer.pl</code> with the full path to the script. |
||
+ | <syntaxhighlight lang="sh" highlight="3"> |
||
− | It is strongly recommended to use the git send-email extension discussed above. However it is possible to send a patch manually using your regular mail client. |
||
+ | # You are in the xen.git sister repository (e.g. livepatch-build-tools) |
||
+ | $ git format-patch ... -o <patchdir> ... |
||
+ | $ $LOCATION-OF-XEN-GIT/scripts/add_maintainers.pl -d <patchdir> |
||
+ | $ git send-email ... <patchdir> |
||
+ | </syntaxhighlight> |
||
+ | A minimum template for such a <code>MAINTAINERS</code> file can be found below. |
||
− | You should be aware that many mail clients have a tendency to mangle the whitespace of a patch making it impossible to apply. It is highly recommended to read Linux's [http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt;hb=HEAD email-clients.txt] for advice on how to avoid this in popular mail clients. |
||
+ | <syntaxhighlight lang="sh"> |
||
− | === Review, Rinse & Repeat === |
||
+ | This file follows the same conventions as outlined in |
||
− | ---- |
||
+ | xen.git:MAINTAINERS. Please refer to the file in xen.git |
||
+ | for more information. |
||
+ | |||
+ | THE REST |
||
+ | M: MAINTAINER1 <maintainer1@email.com> |
||
+ | M: MAINTAINER2 <maintainer2@email.com> |
||
+ | L: xen-devel@lists.xenproject.org |
||
+ | S: Supported |
||
+ | F: * |
||
+ | F: */ |
||
+ | V: xen-maintainers-1 |
||
+ | </syntaxhighlight> |
||
+ | <br> |
||
+ | |||
+ | = Review, Rinse & Repeat = |
||
After posting your patches you will hopefully see some response in the form of comments, patch review and eventually commit. |
After posting your patches you will hopefully see some response in the form of comments, patch review and eventually commit. |
||
+ | == Code Review == |
||
The form of the review may often be quite direct and to the point which may be daunting for some people. However bear in mind that detailed criticism of a patch usually means that the reviewer is interested in your patch and wants to see it go in! |
The form of the review may often be quite direct and to the point which may be daunting for some people. However bear in mind that detailed criticism of a patch usually means that the reviewer is interested in your patch and wants to see it go in! |
||
− | Once you have addressed any review you should normally resend the patch. It is normally expected that you address all review before reposting. This often means code changes in your patch but could also mean simply responding to the review comments explaining you reasoning or giving reasons why something should be the way it is. |
+ | Once you have addressed any review you should normally resend the patch. It is normally expected that you '''address all review comments''' before reposting. This often means code changes in your patch but could also mean simply responding to the review comments explaining you reasoning or giving reasons why something should be the way it is. |
+ | You should also '''rebase your change against the project's development tree before sending out a new version''', such that when it is approved it applies cleanly. The only reason not to do that is if you don't expect it to be approved, and want feedback on what you have. In that case you can do the rebase later. |
||
− | When resending a patch you should normally include a note of what has changed since last time in the message. Common practice is to include these notes after your Signed-off-by separated by a triple-dash ("---"). This indicates patch commentary specific to the posting which need not be included in the final changelog (although you should also remember to update the changelog if necessary). You should also including a "V2" (V3, V4 etc) tag in the subject line (if you are using the git <code>send-email</code> command then the <code>--subject-prefix</code> option is helpful here). |
||
+ | The relevant command line options in <code>git-format-patch</code>, <code>./scripts/add_maintainers.pl</code> and <code>git-send-email</code> (if used standalone) for reposting new revisions is |
||
− | If someone replies to your patch with a tag of the form "Acked-by: <Developer>", "Reviewed-by:", "Tested-by:" etc then, assuming you have not completely reworked the patch, you should include these tags in any reposting after your own Signed-off-by line. This lets people know that the patch has been seen and that someone approves of it and also serves to prevent reviewers wasting time re-reviewing a patch which is not significantly different to last time. The developers with commit access also like to see postings with such tags since it means they are likely to be much easier to deal with and commit. |
||
+ | * <code>--reroll-count|-v</code> |
||
+ | == Highlight changes in the new version == |
||
+ | When resending a patch you should normally include a note of the changes between the current and last version of the patch. Common practice is to include these notes after your Signed-off-by separated by a triple-dash (<code>---</code>). This indicates patch commentary specific to the posting which need not be included in the final changelog (although you should also remember to update the changelog if necessary). You should also include a "V2" (V3, V4 etc) tag in the subject line (if you are using the git <code>send-email</code> command then the <code>--reroll-count=N</code> option is helpful here, or for older git versions <code>--subject-prefix='PATCH vN'</code>). |
||
+ | |||
+ | == Update tags == |
||
+ | If someone replies to your patch with a tag of the form '''Acked-by: <Developer>''', '''Reviewed-by:''', '''Tested-by:''' etc then, assuming you have not significantly reworked the patch, you should include these tags in any reposting after your own Signed-off-by line. This lets people know that the patch has been seen and that someone approves of it and also serves to prevent reviewers wasting time re-reviewing a patch which is not significantly different to last time. The developers with commit access also like to see postings with such tags since it means they are likely to be much easier to deal with and commit. |
||
+ | |||
+ | == An example of a new Patch Version == |
||
An example of a resend of the example patch from above might be: |
An example of a resend of the example patch from above might be: |
||
+ | <syntaxhighlight lang="diff" highlight="7, 10, 11, 12"> |
||
− | <pre> |
||
Subject: [PATCH v2] foobar: Add a new trondle calls |
Subject: [PATCH v2] foobar: Add a new trondle calls |
||
Line 233: | Line 541: | ||
+++ b/tools/libxc/Makefile Mon Dec 21 11:45:00 2009 +0000 |
+++ b/tools/libxc/Makefile Mon Dec 21 11:45:00 2009 +0000 |
||
... |
... |
||
+ | </syntaxhighlight> |
||
− | </pre> |
||
+ | == Resending Patches == |
||
− | If you do not get any response to your patch or you got lots of Acked-by's but the patch has not been committed (remember that reviewers and maintainers are busy people too and sometimes things may fall through the cracks) then after some time, perhaps 2-4 weeks ([http://blog.xen.org/index.php/2009/09/16/submitting-patches-to-xen-org-community/ guidelines]), you should resend the patch, perhaps including [RESEND] in the subject line to alert people that the last mail was dropped. Before resending it may be worth considering if there is anything you can do to improve the commit message or subject line of your patch to better attract the attention of the relevant people. Remember to include any Acked-by/Reviewed-by which you received in response to the previous post. |
||
+ | If you do not get any response to your patch or you got lots of Acked-by's but the patch has not been committed (remember that reviewers and maintainers are busy people too and sometimes things may fall through the cracks) then after some time, perhaps 2-4 weeks ([http://blog.xen.org/index.php/2009/09/16/submitting-patches-to-xen-org-community/ guidelines]), you should resend the patch, perhaps including [RESEND] in the subject line to alert people that the last mail was dropped. Before resending you should: |
||
+ | * Check that the patch has not been applied to the staging branch, since committers do not always send a notification when they apply a patch. |
||
− | === What If === |
||
+ | * Consider if there is anything you can do to improve the commit message or subject line of your patch to better attract the attention of the relevant people. |
||
− | ---- |
||
− | * Your patch is really big? |
||
− | ** Break it up into smaller logically distinct patches |
||
− | ** Example - http://markmail.org/thread/e36vcwk3347de27n |
||
− | * You have lots and lots of patches? If you are going to generate > 20 patches a week: |
||
− | ** You might want to host a repository tree that the maintainers can pull from, talk to the maintainers |
||
+ | Remember to include any Acked-by/Reviewed-by which you received in response to the previous post. |
||
− | === After your patch is committed === |
||
+ | == How to know when a patch has been committed == |
||
− | Changes committed to Xen by the committers are immediately available in the "staging" branch of the main xen.git tree. They are then automatically tested, and if the tests pass the changes are propagated to the "master" branch. |
||
+ | Changes committed to Xen Project by the committers are immediately available in the "staging" branch of the main xen.git tree. They are then automatically tested, and if the tests pass the changes are propagated to the "master" branch. |
||
− | If your patch turns out to break something you will be expected to respond promptly to help diagnose and fix the problem. If it can't be fixed quickly, your change may be reverted. |
||
+ | == After your patch is committed == |
||
− | [[Category:Developers]] |
||
− | [[Category:Development Process]] |
||
− | [[Category:Xen]] |
||
+ | If your patch turns out to break something you will be expected to respond promptly to help diagnose and fix the problem. This assumes that you in particular keep an eye on the osstest flight reports sent to xen-devel. If it can't be fixed quickly, your change may be reverted. |
||
− | == How To Generate and Submit a Patch to Qemu-Xen == |
||
+ | = Workflow for related repositories and projects = |
||
− | Xen uses QEMU as device model, that is the software component that takes care of emulating devices (like the network card) for HVM guests. |
||
+ | == How to Generate and Submit a Patch to Qemu-Xen == |
||
+ | |||
+ | Xen Project uses QEMU as device model, that is the software component that takes care of emulating devices (like the network card) for HVM guests. |
||
Two QEMU trees, both using [http://git-scm.com/ git] for revision control, are currently in use by xen.git: |
Two QEMU trees, both using [http://git-scm.com/ git] for revision control, are currently in use by xen.git: |
||
− | + | * '''[http://xenbits.xen.org/gitweb/?p=qemu-xen-traditional.git;a=summary qemu-xen-traditional]''': old and stable tree, only bug fixes are accepted in this tree at the moment; |
|
− | |||
− | - [http://xenbits.xen.org/gitweb/?p=staging/qemu-upstream-unstable.git;a=summary qemu-xen]: new tree used for development, based on the latest stable branch of Upstream QEMU, that currently is available at [http://git.qemu.org/ qemu git repo]. See [[QEMU Upstream]] on how to manually build it and use it to run VMs. |
||
+ | * '''[http://xenbits.xen.org/gitweb/?p=qemu-xen.git;a=summary qemu-xen]''': new tree used for development, based on the latest stable branch of Upstream QEMU, that currently is available at [http://git.qemu.org/ qemu git repo]. See [[QEMU Upstream]] on how to manually build it and use it to run VMs. |
||
New features should be developed again the latest upstream QEMU first, see http://wiki.qemu.org/Contribute/SubmitAPatch, then upon request they can be backported to qemu-xen. |
New features should be developed again the latest upstream QEMU first, see http://wiki.qemu.org/Contribute/SubmitAPatch, then upon request they can be backported to qemu-xen. |
||
Line 269: | Line 573: | ||
In order to request a backport, send an email to xen-devel@lists.xen.org, specifying the original commit id in qemu.git and the reason why the patch should be backported to qemu-xen. |
In order to request a backport, send an email to xen-devel@lists.xen.org, specifying the original commit id in qemu.git and the reason why the patch should be backported to qemu-xen. |
||
− | == How to Generate |
+ | == How to Generate and Submit a Xen Project Patch to the Linux Tree == |
− | Like Xen, Linux uses the git SCM. The Linux tree contains documentation on submitting patches, in Documentation/SubmittingPatches, as well as a script (scripts/checkpatch.pl) that ensures your patch is in Linux house style. Running git apply --check on your patch can reveal merge errors. |
+ | Like Xen Project, Linux uses the git SCM. The Linux tree contains documentation on submitting patches, in Documentation/SubmittingPatches, as well as a script (scripts/checkpatch.pl) that ensures your patch is in Linux house style. Running git apply --check on your patch can reveal merge errors. |
− | Patches should be emailed to xen-devel@lists. |
+ | Patches should be emailed to xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, and any relevant Linux maintainers (which can be found by running scripts/get_maintainer.pl on your patch). |
+ | |||
+ | == How to Generate and Submit a Xen related patch to FreeBSD == |
||
+ | |||
+ | The FreeBSD project uses [https://svnweb.freebsd.org/base/ svn] for it's master source tree, although [https://github.com/freebsd/freebsd git clones] are also available. Checkout the code from either the svn or git mirror and remember to always create your patch against the [https://svnweb.freebsd.org/base/head/ HEAD] branch. It can be backported to stable versions afterwards, but it needs to be initially committed to HEAD. |
||
+ | |||
+ | The best way to get your patches reviewed is to use [https://wiki.freebsd.org/Phabricator Phabricator] and assign them to the relevant maintainer (check [https://svnweb.freebsd.org/base/head/MAINTAINERS?view=markup MAINTAINERS] in the source tree), for Xen related code that would be royger. |
||
+ | |||
+ | == How to Generate and Submit a Xen Project Patch to MiniOS and Unikraft == |
||
+ | '''MiniOS''' uses the following git tree for development: [http://xenbits.xen.org/gitweb/?p=mini-os.git;a=summary mini-os.git]. Patches are submitted to [https://www.xenproject.org/help/mailing-list.html#minios minios-devel@]. |
||
+ | |||
+ | '''Unikraft''' uses several git trees under the unikraft namespace for development: [http://xenbits.xen.org/gitweb/?a=project_list;pf=unikraft unikraft/]. Patches are submitted to [https://www.xenproject.org/help/mailing-list.html#minios minios-devel@] and are marked with a '''UNIKRAFT''' prefix (or with a library name prefix), such that they can be differentiated from MiniOS patches. A detailed description of the Unikraft convention can be found in the Unikraft base repository [http://xenbits.xen.org/gitweb/?p=unikraft/unikraft.git;a=summary unikraft/unikraft.git]: [http://xenbits.xen.org/gitweb/?p=unikraft/unikraft.git;a=blob_plain;f=CONTRIBUTING.md;hb=HEAD CONTRIBUTING.md]. |
||
+ | |||
+ | [[Category:Developers]] |
||
+ | [[Category:Development Process]] |
||
+ | [[Category:Xen]] |
Latest revision as of 10:01, 9 November 2022
This document assumes that you have cloned xen.git (or a similar repository) and that you have some development patches ready to submit. If you are not familiar with git and Xen Development branches, read the following documents
- Xen Project Repositories
- Managing Xen Patches with Git
- Managing Xen Patches with StGit
- Managing Xen Patches with_Git-series
This document outlines the code submission process primarily to the xen-devel@ mailing list. The process does also apply to other mailing list based subprojects: specific information related to these subprojects can be found at the bottom of this document.
Contents
- 1 What is in a patch and a patch series?
- 1.1 What is in a patch?
- 1.1.1 Title and description of the patch
- 1.1.2 Signed-off-by
- 1.1.3 Other *-by tags
- 1.1.4 CC lists of the maintainers or reviewers for the patch
- 1.1.5 Change log (for patch revisions >= 2)
- 1.1.6 Information that is useful for the reviewer, which you do not want in the final commit messages
- 1.1.7 A version number and subject prefix
- 1.1.8 Example patch
- 1.2 What is in a patch series?
- 1.1 What is in a patch?
- 2 Sending the patches to the list
- 3 Review, Rinse & Repeat
- 4 Workflow for related repositories and projects
What is in a patch and a patch series?
Changes to the Xen Project are submitted to a mailing list either as an individual patch or as a patch series. A patch series is a series of patches that are related and broken into a logical series of individual patches.
Patches, if accepted, will turn into commits in the git source tree. However, frequently multiple versions of patches will have to be posted, before the patch is accepted.
There are three groups of people to think about when writing the patch and the commit message:
- Reviewers on the mailing list, who are trying to understand what your patch does, if it's correct, and what side effects it will have.
- People skimming through changesets deciding if a patch is important for them to look at for backporting, review, implications on out-of-tree patches, &c.
- Someone in the perhaps distant future, who is trying to understand why the code is the way it is.
Try to make your patches with all of these people in mind.
What is in a patch?
A patch is an e-mail, which is generated from a single commit in your git tree and augmented by using a tool (e.g. git-format, add_maintainers.pl, git-send-email) which may prompt you to edit the final e-mail manually.
It is important to note, that your patch will inherit the following information from the git commit message on which it is based, assuming you recorded the information in the commit message at commit time
- Title and description
- Signed-off-by
- Other *-by tags such as Reported-by
- CC lists of reviewers - these can be added automatically later
- Change log and other information that is useful for the reviewer
![]() |
More documentation can be found at https://xenbits.xen.org/docs/unstable/process/sending-patches.html including the format of Fixes: lines |
Title and description of the patch
- The first line the top of the patch should contain a short description of what the patch does, and hints as to what code it touches. It typically contains a prefix, such as xen/x86:, xen/arm:, tools:, etc. Note that maintainers may propose a different prefix or change it at commit-time if it isn't correct
- The description should be useful for both the reviewers and people in the future trying to understand what the patch does. It can be as short as
Code cleanup -- no functional changes
, or as long as it takes to accurately describe the bug you are trying to solve or the new functionality you are introducing.
The description should be wrapped to a maximum of 80 columns, unless it includes an error message which is longer than 80 characters.
Signed-off-by
All patches to the Xen Project code base must include the line Signed-off-by: your_name <your_email> at the end of the change description.
You can use -s or --signoff when committing a patch, if you change your default settings as outlined here.
This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states:
Developer's Certificate of Origin 1.1
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
Other *-by tags
You may add the following git tags before or after the signed-off-by tag:
- Reported-by: is used to credit someone who found the bug that the patch attempts to fix
- Tested-by: is used to indicate that the listed person applied the patch and found it to have the desired effect.
Later in the review process, your patch or patch series may accumulate tags, such as
- Acked-by: a maintainer of the patch reviewed the patch and found it is good to go in.
- Reviewed-by: another community member, a designated reviewer or a maintainer of another component reviewed the patch
Note that if there are several revisions of a patch, you ought to copy tags that have accumulated during the review. For example, if person A and person B added a Reviewed-by: tags to v1 of your patch, include it into v2 of your patch. If you make substantial changes after certain tags were already applied, you will want to consider which ones are no longer applicable (and may require re-providing).
CC lists of the maintainers or reviewers for the patch
- Always send a copy of the patch to (via CC) the maintainer of the code you are modifying. The maintainers and reviewers are listed in the MAINTAINERS file at the top of the Xen tree. If no maintainer is listed then the maintainers listed under THE REST in the MAINTAINERS file
- Note that if you follow the steps described in this document, the add_maintainers.pl/get_maintainer.pl scripts will add the correct people automatically
- If any people who were not on the CC list for an earlier review, but have reviewed your patch, you may want to manually add them (via CC) to the CC list.
Change log (for patch revisions >= 2)
Changes to previous versions of a patch are recorded in the patch commit message after the CC lists. These are required to be very detailed and will help the reviewer to check whether changes have been addressed. An example of a change log can be found below (the actual patch for this example is here).
Changes in v4:
- ARM -> Arm
- libxl__memory_policy_to_xc -> libxl__arch_memory_policy_to_xc
- keep Arm policies together
Changes in v3:
- s/nGRE/nGnRE/g
- s/LIBXL_MEMORY_POLICY_ARM_DEV_NGRE/LIBXL_MEMORY_POLICY_ARM_DEV_NGNRE/g
- s/arm_devmem/arm_dev_nGnRE/g
- s/arm_memory/arm_mem_WB/g
- improve commit message
- improve man page
- s/MEMORY_POLICY_X86_UC/MEMORY_POLICY_X86_UC_MINUS/g
- s/x86_uc/x86_UC_minus/g
- move security support clarification to a separate patch
Changes in v2:
- add #define LIBXL_HAVE_MEMORY_POLICY
- ability to part the memory policy parameter even if gfn is not passed
- rename cache_policy to memory policy
- rename MEMORY_POLICY_DEVMEM to MEMORY_POLICY_ARM_DEV_nGRE
- rename MEMORY_POLICY_MEMORY to MEMORY_POLICY_ARM_MEM_WB
- rename memory to arm_memory and devmem to arm_devmem
- expand the non-security support status to non device passthrough iomem
configurations
- rename iomem options
- add x86 specific iomem option
Information that is useful for the reviewer, which you do not want in the final commit messages
You can manually add additional information that you do not want to appear in the commit message, after the maintainers list. An example can be found in this patch.
A version number and subject prefix
Each patch contains a revision number, or a subject prefix. For example, a patch email may contain [RFC PATCH v2] or [PATCH v3 4/11] indicating that the patch is part of a patch series.
Example patch
The example patch here that resulted in the following commit.
1 vif-common.sh: Have iptables wait for the xtables lock permalink
2
3 iptables has a system-wide lock on the xtables. Strangely though, in
4 the case of two concurrent invocations, the default is for the
5 instance not grabbing the lock to exit out rather than waiting for it.
6 This means that when starting a large number of guests in parallel,
7 many will fail out with messages like this:
8
9 2017-05-10 11:45:40 UTC libxl: error: libxl_exec.c:118: libxl_report_child_exitstatus: /etc/xen/scripts/vif-bridge remove [18767] exited with error status 4
10 2017-05-10 11:50:52 UTC libxl: error: libxl_exec.c:118: libxl_report_child_exitstatus: /etc/xen/scripts/vif-bridge offline [1554] exited with error status 4
11
12 In order to instruct iptables to wait for the lock, you have to
13 specify '-w'. Unfortunately, not all versions of iptables have the
14 '-w' option, so on first invocation check to see if it accepts the -w
15 command.
16
17 Reported-by: Antony Saba <aws...@gmail.com>
18 Signed-off-by: George Dunlap <geor...@citrix.com>
19 ---
20 Cc: Ian Jackson <ian....@citrix.com>
21 Cc: Wei Liu <wei....@citrix.com>
22 ---
23 tools/hotplug/Linux/vif-common.sh | 38 +++++++++++++++++++++++++++++++++++---
24 1 file changed, 35 insertions(+), 3 deletions(-)
25
26 diff --git a/tools/hotplug/Linux/vif-common.sh
27 b/tools/hotplug/Linux/vif-common.sh
28 index 6e8d584..29cd8dd 100644
29 --- a/tools/hotplug/Linux/vif-common.sh
30 ...
- Lines 1 to 15: A good patch description answers the following questions
- What is the situation
- Why is the current situation a problem
- How does this patch fix the problem
- Anything else the reviewers need to know to understand the patch
- Lines 17 to 18: Contains the *by tags including the mandatory Signed-off-by tag
- Lines 20 to 21: Contains CC statements of the maintainers. Note that these can be added manually, but if you follow the steps below these will be added automatically
- Before Line 22: added in lines before the
---
- After Line 22: This is the actual code patch
What is in a patch series?
A patch series is a sequence of threaded emails, which are generated from multiple git commits (typically the last N ones) in your git tree and augmented by using a tool (e.g. git-format
, add_maintainers.pl
, git-send-email
) which may prompt you to edit the final email manually.
Patch series can be anything from bug fixes or small improvements that are too big for an individual patch or encompass different components (e.g. hypervisor, toolstack change and a docs change) to new features with 10s of patches spanning multiple components.
In addition to the individual patches, which describe the patches making up the series, a patch series must contain a cover letter as the first email in the series. If you want any of the automated tooling which we use on xen-devel@ to work, the patch series must be threaded, with the cover letter the parent and each individual patch being a child of the cover letter.
Break down your patches appropriately
- Each patch in a patch series should preferably do one logical thing (or one related set of things). The goal is for reviewers to understand the change that each patch is making with a minimum of effort.
- No patch should ever break existing functionality.
- Never fix bugs in one of your patches by changing it in a later patch; go back and change the patch with the bug in it.
- Don't mix clean-up patches (which make things look prettier or refactor but don't change functionality) with code-change patches. However, e.g. style corrections on lines changed anyway are generally accepted (and often welcomed). Clean-up patches should be clearly marked as having no functional changes.
Cover letter
Cover letters are an important part of a patch series. The Xen Project has high standards when it comes to large and complex series. The cover letter should cover the following information
- It should contain an explanation of what you are trying to achieve
- It should explain as to why the series is needed (if it is not obvious)
- How the series achieves your goal
- It should list any pre-conditions and major assumptions you have made
- It should refer to any specifications, design documents, previous discussion on mailing lists and other relevant information
The amount of detail required depends on the complexity and size of the patch series. The larger and more complex the series, the more information is required. The goal of the information is to make it easier for the code reviewer to understand your series. The more time you invest in preparing the series’ cover letter, the easier it will be for a reviewer to understand your code.
However, cover letters do not get committed to the git tree: if the cover letter contains information that should be in the git tree, you may want to create a new document under docs/designs, docs/features or docs/specs instead as part of the series and refer to it from the series.
Change log (for patch series revisions >= 2)
The end of the cover letter is also the location where you would typically track changes between different versions of the patch series. The format is identical to change logs of individual patches (see here), but a series change log is much less detailed and more high-level than for a patch. The key purpose is to point reviewers to areas of the series which need to be revisited.
Providing a git branch
If you send a large patch series, it is recommended to include a branch in your public git repository in your cover letter, so that reviewers can pull directly from that branch.
If you do not have a public repository, you can fork the Xen Project’s GitHub or GitLab mirrors at https://github.com/xen-project/xen and https://gitlab.com/xen-project/xen into your own project space and push your branch to your own fork and provide a link to that branch. If you are a regular contributor to the Xen Project, you can request to host personal repositories at xenbits by sending an email to community dot manager at xenproject dot org.
Sending the patches to the list
The xen-devel mailing list is moderated for non-subscribers. It is not mandatory to subscribe but it can help avoid this delay. It is possible to subscribe and then disable delivery in the mailman options so as to avoid moderation but not receive list traffic if that is what you would prefer.
Setting up git send-email
The most robust way to send files is by using the git send-email command. (If you're using mercurial, please see our Submitting_Xen_Patches_-_mercurial).
To do this, first set configure your information:
git config --global sendemail.from "YOUR NAME <user@example.org>"
git config --global sendemail.smtpserver imap.example.org
git config --global sendemail.smtpuser USER
# depending on your config you may also set:
git config --global sendemail.smtpencryption tls
If you don't want to enter password for your SMTP server all the time:
git config --global sendemail.smtppass = PASS
Settings that help save you time
There are a number of settings you may want to set globally or per repository, such as
# Allows you to drop --thread from git format-patch ...
git config --global format.thread true
# Allows you to use -s or --signoff when committing a patch
git config --global user.name "YOUR NAME"
git config --global user.email "<user@example.org>"
Subject Prefix
Patches and patch series are posted to a mailing list: when using the git format-patch
command a subject prefix is added to a patch or patch series, which helps disambiguate normal discussions from posted patches and code reviews.
git format-patch
creates mails with subject lines such as
# Single patch
[<SUBJECT-PREFIX>] <Title of patch in git>
# Patch series
[<SUBJECT-PREFIX> 0/n] <Title of cover letter/patch series>
[<SUBJECT-PREFIX> 1/n] <Title of 1st patch in git>
...
[<SUBJECT-PREFIX> n/n] <Title of nth patch in git>
Canonical Subject Prefix
As patches for different git repositories can be posted (or CC'ed) to different mailing lists, some conventions have emerged over time which help code reviewers identify which patch is intended for which git tree. We refer to these as canonical subject prefix. The table below shows recommended default Subject Prefixes, which either
- need to be passed using the
--subject-prefix=<Subject-Prefix>
option when callinggit format-patch
, - or can be set as defaults per git repository as described here
The following table lists common canonical subject prefixes used in the project
repo | Subject Prefix | List |
---|---|---|
git default | PATCH
| |
xen.git | PATCH or XEN PATCH (recommended)
|
xen-devel@ |
osstest.git | OSSTEST PATCH
|
xen-devel@ |
xtf.git | XTF PATCH
|
xen-devel@ |
xenalyze.git | XENALYZE PATCH
|
xen-devel@ |
livepatch-build-tools.git | LIVEPATCH-BUILD-TOOLS PATCH
|
xen-devel@ |
mini-os.git | PATCH or MINI-OS PATCH (recommended)
|
minios-devel@ but sometimes xen-devel@ is CC'ed |
unikraft/unikraft.git | UNIKRAFT PATCH
|
minios-devel@ |
unikraft/.../<REPO>.git | UNIKRAFT/<REPO> PATCH
|
minios-devel@ |
IMPORTANT: If you do not use a canonical subject prefix some existing and future automated tooling may not work correctly: consider for example a situation in which a patch for livepatch-build-tools.git is sent to xen-devel@ and that this triggers a build test in different environments. In this case, the build test would try and apply the patch to xen.git and might fail in unexpected ways.
Common modifications to the Subject Prefix
In addition, the following modifications to Subject Prefixes are commonly used and can also be used in combination
Subject Prefix Modification | Example | When to use |
---|---|---|
RFC ...
|
RFC XTF PATCH
|
RFC means "Request For Comments"; use this when sending an experimental patch for discussion rather than application. |
... RESEND
|
XTF PATCH RESEND
|
Typically used if your patch has not been reviewed for some time and you want to remind people of the patch |
... for-<release>
|
XEN PATCH for-4.13
|
Typically used after the master xen.git branch has been feature frozen and you want to highlight a patch that is destined for the release which is currently being developed |
... for-next
|
XEN PATCH for-next
|
Typically used towards the end of a development cycle when you want to highlight a patch that is destined for the next release |
git format-patch
contains options to help with these
--rfc
will mark a patch as RFC, but will always lead toRFC PATCH
--subject-prefix=<Subject-Prefix>
allows you to set a subject prefix, but will overwrite any defaults you may have set
Changing git defaults to use the Canonical Subject Prefix
To change the setting perform
cd <local-git-tree>
git config --local format.subjectPrefix "<canonical subject prefix>"
Sending a Patch Series
Step 1: Create patches using git format-patch
Git format patch allows you to create and save formatted patches in a directory. By default it will create them in the root of your git directory, but you probably want to direct this into a ../patches/feature-version
directory (in the examples below ../patches/feature-v2
would contain only v2 of that series). It is also possible to store several versions of a patch, e.g. v1, v2, etc in the same ../patches/feature
directory). Let's say the last two commits of your head are part of your series you want to send out. In this case, the command line would look like
$ git format-patch --reroll-count=2 --thread --cover-letter -o ../patches/feature-v2 -2
This will create 3 files, such as
v2-0000-cover-letter.patch v2-0002-Patch-to-do-bar.patch v2-0001-Patch-to-do-foo.patch
Notes
- You will need to edit the subject and body of
v2-0000-cover-letter.patch
- You must always use the --thread and --cover-letter options. If you omit --thread, any automatic tooling and patch checking that is triggered by sending a mail to one of our mailing lists may not work without using this option. You can set --thread as a default, as outlined here.
Other useful options include:
--rfc
and--subject-prefix=Subject-Prefix
: please read this document before using these options
Step 2: Use add_maintainers.pl (or get_maintainer.pl)
Option 1: use add_maintainers.pl
$ ./scripts/add_maintainers.pl -d ../patches/feature-v2
Then follow the instructions (which are essentially Step 3, with correct command line options based on what you pass to add_maintainers.pl
). Note that the add_maintainers.pl
script works around a limitation of git send-email ... --cc-cmd="./scripts/get_maintainer.pl" ...
, which does not automatically update the CC list of your cover letter.
Other useful options include:
--reroll-count|-v
(e.g.-v 2
): If you store your patches in one directory with different versions in it generated by--reroll-count=2
you want to use this option--patchcc|-p LOCATION
: Inserts CC's into a specific location (see--help
) to *.patch files--covercc|-c LOCATION
: Inserts CC's into a specific location (see--help
) to the cover letter--tags|-t
: Adds people who have *-by tags to the mail header (git send-email
does not do this itself)--tagscc
: Adds people who have *-by tags to the CC list--arg|-a
(e.g.-a "<argument1 with space>" -a <argument2> ...
for arguments you want to pass to./scripts/get_maintainer.pl
)
Common LOCATION combinations include:
-p commit
: copy CC blocks into the *.patch body (CC's will become part of the commit message)-p none -c header
: copy CC blocks from *.patch files in the cover letter without modifying *.patch files. This is useful in rerolled patches, where you do not want to modify the CC blocks.-p ccbody
behaves similarly, only that any new CCs that may come from an updated MAINTAINERS file or from new files that have been added to *.patch files will be added to *.patch files.-p comment -c end
: copy CC blocks after the *.patch body (CC's will not be committed) and into the body of the cover letter
Option 2: use get_maintainer.pl manually
Foreach <v2 patchfile> in ../patches/feature (with XXXX > 0000): - Run: ./scripts/get_maintainer.pl <v2 patchfile> - Review the e-mail list and prefix each line with Cc: - Copy the e-mail CC block into patchfile - Do anything else you want to do manually For v2-0000-cover-letter.patch take the superset of all the files generated previously and merge, then copy the e-mail CC block into 0000-cover-letter.patch
Step 3: Send patches using git send-email
Send the patches using
$ git send-email --to xen-devel@lists.xenproject.org ../patches/feature-v2/*.patch
Other useful options include:
--dry-run
--cc
if you need to CC additional reviewers (e.g. from within your team)
Submitting patches against point releases, such as Xen 4.10
When you develop against older Xen versions you are likely going to use an outdated version of the MAINTAINERS file. Typically only the files on staging or master are up-to-date. To work around this, use the following slightly modified workflow
$ git format-patch ... -o <patchdir> ...
$ checkout master
$ ./scripts/add_maintainers.pl -d <patchdir>
$ checkout <original branch>
$ git send-email ... <patchdir>
This ensures that you use the latest set of tools and the latest MAINTAINERS file
Sending an individual patch
You can use the same workflow as outlined in the previous section, without generating a cover letter. In this case, the git-format-patch
command line in step 1 would look like
$ git format-patch --thread -o ../patches/bugfix-v2 -1
This is followed by steps 2 and 3. However, for single patches without a cover letter, using git-send-email
alone, is quite a reasonable option. In this case, you can use the following command line, which will get the CC list from the ./scripts/get_maintainer.pl
script, which allows you to fold all 3 steps into 1.
$ git send-email --to xen-devel@lists.xenproject.org --cc-cmd="./scripts/get_maintainer.pl" -1
Other useful options include:
--dry-run
will go through all the motions of preparing the patchbomb, but instead of sending a mail, will just output the mails it would have sent. Useful for testing.--cc
if you need to CC additional reviewers (e.g. from within your team)--reroll-count=N
allows you to change the revision of the patch--rfc
and--subject-prefix=Subject-Prefix
allow you to change the subject prefix: please read this document before using these options
Using add_maintainers.pl (or get_maintainer.pl) from outside of xen.git
You can use add_maintainers.pl
or get_maintainer.pl
on any Xen Project git repository with a compatible MAINTAINERS
file in the root of its tree. An example is livepatch-build-tools.git
. In this case simply replace ./scripts/add_maintainers.pl
or ./scripts/get_maintainer.pl
with the full path to the script.
# You are in the xen.git sister repository (e.g. livepatch-build-tools)
$ git format-patch ... -o <patchdir> ...
$ $LOCATION-OF-XEN-GIT/scripts/add_maintainers.pl -d <patchdir>
$ git send-email ... <patchdir>
A minimum template for such a MAINTAINERS
file can be found below.
This file follows the same conventions as outlined in
xen.git:MAINTAINERS. Please refer to the file in xen.git
for more information.
THE REST
M: MAINTAINER1 <maintainer1@email.com>
M: MAINTAINER2 <maintainer2@email.com>
L: xen-devel@lists.xenproject.org
S: Supported
F: *
F: */
V: xen-maintainers-1
Review, Rinse & Repeat
After posting your patches you will hopefully see some response in the form of comments, patch review and eventually commit.
Code Review
The form of the review may often be quite direct and to the point which may be daunting for some people. However bear in mind that detailed criticism of a patch usually means that the reviewer is interested in your patch and wants to see it go in!
Once you have addressed any review you should normally resend the patch. It is normally expected that you address all review comments before reposting. This often means code changes in your patch but could also mean simply responding to the review comments explaining you reasoning or giving reasons why something should be the way it is.
You should also rebase your change against the project's development tree before sending out a new version, such that when it is approved it applies cleanly. The only reason not to do that is if you don't expect it to be approved, and want feedback on what you have. In that case you can do the rebase later.
The relevant command line options in git-format-patch
, ./scripts/add_maintainers.pl
and git-send-email
(if used standalone) for reposting new revisions is
--reroll-count|-v
Highlight changes in the new version
When resending a patch you should normally include a note of the changes between the current and last version of the patch. Common practice is to include these notes after your Signed-off-by separated by a triple-dash (---
). This indicates patch commentary specific to the posting which need not be included in the final changelog (although you should also remember to update the changelog if necessary). You should also include a "V2" (V3, V4 etc) tag in the subject line (if you are using the git send-email
command then the --reroll-count=N
option is helpful here, or for older git versions --subject-prefix='PATCH vN'
).
Update tags
If someone replies to your patch with a tag of the form Acked-by: <Developer>, Reviewed-by:, Tested-by: etc then, assuming you have not significantly reworked the patch, you should include these tags in any reposting after your own Signed-off-by line. This lets people know that the patch has been seen and that someone approves of it and also serves to prevent reviewers wasting time re-reviewing a patch which is not significantly different to last time. The developers with commit access also like to see postings with such tags since it means they are likely to be much easier to deal with and commit.
An example of a new Patch Version
An example of a resend of the example patch from above might be:
Subject: [PATCH v2] foobar: Add a new trondle calls
Add a some new trondle calls to the foobar interface to support
the new zot feature.
Signed-off-by: Joe Smith <joe.smith@citrix.com>
Acked-by: Jane Doe <jane.doe@example.com>
---
Changed since v1:
* fix coding style
* be sure to treadle the trondle in the error case.
diff -r 63531e640828 tools/libxc/Makefile
--- a/tools/libxc/Makefile Mon Dec 07 17:01:11 2009 +0000
+++ b/tools/libxc/Makefile Mon Dec 21 11:45:00 2009 +0000
...
Resending Patches
If you do not get any response to your patch or you got lots of Acked-by's but the patch has not been committed (remember that reviewers and maintainers are busy people too and sometimes things may fall through the cracks) then after some time, perhaps 2-4 weeks (guidelines), you should resend the patch, perhaps including [RESEND] in the subject line to alert people that the last mail was dropped. Before resending you should:
- Check that the patch has not been applied to the staging branch, since committers do not always send a notification when they apply a patch.
- Consider if there is anything you can do to improve the commit message or subject line of your patch to better attract the attention of the relevant people.
Remember to include any Acked-by/Reviewed-by which you received in response to the previous post.
How to know when a patch has been committed
Changes committed to Xen Project by the committers are immediately available in the "staging" branch of the main xen.git tree. They are then automatically tested, and if the tests pass the changes are propagated to the "master" branch.
After your patch is committed
If your patch turns out to break something you will be expected to respond promptly to help diagnose and fix the problem. This assumes that you in particular keep an eye on the osstest flight reports sent to xen-devel. If it can't be fixed quickly, your change may be reverted.
How to Generate and Submit a Patch to Qemu-Xen
Xen Project uses QEMU as device model, that is the software component that takes care of emulating devices (like the network card) for HVM guests. Two QEMU trees, both using git for revision control, are currently in use by xen.git:
- qemu-xen-traditional: old and stable tree, only bug fixes are accepted in this tree at the moment;
- qemu-xen: new tree used for development, based on the latest stable branch of Upstream QEMU, that currently is available at qemu git repo. See QEMU Upstream on how to manually build it and use it to run VMs.
New features should be developed again the latest upstream QEMU first, see http://wiki.qemu.org/Contribute/SubmitAPatch, then upon request they can be backported to qemu-xen. Only patches that have been acked and committed to qemu.git are eligible to be backported to qemu-xen. In order to request a backport, send an email to xen-devel@lists.xen.org, specifying the original commit id in qemu.git and the reason why the patch should be backported to qemu-xen.
How to Generate and Submit a Xen Project Patch to the Linux Tree
Like Xen Project, Linux uses the git SCM. The Linux tree contains documentation on submitting patches, in Documentation/SubmittingPatches, as well as a script (scripts/checkpatch.pl) that ensures your patch is in Linux house style. Running git apply --check on your patch can reveal merge errors.
Patches should be emailed to xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org, and any relevant Linux maintainers (which can be found by running scripts/get_maintainer.pl on your patch).
The FreeBSD project uses svn for it's master source tree, although git clones are also available. Checkout the code from either the svn or git mirror and remember to always create your patch against the HEAD branch. It can be backported to stable versions afterwards, but it needs to be initially committed to HEAD.
The best way to get your patches reviewed is to use Phabricator and assign them to the relevant maintainer (check MAINTAINERS in the source tree), for Xen related code that would be royger.
How to Generate and Submit a Xen Project Patch to MiniOS and Unikraft
MiniOS uses the following git tree for development: mini-os.git. Patches are submitted to minios-devel@.
Unikraft uses several git trees under the unikraft namespace for development: unikraft/. Patches are submitted to minios-devel@ and are marked with a UNIKRAFT prefix (or with a library name prefix), such that they can be differentiated from MiniOS patches. A detailed description of the Unikraft convention can be found in the Unikraft base repository unikraft/unikraft.git: CONTRIBUTING.md.