Difference between revisions of "Submitting Xen Project Patches"

From Xen
(Git format-patch, get_maintainer.pl and git send-email: Fix typo)
(Step 3: Send patches using git send-email: Fixed typo -to > --to)
 
(73 intermediate revisions by 3 users not shown)
Line 1: Line 1:
== How To Generate and Submit Patches to the Xen Project ==
+
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]]
  
The Xen Project team uses [http://git-scm.com Git] for its source code management. The current Xen Project development tree is at http://xenbits.xen.org/gitweb/?p=xen.git; "master" is the branch containing the latest development changesets to pass our automated build and test system.
+
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.
  
=== Generating a patch ===
+
= 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 Project maintainers. 
 
  
{{InfoLeft|The instructions on this page use the most common git workflow: aka '''the patch series is developed on a git branch'''. Many people find this '''difficult to work with'''. There are some alternatives
+
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.
* '''StGit''' is a Python application providing similar functionality to Quilt (i.e. pushing/popping patches to/from a stack) on top of Git. You can find instructions at [[Submitting_Xen_Patches_with_StGit]].
 
* '''Patman''' is a Python script originally developed for U-Boot (but it also should work with Xen), which creates patches directly from your branch, cleans them up by removing unwanted tags, inserts a cover letter with change lists, ... This tool is useful for specially for long series that you expect to resend a couple of times.
 
  
: Patman is available from <pre>http://git.denx.de/?p=u-boot.git;a=tree;f=tools/patman</pre> (also see the README).
+
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.  
}}
 
If you want to use mercurial, please see [[Submitting_Xen_Patches_-_mercurial]].
 
<hr>
 
<br>
 
Begin by cloning the git repo from XenProject.org:
 
<syntaxhighlight lang="sh">
 
$ git clone git://xenbits.xenproject.org/xen.git xen.git
 
</syntaxhighlight>
 
  
At this point you should have <code>xenbits</code> set up as the remote repostiory called "origin":
+
There are three groups of people to think about when writing the patch and the commit message:
<syntaxhighlight lang="sh">
+
* 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.
$ git branch -a
+
* 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.
* master
+
* Someone in the perhaps distant future, who is trying to understand why the code is the way it is.
  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
 
</syntaxhighlight>
 
 
 
This process automatically creates a local branch called <code>master</code> that will track the XenProject.org branch called <code>master</code>.
 
  
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.
+
Try to make your patches with all of these people in mind.
  
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:
+
== 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.
  
<syntaxhighlight lang="sh">
+
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
$ git checkout -b out/trondle-calls master
+
* Title and description
Switched to a new branch 'out/trondle-calls'
+
* Signed-off-by
</syntaxhighlight>
+
* 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
  
Then edit the source files you want to change. You can see which files have changed by using <code>git status</code>.
+
=== 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
  
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 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, unless it includes an error message which is longer than 80 characters.
<syntaxhighlight lang="sh">
 
$ 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
 
</syntaxhighlight>
 
  
Alternatively, you can commit all changes using "git commit -a":
+
=== Signed-off-by ===
  
<syntaxhighlight lang="sh">
+
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.  
$ git commit -a
+
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]].
</syntaxhighlight>
+
<br>
+
<br>
<syntaxhighlight lang="sh">
+
{{WarningLeft|It is important that the '''e-mail address reflects the copyright holder of the code'''.  
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>
 
</syntaxhighlight>
 
 
 
Make as many commits on your new branch as are necessary.
 
 
 
 
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 XenProject.org nightly testing to fail), you can do the following:
 
<syntaxhighlight lang="sh">
 
$ git checkout -b staging remotes/origin/staging
 
Branch staging set up to track remote branch staging from origin.
 
Switched to a new branch 'staging'
 
</syntaxhighlight>
 
Then in the commands above, you would use "staging" instead of "master" as the base branch.
 
...
 
  
=== Signing off a patch ===
+
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.}}
----
 
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. This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states:
 
  
 +
This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states:
 
   
 
   
 
<syntaxhighlight>
 
<syntaxhighlight>
Line 126: Line 77:
 
</syntaxhighlight>
 
</syntaxhighlight>
  
=== Making good patches ===
+
=== Other *-by tags ===
----
+
You may add the following git tags before or after the signed-off-by tag:
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:
+
* '''Reported-by:''' is used to credit someone who found the bug that the patch attempts to fix
* 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.
+
* '''Tested-by:''' is used to indicate that the listed person applied the patch and found it to have the desired effect.
* 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.
+
Later in the review process, your patch or patch series may accumulate tags, such as
Try to make your patches with all of these people in mind. To do this:
+
* '''Acked-by:''' a maintainer of the patch reviewed the patch and found it is good to go in.
==== Break down your patches ====
+
* '''Reviewed-by:''' another community member, a designated reviewer or a maintainer of another component reviewed the patch
* 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 move things round but don't change functionality) with code-change patches. Clean-up patches should be clearly marked as having no functional changes.
+
* 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.
 
  
==== Cc the maintainer of the code you are modifying ====
+
=== 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.
  
