Submitting Xen Project Patches: Difference between revisions

From Xen
Jump to navigationJump to search
 
(28 intermediate revisions by 3 users not shown)
Line 30: Line 30:
* CC lists of reviewers - these can be added automatically later
* CC lists of reviewers - these can be added automatically later
* Change log and other information that is useful for the reviewer
* 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}}


=== Title and description of the patch ===
=== Title and description of the patch ===
Line 40: Line 42:


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.
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 [[#Settings_that_help_save_you_time|here]].
<br>
<br>
<br>
<br>
Line 155: Line 158:
Signed-off-by: George Dunlap <geor...@citrix.com>
Signed-off-by: George Dunlap <geor...@citrix.com>
---
---
CC: Ian Jackson <ian....@citrix.com>
Cc: Ian Jackson <ian....@citrix.com>
CC: Wei Liu <wei....@citrix.com>
Cc: Wei Liu <wei....@citrix.com>
---
---
tools/hotplug/Linux/vif-common.sh | 38 +++++++++++++++++++++++++++++++++++---
tools/hotplug/Linux/vif-common.sh | 38 +++++++++++++++++++++++++++++++++++---
Line 181: Line 184:


<br>
<br>

== What is in a patch series? ==
== 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.
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.
Line 216: Line 220:


{{Anchor|send}}
{{Anchor|send}}

= Sending the patches to the list =
= Sending the patches to the list =


Line 235: Line 240:
<syntaxhighlight lang="sh" highlight="1">
<syntaxhighlight lang="sh" highlight="1">
git config --global sendemail.smtppass = PASS
git config --global sendemail.smtppass = PASS
</syntaxhighlight>

== 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" highlight="2,5,6">
# 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>"
</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>
</syntaxhighlight>


Line 255: Line 374:
Notes
Notes
* You will need to edit the subject and body of <code>v2-0000-cover-letter.patch</code>
* 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 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>: generates subject lines such as <code>[RFC PATCH ...]</code>
* <code>--rfc</code> and <code>--subject-prefix=Subject-Prefix</code>: please read [[#Subject_Prefix|this document]] before using these options
* <code>--subject-prefix=Subject-Prefix</code>: e.g. example values are <code>PATCH RESEND</code>, <code>RFC PATCH RESEND</code> (ignores <code>--rfc</code>) ,<code>PATCH for-4.11</code>, etc.


<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''') ===
=== '''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''') ===
Line 269: Line 388:
$ ./scripts/add_maintainers.pl -d ../patches/feature-v2
$ ./scripts/add_maintainers.pl -d ../patches/feature-v2
</syntaxhighlight>
</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.
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.


Line 299: Line 421:
Send the patches using
Send the patches using
<syntaxhighlight lang="sh" highlight="1">
<syntaxhighlight lang="sh" highlight="1">
$ git send-email -to xen-devel@lists.xenproject.org ../patches/feature-v2/*.patch
$ git send-email --to xen-devel@lists.xenproject.org ../patches/feature-v2/*.patch
</syntaxhighlight>
</syntaxhighlight>


Line 307: Line 429:


<br>
<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 ===
=== 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
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">
<syntaxhighlight lang="sh" highlight="2,4">
$ git format-patch ...
$ git format-patch ... -o <patchdir> ...
$ checkout master
$ checkout master
$ ./scripts/add_maintainers.pl -d ...
$ ./scripts/add_maintainers.pl -d <patchdir>
$ checkout <original branch>
$ checkout <original branch>
$ git send-email ...
$ git send-email ... <patchdir>
</syntaxhighlight>
</syntaxhighlight>
This ensures that you use the latest set of tools and the latest MAINTAINERS file
This ensures that you use the latest set of tools and the latest MAINTAINERS file
Line 336: Line 468:
* <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>--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>--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>).
* <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>
<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 ==
== 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.
{{InfoLeft|This capability outlined in this section is not yet available in xen.git. The relevant patches can be found [https://lists.xenproject.org/archives/html/xen-devel/2019-09/threads.html#00263 here].}}

<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>


A minimum template for such a <code>MAINTAINERS</code> file can be found below.
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. A minimum template for such a <code>MAINTAINERS</code> file can be found below.


<syntaxhighlight lang="sh">
<syntaxhighlight lang="sh">

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

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.

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
Icon Ambox.png 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.

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.

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
...
  • 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
Icon Ambox.png 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 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)


Icon Ambox.png 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:

$ git send-email --to security@xenproject.org --suppress-cc=all ../patches/feature-v2/*.patch



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.