KernelBugFixing

Differences between revisions 3 and 4
Revision 3 as of 2008-11-22 10:05:05
Size: 10306
Editor: a91-154-121-48
Comment:
Revision 4 as of 2008-11-27 09:52:38
Size: 11618
Editor: 79-66-201-225
Comment:
Deletions are marked like this. Additions are marked like this.
Line 23: Line 23:
The patch is not the same but the details of the patch itself are not that important. The patch is not the same (it is a backport to the hardy kernel gfs2 version) but the details of the patch itself are not that important.
Line 27: Line 27:
This bug is reported in the Hardy kernel so we get a fresh kernel tree for hardy the hardy
repository on ''zinc.canonical.com''.
This bug is reported in the Hardy kernel so we need to start with the latest hardy kernel tree, from the hardy repository on ''zinc.canonical.com''. If you do not yet have one you will need to ''clone'' the Hardy repository, otherwise you will want to ensure your repository is up to date.

=== Cloning a new repository ===
Line 38: Line 39:
An alternative method is to
keep a current local clone of Linus's tree and only clone the difference
One optimisation is to keep a current local clone of Linus's tree and use that as a seed for the clone, allowing git to only clone the difference
Line 46: Line 46:
 /!\ Note the {{{--reference}}} which points to "linux-linus" which is the directory name of the git tree at the same level as the git we are about to create. This is a clone of the linus tree that is always kept current and is read-only (by convention). This is a git "good practice".
 /!\ Note the {{{--reference}}} which in this case points to "linux-linus" which is a path to the local clone of Linus' git tree, here at the same level as the git we are about to create. This Linus tree always kept current and is read-only (by convention). This is a git "good practice".

=== Updating an existing repository ===

If you already have a hardy repository you will need to ensure it is up to date with all changes in the main repository. To do this simply change directory into the repository and request a fetch (here we assume that this tree was cloned directly from the main repository as detailed in the previous section):

{{{
git fetch origin
}}}

== Create a bug specific branch ==

As we have no control over the order in which patches will be accepted into the main tree it is helpful if all bugs are applied relative to the current development tip. The easiest way to do this is to work on bug specific branches within your repository.
Line 52: Line 63:
git checkout -b lp276641
}}}

The upstream patch is a performance optimization. The patch we find in
git checkout -b lp276641 origin/master
}}}


== Apply the Patch ==

In our example t
he upstream patch is a performance optimization. The patch we find in
Line 59: Line 73:
== Apply the Patch ==
Line 67: Line 80:
NOTE: if the patch updates the debian/changelog you will need to undo any changes there, the changelog is generated automatically at release time from the git commit history; {{{git checkout debian/changelog}}} will undo any changes here.

== Test Build the patch ==
Line 109: Line 125:
Line 122: Line 139:
came from upstream.

Filling in the {{{Bug}}} field is important since it is automatically linked to Launchpad bug (LP: xxxxxx) and the bug status is changed automatically.
came from upstream. It is very important that the one line summary (first line) is separated from the remainder to prevent the Ubuntu tag fields being pulled up into the summary.

Filling in the {{{Bug}}} field is important since it is automatically linked to Launchpad bug (LP: xxxxxx) and the bug status is changed automatically as the released package makes its way into -proposed and -updates.
Line 176: Line 193:
Once the ''kernel-team'' replies back with approvals, we update the commit
to add the approvals and send another email. The email also includes
a pull request to the main repo.

== Push the Change ==
If you are reading this page and going through the process for the first time
you probably do not have access to push your change directly.
Therefore, we email the ''kernel-team'' and request a pull.

== Apply approvals ==
Line 198: Line 209:
Since we cannot do the push directly, we push it to our personal repository on
'''kernel.ubuntu.com''' and request that the owner of the repository pull the patch from there.
This is where the branch-per-patch becomes useful.
== Push the Change ==

If you are reading this page and going through the process for the first time
you probably do not have access to push your change directly to the master repository.
Therefore we must publish the change to a personal repository on ''kernel.ubuntu.com'' and then request the change be pulled into the main tree. This is where the branch-per-patch becomes useful.
Line 205: Line 218:
To do that, we log into '''kernel.ubuntu.com''', and create our own repository as a ''bare''
clone of the master.
To do that, we log into '''kernel.ubuntu.com''', and create our own repository as a ''bare'' clone of the master.
Line 223: Line 235:
This entry in {{{.git/config}}} is now edited to resemble the following. This entry in {{{.git/config}}} should now resemble the following.
Line 233: Line 245:
Note the {{{+ssh}}}. This allows us to do the push. Note also that the URL is changed as well
to reflect that we are using it for a ''push''.
Note the {{{+ssh}}}. This allows us to do the push. Note also that the URL is changed as well to reflect that we are using it for a ''push''.
Line 238: Line 249:
as '''up''' in our repository config, We can just push the branch up. as '''up''' in our repository config, we can just push the branch as below:
Line 246: Line 257:
=== Requesting a pull ===