* Please always send a copy of the patch to (via CC) the maintainer of the code you are modifying.
+
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.
* 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!).
+
 
** You can pipe your patch to the <code>./scripts/get_maintainer.pl</code> tool and it will list the relevant maintainers.
+
=== Change log (for patch series revisions >= 2) ===
* In addition to CCing the maintainer you should always send patches to (via TO) the xen-devel@lists.xenproject.org mailing list as well.
+
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.
* 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.
 
  
 
=== Providing a git branch ===
 
=== Providing a git branch ===
  
If you need to send more than one patches (which normally means you're sending a patch series with cover letter), it is recommended to include a branch pointing to your public git repository (if you have one) in your cover letter, so that people can pull directly from that 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.
  
One option is to [https://github.com/xen-project/xen/fork fork] the GitHub mirror at https://github.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 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 ===
+
{{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.
  
==== Git send-email ====
+
== Setting up git send-email ==
 
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">
+
<syntaxhighlight lang="sh"  highlight="1,2,3,5">
 
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 175: Line 235:
  
 
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">
+
<syntaxhighlight lang="sh"  highlight="1">
 
git config --global sendemail.smtppass = PASS
 
git config --global sendemail.smtppass = PASS
 
</syntaxhighlight>
 
</syntaxhighlight>
  
Then check to make sure you have the right patches to be submitted by using <code>git </code>:
+
== Settings that help save you time ==
+
There are a number of settings you may want to set globally or per repository, such as
<syntaxhighlight lang="sh">
+
<syntaxhighlight lang="sh"  highlight="2,5,6">
$ git log master..
+
# Allows you to drop --thread from git format-patch ...
commit 347175ecaba64c324853f004e4860a941b07e5a9
+
git config --global format.thread true
Author: Joe Smith <joe.smith@citrix.com>
+
 
 +
# 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>"
 +
</syntaxhighlight>
 +
 
 +
== Subject Prefix ==
  
foobar: Add new trondle calls
+
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>
 
</syntaxhighlight>
  
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 Project 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.xenproject.org mailing list as well.
+
=== 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@
 +
|}
  
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>:
+
<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>
<syntaxhighlight lang="sh">
 
$ git send-email --to xen-devel@lists.xenproject.org --cc the.maintainer@example.com master..
 
</syntaxhighlight>
 
  
<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>.
 
  
Note well, 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.
+
=== 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
 +
|}
  
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>[[#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
  
When sending a series (as opposed to a single patch) it is recommended to preceed it with a "[PATCH 0/N]" mail giving a brief overview of the series. This can be done using the <code>--compose</code> option to <code>git send-email</code> or by sending it from you regular email client and then using the <code>--in-reply-to=&lt;MESSAGE-ID&gt;</code> option (with the contents of message-id header of the 0/N mail) to cause the series to appear as a reply to that message. Note well, that the &lt; and &gt; are a literal part of the message ID (i.e. you should include them).
+
<br>
  
Other useful options include:
+
=== Changing git defaults to use the Canonical Subject Prefix ===
* <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.
+
To change the setting perform
* <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 (see also <code>--reroll-count=N</code>).
+
<syntaxhighlight lang="sh" highlight="2">
 +
cd <local-git-tree>
 +
git config --local format.subjectPrefix "<canonical subject prefix>"
 +
</syntaxhighlight>
  
==== Git format-patch, get_maintainer.pl and git send-email ====
+
== Sending a Patch Series ==
An alternative workflow to just using is git send-email is to use a combination of Git format-patch, git send-email and get_maintainer.pl
 
  
'''Step 1''': Create patches using '''git format-patch'''
+
=== '''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/N</code> directory (in the examples below <code>../patches/2</code> which would be v2 of that series). 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
+
[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">
+
<syntaxhighlight lang="sh"  highlight="1">
$ git format-patch --reroll-count=2 --thread --cover-letter -o ../patches/2 HEAD~2
+
$ git format-patch --reroll-count=2 --thread --cover-letter -o ../patches/feature-v2 -2
 
</syntaxhighlight>
 
</syntaxhighlight>
  
 
This will create 3 files, such as
 
This will create 3 files, such as
 
<pre>
 
<pre>
0000-cover-letter.patch
+
v2-0000-cover-letter.patch
0002-Add-Brian-Woods-as-Designated-reviewer-to-AMD-IOMMU-.patch
+
v2-0002-Patch-to-do-bar.patch
0001-Add-Designated-Reviewer-R-to-MAINTAINERS-file-and-ad.patch
+
v2-0001-Patch-to-do-foo.patch
 
</pre>
 
</pre>
  
Note that you will need to edit the subject and body of <code>0000-cover-letter.patch</code>
+
Notes
 +
* 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:
 
Other useful options include:
* <code>--rfc</code>
+
* <code>--rfc</code> and <code>--subject-prefix=Subject-Prefix</code>: please read [[#Subject_Prefix|this document]] before using these options
 +
 
 +
<br>
  
<br>'''Step 2''': Use '''get_maintainer.pl'''
+
=== '''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''') ===
{{TODOLeft|For now, write a little script which automates the process outlined in this step. This is necessary because <code>git send-email --cc-cmd './script/get_maintainer.pl ...</code> does not work correctly, as it does not apply the CC list to the cover letter. This is a bug in <code>git send-email</code>.}}
 
  
Foreach <code><patchfile></code> in <code>../patches/2</code> (with <code>XXXX > 0000</code>):
+
'''Option 1''': use '''[http://xenbits.xen.org/gitweb/?p=xen.git;a=blob_plain;f=scripts/add_maintainers.pl add_maintainers.pl]'''
* Run: <code>./scripts/get_maintainer.pl < <patchfile></code>
 
* Review the e-mail list and prefix each line with <code>CC: </code>
 
* Copy the e-mail CC block into <code>patchfile</code>
 
  
For <code>0000-cover-letter.patch</code> take the superset of all the files generated previously and merge, then copy the e-mail CC block into <code>0000-cover-letter.patch</code>
+
<syntaxhighlight lang="sh"  highlight="1">
 +
$ ./scripts/add_maintainers.pl -d ../patches/feature-v2
 +
</syntaxhighlight>
 +
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.
  
<br>'''Step 3''': Send patches using '''git send-email'''
+
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>
 +
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
 +
</pre>
 +
 
 +
=== '''Step 3''': Send patches using '''git send-email''' ===
  
 
Send the patches using
 
Send the patches using
<syntaxhighlight lang="sh">
+
<syntaxhighlight lang="sh"  highlight="1">
$ git send-email -to xen-devel@lists.xenproject.org ../patches/2/*.patch
+
$ git send-email --to xen-devel@lists.xenproject.org ../patches/feature-v2/*.patch
 
</syntaxhighlight>
 
</syntaxhighlight>
  
 
Other useful options include:
 
Other useful options include:
 
* <code>--dry-run</code>
 
* <code>--dry-run</code>
 +
* <code>--cc</code> if you need to CC additional reviewers (e.g. from within your team)
 +
 +
<br>
 +
 +
=== 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:
 +
* <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>--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>
  
==== Sending Patches Manually ====
+
== 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 ==
 +
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.
  
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.  
+
<syntaxhighlight lang="sh" highlight="3">
 +
# 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>
  
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.
+
A minimum template for such a <code>MAINTAINERS</code> file can be found below.  
  
=== Review, Rinse & Repeat ===
+
<syntaxhighlight lang="sh">
----
+
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.
 +
 
 +
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
 +
* <code>--reroll-count|-v</code>
  
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>--reroll-count=N</code> option is helpful here, or for older git versions <code>--subject-prefix='PATCH vN'</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>).
  
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.
+
== 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">
+
<syntaxhighlight lang="diff"  highlight="7, 10, 11, 12">
 
Subject: [PATCH v2] foobar: Add a new trondle calls
 
Subject: [PATCH v2] foobar: Add a new trondle calls
  
Line 293: Line 527:
 
</syntaxhighlight>
 
</syntaxhighlight>
  
 +
== 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 you should:
 
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:
  
Line 300: Line 535:
 
Remember to include any Acked-by/Reviewed-by which you received in response to the previous post.
 
Remember to include any Acked-by/Reviewed-by which you received in response to the previous post.
  
=== What If ===
+
== How to know when a patch has been committed ==
----
 
* 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
 
 
 
=== 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.
 
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 ===
+
== 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. If it can't be fixed quickly, your change may be reverted.
+
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.
  
[[Category:Developers]]
+
= Workflow for related repositories and projects =
[[Category:Development Process]]
+
== How to Generate and Submit a Patch to Qemu-Xen ==
[[Category:Xen]]
 
 
 
== 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.
 
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.
Line 333: Line 557:
 
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, and Submit a Xen Project Patch to the Linux Tree ==
+
== 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.
 
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.
Line 341: Line 565:
 
== How to Generate and Submit a Xen related patch to FreeBSD ==
 
== How to Generate and Submit a Xen related patch to FreeBSD ==
  
The FreeBSD prject 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 the 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 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.
 
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 ==
+
== 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@].
 
'''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].
 
'''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 19:37, 26 September 2019

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

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

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

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.

Icon Ambox.png It is important that the e-mail address reflects the copyright holder of the code.

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.


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 ---
    • Change log of the patch (see here for an example)
    • Any extra information as outlined here
  • 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 calling git 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 to RFC 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.

Workflow for related repositories and projects

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: 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).

How to Generate and Submit a Xen related patch to FreeBSD

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.