Managing Xen Patches with StGit

From Xen
Revision as of 12:49, 28 January 2013 by Royger (talk | contribs) (Created page with "== How To Generate and Submit a Patch to Xen using Git == Xen uses [http://git-scm.com/ Git] for its source code management. The current Xen development tree is at http://xenbit…")
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)
Jump to navigationJump to search

How To Generate and Submit a Patch to Xen using Git

Xen uses Git for its source code management. The current Xen development tree is at http://xenbits.xen.org/gitweb/?p=xen.git;a=summary. Git provides excellent documentation, please read it if this is your first time using Git. Download and install git to your system.

Recommended git extensions

To be able to manage patches better I recommend StGit, which is an application that runs on top of git and provides a functionality similar to Mercurial PatchQueue extension. This tutorial will show how to manage patch queues with git and StgGit, so before proceeding please make sure that StGit is installed.

Generating a patch


Here's a recommendation on how to send patches, as suggested by the Xen maintainers.

The first thing to do is cloning the xen git repository:

git clone git://xenbits.xen.org/xen.git

This will create a new folder, called xen, where you will work on your patches. We are going to create a branch for each series of patches that we are going to work on. This will allow a developer to work on several patch series at the same time, keeping the patches contained and well classified. Now we will create a new branch on top of the default branch, which is called "master".

stg branch -c my_new_feature

Now you are working on a different branch, you can switch back to the "master" branch at any time by using:

stg branch master

Now that we are on the branch we wish to use to develop feature X we will start creating patches. The first thing to do is creating a new patch, StGit will ask us to write the description of the patch, but if unsure you can leave it blank and fill it later.

stg new first_patch_in_the_series

Here is an example of a simple description:

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>

The first line in the description will be used as the subject when the patch is sent to the mailing list, so please make sure it contains an accurate description about what the patch is expected to introduce. It should be followed by a more accurate description and finally a "Signed-off-by".

Now you can starting modifying the code, all your changes will be contained inside this patch. When finished, you will have to refresh the patch, and create a new one on top if desired.

stg refresh
stg new second_patch_in_the_series

If you want to modify the description of a patch, you should use the edit command:

stg edit first_patch_in_the_series

Also, if you want to move between the series, you can use the following StGit commands, push, pop and goto.

Signing off a patch


All patches to the Xen code base must include the the line "Signed-off-by: your_name <your_email>" at the end of the change description. This is required and indicates that you certify the patch under the "Developer's Certificate of Origin" which states:

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.

Making good patches


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:

  • 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. To do this:

Break down your patches

  • Each patch should preferably do one logical thing (or one related set of things). The goal is for reviewers to understand the change that each patch is making with a minimum of effort.
  • 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 move things round but don't change functionality) with code-change patches. 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 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.

Cc the maintainer of the code you are modifying

  • To do this you should consult the MAINTAINERS file at the top of the Xen source tree. If there is a specific maintainer listed for the area of the code you are modifying then it is best to Cc the patches to them. You should always send patches to the xen-devel@lists.xen.org mailing list as well. The easier way to Cc the maintainers is to add the following field after your "Signed-off-by"
Cc: Joseph Foo <joseph.foo@example.com>

Then git will automagically add the Cc to the email when sending it.

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.

Git format-patch and send-email

The most robust way to send files is by using the git format-patch and send-email commands.

First, we will set up the configuration git send-email will use to send the patches. Edit your ~/.gitconfig file and add the following lines:

[user]
	name = John Smith
	email = john.smith@example.com
[sendemail]
	smtpserver = mail.example.com
	smtpserverport = 25
	smtpuser = johnsmith

Now we can proceed to generate the patches and send them. We will use format-patch to generate the patches and send-email to send them to the mailing list.

git format-patch --subject-prefix="PATCH RFC" -o outgoing/ HEAD~2