DRAFT DRAFT as in a work in progress

A day in the life of a fixed bug

This page is the condensation of notes and discussion that took place during a kernel developer training. The bug is real and we assume that none of us have a clue (yet) on how to process bugs or add new code in the Canonical/Ubuntu way. We do assume we all know how to use editors, fix and build kernels, and boot machines.

We have a bug and its fix. In this case, it is a gfs2 bug that we will fix in the Hardy kernel. It was chosen because the patch already exists. Our job is to go through the steps to get it on its way into a proposed kernel and eventually into the release.

Checking upstream commit by way of the link (reference) in Launchpad. The commit itself is not in the Linus tree but it is in the maintainer's tree (linux/kernel/steve/gfs2-2.6). The patch is not the same (it is a backport to the hardy kernel gfs2 version) but the details of the patch itself are not that important. What we will do is add the patch by hand and test it.

Start with the Source

This bug is reported in the Hardy kernel so we need to start with the latest hardy kernel tree, from the hardy repository on zinc.canonical.com. If you do not yet have one you will need to clone the Hardy repository, otherwise you will want to ensure your repository is up to date.

Cloning a new repository

The simple way is to simply clone the Hardy repository.

git clone git://kernel.ubuntu.com/ubuntu/ubuntu-hardy.git 

This pulls down the whole hardy tree which takes a while and takes up a lot of space that is not really needed for this work. See KernelGitGuide for more detail. One optimisation is to keep a current local clone of Linus's tree and use that as a seed for the clone, allowing git to only clone the difference between the two, saving both download time and space.

git clone --reference linux-linus git://kernel.ubuntu.com/ubuntu/ubuntu-hardy.git 
  • Warning /!\ Note the --reference which in this case points to "linux-linus" which is a path to the local clone of Linus' git tree, here at the same level as the git we are about to create. This Linus tree always kept current and is read-only (by convention). This is a git "good practice".

Updating an existing repository

If you already have a hardy repository you will need to ensure it is up to date with all changes in the main repository. To do this simply change directory into the repository and request a fetch (here we assume that this tree was cloned directly from the main repository as detailed in the previous section):

git fetch origin

Create a bug specific branch

As we have no control over the order in which patches will be accepted into the main tree it is helpful if all bugs are applied relative to the current development tip. The easiest way to do this is to work on bug specific branches within your repository. We can now create a branch named after our bug to hold our patch. Branches are cheap and this will isolate what we do to just the patch.

git checkout -b lp276641 origin/master

Apply the Patch

In our example the upstream patch is a performance optimization. The patch we find in launchpad is a fix on top of it. Follow the link in comment #8 to the patch.

It started as a debdiff. We will take just the patch itself from the attachment in the launchpad comment and apply it by hand.

patch -p1 <~/PATCH

NOTE: if the patch updates the debian/changelog you will need to undo any changes there, the changelog is generated automatically at release time from the git commit history; git checkout debian/changelog will undo any changes here.

Test Build the patch

  • Warning /!\ Assume we added alias fdr='fakeroot debian/rules' into our ~/.bashrc to make things a bit easier to read.

The first step is to prepare the build if this git tree is fresh.

fdr prepare-generic

We can then build one flavor of the kernel package to see if it works.

fdr binary-debs flavours=generic.   #note the .uk spelling.

A second way is to just build the portion of the tree that we applied the patch to. We can try this here because the patch is simple and has already been tested. We want just make sure that the patch applied cleanly.

make -C debian/build/build-generic M=`pwd`/fs/gfs2

This approach should only be used in known circumstances. If there is any chance that that the new tree may have new dependencies, use debuild -b to do a clean of the tree followed complete build. This will check the sanity of the environment and the source pool.

Commit the Patch

We will now commit the patch using a template in the hardy tree.

git commit -a -s -F debian/commit-templates/sauce-patch

The -a adds the changes to the index first and the -s adds our signoff. The sauce template comes up in our favorite editor and we edit the fields. Here is what the commit from the sauce template looks like after we edited it in vi:

    UBUNTU: SAUCE: replace gfs2_bitfit with upstream version to prevent oops

    OriginalAuthor: Sergio Tosti <zeno979@gmail.com>
    Bug: #276641
    
    Backport of the recent mainline gfs2_bitfit() to fix occasional OOPS.
    This is already fixed in mainline and does not apply to Intrepid or
    later.
    
    Signed-off-by: Andy Whitcroft <apw@canonical.com>

Everything in the template with a # at the beginning of the line is stripped. The key fields here are the first line, Bug, and Signed-off-by. The OriginalAuthor is included where appropriate, particularly if the patch came from upstream. It is very important that the one line summary (first line) is separated from the remainder to prevent the Ubuntu tag fields being pulled up into the summary.

Filling in the Bug field is important since it is automatically linked to Launchpad bug (LP: xxxxxx) and the bug status is changed automatically as the released package makes its way into -proposed and -updates.

Forward the Commit for Approval

The next step to pass the patch on for approval is manual. Use your favorite mail agent to send the email to the kernel-team list to have it reviewed:

First we generate the patch and its diff.

git format-patch HEAD~1..HEAD

We have just the one commit so this captures it into a file named "0001-<the first line of the commit>.patch". We now send it with

git send-email \
        --from 'Andy Whitcroft <apw@canonical.com>' \
        --identity canonical \
        --no-chain-reply-to \
        --thread \
        --suppress-cc all \
        --to kernel-team@lists.ubuntu.com \
    ./????-*

The "????-*" is our patch file. The editor comes up and we enter the introduction of our submittal. This is what it looks like just before we send it.

From: Andy Whitcroft <apw@canonical.com>
To: kernel-team@lists.ubuntu.com
Subject: [PATCH 0/1] SRU BUG #276641 -- fix gfs2_bitfit OOPS

Hardy is vunerable to a gfs2 OOPS when copying large files.  I have built
kernels with this patch applied.  The submitter has tested it fixes the
issue.  This bug is already fixed in mainline, Intrepid is already immune.

Following this email is the patch as attached to the bug.

Proposing this for SRU to Hardy.

-apw

The diffstat would be included by the script. In this case, the patch itself is in email 1/1. This may be an iterative process as we address issues that other developers may raise.

Apply approvals

Before we push the change, we must include the approvals we received from other developers on kernel-team. Open the commit and add the approvals after the Signed-off-by line in the commit we did earlier.

git commit --amend

This will change the commit name but a git show will display it for us to cut-and-paste to the submission email. The patch is now ready for the main repository.

Push the Change

If you are reading this page and going through the process for the first time you probably do not have access to push your change directly to the master repository. Therefore we must publish the change to a personal repository on kernel.ubuntu.com and then request the change be pulled into the main tree. This is where the branch-per-patch becomes useful.

Personal repository setup

Before we actually do the push, we need a repository to push to. Since we assumed that we have not done this before, we have to create it first. To do that, we log into kernel.ubuntu.com, and create our own repository as a bare clone of the master.

# we got here by way of ssh...
cd /srv/kernel.ubuntu.com/git/apw  # created for us by IT...
git clone --bare ../ubuntu/ubuntu-hardy.git/ ubuntu-hardy.git

This results in a repository that we can push our changes to although it cannot be used for anything else. The next step is to link the local repository to it so we can push the change without breaking our back. Add a remote called up with

git remote add up git://zinc.canonical.com/srv/kernel.ubuntu.com/git/<your-login>/<repository>.git

This entry in .git/config should now resemble the following. Remember that your-login and repository need to be set appropriately.

[remote "up"]
        url = git+ssh://zinc.canonical.com/srv/kernel.ubuntu.com/git/apw/ubuntu-hardy.git
        fetch = +refs/heads/*:refs/remotes/origin/*

Note the +ssh. This allows us to do the push. Note also that the URL is changed as well to reflect that we are using it for a push.

Pushing the Change

We now push it up to our repository on kernel.ubuntu.com. If we added that repository as up in our repository config, we can just push the branch as below:

git push up +lp276641
  • Warning /!\ Note the + in +lp276641. This will overwrite the branch! We may have pushed the branch earlier so that others could access the patch for testing. DO NOT do this to the master repository!

Requesting a pull

The last step is to send an email to kernel-team requesting a pull of the change. The subject line should contain:

  • SRU LP <launchpad bug number> <summary line from bug.

and the body should contain the following contents although it can be free-form:

  • A link to the Launchpad bug, i.e. https://bugs.launchpad.net/ubuntu/+source/linux/+bug/276641

  • A link to the repository where the patch can be found, and
  • The commit name.

The following is an example body from another bug:

The patch with updated approvals and commit message is available at:

git://kernel.ubuntu.com/lieb/ubuntu-intrepid.git lp292429

The commit is:
e277e90a1aa969d957d16f0aa53ff33a02e2da8f

The appropriate person on the kernel team will pull the patch from the named repository and commit it to the master repository.

The SRU Sequence

At this point in the process flow, the patch is ready to be nominated for inclusion into the release or update.

KernelTeam/KernelBugFixing (last edited 2010-06-30 17:25:49 by c-76-105-148-120)