The format-patch command allows us to set a prefix to our patch series, in this case I've want them to be marked as "RFC", if you don't need to include any specific prefix in the subject you don't need to specify "subject-prefix", and git will use the default one "PATCH X/Y". I've also used the "-o" option so format patch outputs the patches to the "outgoing" folder. Lastly you have to specify the first commit ID that will be used to generate your series. This can either be something like HEAD~3 if your series has 3 patches, and HEAD is the last patch in the series. Alternatively you can also use pass a commit ID instead of HEAD, and all commits on top of commit ID will be outputted.

Before sending the patches I usually review them myself, this can avoid sending patches that contain mistakes that could be avoided, and will allow you to send less revisions. After you have checked that the patches are fine, you can send them using the send-email git command.

git send-email --compose outgoing/*

The "compose" option will allow you to write an introductory email to the series. This is recommended, so the reviewer can understand easily what you are trying to accomplish with this series. Finally git will ask you the mail address where patches should be sent, which the user will have to fill with xen-devel@lists.xen.org.

Sending Patches Manually

It is strongly recommended to use the git send-email command discussed above. However it is possible to send a patch manually using your regular mail client.

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 email-clients.txt for advice on how to avoid this in popular mail clients.

Review, Rinse & Repeat


After posting your patches you will hopefully see some response in the form of comments, patch review and eventually commit.

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.

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, this can be easily accomplished by using the format-patch --subject-prefix option.

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.

An example of a resend of a patch 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
...

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 it may be worth considering if there is anything you can do to improve the commit message or subject line of your patch to better attract the attention of the relevant people. Remember to include any Acked-by/Reviewed-by which you received in response to the previous post.

Common tasks


If you are working on a big or controversial patch series, it is very likely that you will have to submit several versions of them, and you will need to rebase your code to match the changes that will be committed to the repository between each revision of your series. You can do that easily with stg rebase. The first step is to update the "master" branch of your repository.

stg branch master
git pull

After that you will need to find the commit ID you want to rebase your series on top of, this can be done using git log. Once you have the commit ID, switch to the branch with your patches and execute:

stg rebase <commit-id>

This will pop all your patches, move the branch to the commit specified and then push your patches again. There's a chance that your patches don't apply cleanly on top of this commit, if this is the case stg will complain loudly, and you will have to manually edit the conflicting file. After editing the file, you can add it to your repository again and continue with the rebase:

git add <conflicting/file>
stg refresh
stg push

stg rebase is really useful, because it allows you to rebase your series on top of any commit, as an example, you can rebase one of your series on top of another, or rebase a series on top of staging changes.

What If


  • Your patch is really big?
  • 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

After your patch is committed

Changes committed to Xen by the committers are immediately available in the "staging" repositories, for example http://xenbits.xen.org/gitweb/?p=xen.git;a=shortlog;h=refs/heads/staging. They are then automatically tested, and if the tests pass the changes are propagated to the main tree http://xenbits.xen.org/gitweb/?p=xen.git;a=summary.

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.

How To Generate and Submit a Patch to Qemu-Xen

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

- qemu-xen-traditional: old and stable tree, only bug fixes are accepted in this tree at the moment;

- qemu-xen: new tree used for development, based on the latest stable branch of Upstream QEMU, that currently is available at qemu git repo. See QEMU Upstream on how to manually build it and use it to run VMs.


New features should be developed again the latest upstream QEMU first, see http://wiki.qemu.org/Contribute/SubmitAPatch, then upon request they can be backported to qemu-xen. Only patches that have been acked and committed to qemu.git are eligible to be backported to qemu-xen. In order to request a backport, send an email to xen-devel@lists.xen.org, specifying the original commit id in qemu.git and the reason why the patch should be backported to qemu-xen.

How to Generate, and Submit a Xen Patch to the Linux Tree

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.xen.org, linux-kernel@vger.kernel.org, and any relevant maintainers (which can be found by running scripts/get_maintainer.pl on your patch).