Use the QEMU coding style

You can run runscripts/checkpatch.pl <patchfile> before submitting tocheck that you are in compliance with our coding standards. Be awarethatcheckpatch.pl is not infallible, though, especially where Cpreprocessor macros are involved; use some common sense too. See also:

• QEMU Coding Style

• Automate a checkpatch run oncommit

Base patches against current git master

There’s no point submitting a patch which is based on a released versionof QEMU because development will have moved on from then and it probablywon’t even apply to master. We only apply selected bugfixes to releasebranches and then only as backports once the code has gone into master.

It is also okay to base patches on top of other on-going work that isnot yet part of the git master branch. To aid continuous integrationtools, such aspatchew, you shouldadd ataglineBased-on:$MESSAGE_ID to your cover letter to make the seriesdependency obvious.

Split up long patches

Split up longer patches into a patch series of logical code changes.Each change should compile and execute successfully. For instance, don’tadd a file to the makefile in patch one and then add the file itself inpatch two. (This rule is here so that people can later use tools likegit bisect without hittingpoints in the commit history where QEMU doesn’t work for reasonsunrelated to the bug they’re chasing.) Put documentation first, notlast, so that someone reading the series can do a clean-room evaluationof the documentation, then validate that the code matched thedocumentation. A commit message that mentions “Also, …” is often agood candidate for splitting into multiple patches. For more thoughts onproperly splitting patches and writing good commit messages, seethisadvice fromOpenStack.

Make code motion patches easy to review

If a series requires large blocks of code motion, there are tricks formaking the refactoring easier to review. Split up the series so thatsemantic changes (or even function renames) are done in a separate patchfrom the raw code motion. Use a one-time setup ofgitconfigdiff.renamestrue;gitconfigdiff.algorithmpatience (refer togit-config). The ‘diff.renames’property ensures file rename patches will be given in a more compactrepresentation that focuses only on the differences across the filerename, instead of showing the entire old file as a deletion and the newfile as an insertion. Meanwhile, the ‘diff.algorithm’ property ensuresthat extracting a non-contiguous subset of one file into a new file, butwhere all extracted parts occur in the same order both before and afterthe patch, will reduce churn in trying to treat unrelated} lines inthe original file as separating hunks of changes.

Ideally, a code motion patch can be reviewed by doing:

gitformat-patch--stdout-1>patch;diff-u<(sed-n's/^-//p'patch)<(sed-n's/^\+//p'patch)

to focus on the few changes that weren’t wholesale code motion.

Don’t include irrelevant changes

In particular, don’t include formatting, coding style or whitespacechanges to bits of code that would otherwise not be touched by thepatch. (It’s OK to fix coding style issues in the immediate area (fewlines) of the lines you’re changing.) If you think a section of codereally does need a reindent or other large-scale style fix, submit thisas a separate patch which makes no semantic changes; don’t put it in thesame patch as your bug fix.

For smaller patches in less frequently changed areas of QEMU, considerusing theTrivial Patches process.

Write a meaningful commit message

Commit messages should be meaningful and should stand on their own as ahistorical record of why the changes you applied were necessary oruseful.

QEMU follows the usual standard for git commit messages: the first line(which becomes the email subject line) is “subsystem: single linesummary of change”. Whether the “single line summary of change” startswith a capital is a matter of taste, but we prefer that the summary doesnot end in a dot. Look atgitshortlog-30 for an idea of samplesubject lines. Then there is a blank line and a more detaileddescription of the patch, another blank and your Signed-off-by: line.Please do not use lines that are longer than 76 characters in yourcommit message (so that the text still shows up nicely with “git show”in a 80-columns terminal window).

The body of the commit message is a good place to document why yourchange is important. Don’t include comments like “This is a suggestionfor fixing this bug” (they can go below the--- line in the email sothey don’t go into the final commit message). Make sure the body of thecommit message can be read in isolation even if the reader’s mailerdisplays the subject line some distance apart (that is, a body thatstarts with “… so that” as a continuation of the subject line isharder to follow).

If your patch fixes a commit that is already in the repository, pleaseadd an additional line with “Fixes: <at-least-12-digits-of-SHA-commit-id>(“Fixed commit subject”)” below the patch description / before your“Signed-off-by:” line in the commit message.

If your patch fixes a bug in the gitlab bug tracker, please add a linewith “Resolves: <URL-of-the-bug>” to the commit message, too. Gitlab canclose bugs automatically once commits with the “Resolves:” keyword getmerged into the master branch of the project. And if your patch addressesa bug in another public bug tracker, you can also use a line with“Buglink: <URL-of-the-bug>” for reference here, too.

Example:

Fixes: 14055ce53c2d ("s390x/tcg: avoid overflows in time2tod/tod2time")Resolves: https://gitlab.com/qemu-project/qemu/-/issues/42Buglink: https://bugs.launchpad.net/qemu/+bug/1804323``

Some other tags that are used in commit messages include “Message-Id:”“Tested-by:”, “Acked-by:”, “Reported-by:”, “Suggested-by:”. Seegitlog for these keywords for example usage.

Test your patches

Although QEMU uses variousCI services that attempt to testpatches submitted to the list, it still saves everyone time if youhave already tested that your patch compiles and works. Because QEMUis such a large project the default configuration won’t create atesting pipeline on GitLab when a branch is pushed. See theCIvariable documentation for details on how to control therunning of tests; but it is still wise to also check that your patcheswork with a full build before submitting a series, especially if yourchanges might have an unintended effect on other areas of the code youdon’t normally experiment with. SeeTesting in QEMU for more details onwhat tests are available.

Also, it is a wise idea to include a testsuite addition as part ofyour patches - either to ensure that future changes won’t regress yournew feature, or to add a test which exposes the bug that the rest ofyour series fixes. Keeping separate commits for the test and the fixallows reviewers to rebase the test to occur first to prove it catchesthe problem, then again to place it last in the series so thatbisection doesn’t land on a known-broken state.

Use git-publish

If you already configured git send-email, you can simply usegit-publish to send series.

$ git checkout master -b my-feature$ # work on new commits, add your 'Signed-off-by' lines to each$ git publish$ ... more work, rebase on master, ...$ git publish # will send a v2

Each time you post a series, git-publish will create a local tag with the format<branchname>-v<version> to record the patch series.

When sending patch emails, ‘git publish’ will consult the output of‘scripts/get_maintainers.pl’ and automatically CC anyone listed as maintainersof the affected code. Generally you should accept the suggested CC list, butthere may sometimes be scenarios where it is appropriate to cut it down (eg oncertain large tree-wide cleanups), or augment it with other interested people.

If you cannot send patch emails

In rare cases it may not be possible to send properly formatted patchemails. You can usesourcehut to send yourpatches to the QEMU mailing list by following these steps:

1. Register or sign in to your account

2. Add your SSH public key inmeta |keys.

3. Publish your git branch usinggit push git@git.sr.ht:~USERNAME/qemuHEAD

4. Send your patches to the QEMU mailing list using the web-basedgit-send-email UI athttps://git.sr.ht/~USERNAME/qemu/send-email

Documentation for sourcehut is availablehere.

CC the relevant maintainer

Send patches both to the mailing list and CC the maintainer(s) of thefiles you are modifying. look in the MAINTAINERS file to find out whothat is. Also try using scripts/get_maintainer.pl from the repositoryfor learning the most common committers for the files you touched.

Example:

~/src/qemu/scripts/get_maintainer.pl -f hw/ide/core.c

In fact, you can automate this, via a one-time setup ofgitconfigsendemail.cccmd'scripts/get_maintainer.pl--nogit-fallback' (Refer togit-config.)

Do not send as an attachment

Send patches inline so they are easy to reply to with review comments.Do not put patches in attachments.

Usegitformat-patch

Use the right diff format.git format-patch willproduce patch emails in the right format (check the documentation tofind out how to drive it). You can then edit the cover letter beforeusinggitsend-email to mail the files to the mailing list. (Werecommendgit send-emailbecause mail clients often mangle patches by wrapping long lines ormessing up whitespace. Some distributions do not include send-email in adefault install of git; you may need to download additional packages,such as ‘git-email’ on Fedora-based systems.) Patch series need a coverletter, with shallow threading (all patches in the series arein-reply-to the cover letter, but not to each other); single unrelatedpatches do not need a cover letter (but if you do send a cover letter,use--numbered so the cover and the patch have distinct subject lines).Patches are easier to find if they start a new top-level thread, ratherthan being buried in-reply-to another existing thread.

Avoid posting large binary blob

If you added binaries to the repository, consider producing the patchemails usinggitformat-patch--no-binary and include a link to agit repository to fetch the original commit.

Patch emails must include aSigned-off-by: line

Your patchesmust include a Signed-off-by: line. This is a hardrequirement because it’s how you say “I’m legally okay to contributethis and happy for it to go into QEMU”. The process is modelled aftertheLinux kernelpolicy.

If you wrote the patch, make sure your “From:” and “Signed-off-by:”lines use the same spelling. It’s okay if you subscribe or contribute tothe list via more than one address, but using multiple addresses in onecommit just confuses things. If someone else wrote the patch, git willinclude a “From:” line in the body of the email (different from yourenvelope From:) that will give credit to the correct author; but again,that author’s Signed-off-by: line is mandatory, with the same spelling.

The name used with “Signed-off-by” does not need to be your legal name,nor birth name, nor appear on any government ID. It is the identity youchoose to be known by in the community, but should not be anonymous,nor misrepresent whom you are.

There are various tooling options for automatically adding these tagsinclude usinggitcommit-s orgitformat-patch-s. For moreinformation seeSubmittingPatches 1.12.

Include a meaningful cover letter

This is a requirement for any series with multiple patches (as it aidscontinuous integration), but optional for an isolated patch. The coverletter explains the overall goal of such a series, and also provides aconvenient 0/N email for others to reply to the series as a whole. Aone-time setup ofgitconfigformat.coverletterauto (refer togit-config) will generate thecover letter as needed.

When reviewers don’t know your goal at the start of their review, theymay object to early changes that don’t make sense until the end of theseries, because they do not have enough context yet at that point oftheir review. A series where the goal is unclear also risks a highernumber of review-fix cycles because the reviewers haven’t bought intothe idea yet. If the cover letter can explain these points to thereviewer, the process will be smoother patches will get merged faster.Make sure your cover letter includes a diffstat of changes made over theentire series; potential reviewers know what files they are interestedin, and they need an easy way determine if your series touches them.

Use the RFC tag if needed

For example, “[PATCH RFC v2]”.gitformat-patch--subject-prefix=RFCcan help.

“RFC” means “Request For Comments” and is a statement that you don’tintend for your patchset to be applied to master, but would like somereview on it anyway. Reasons for doing this include:

• the patch depends on some pending kernel changes which haven’t yetbeen accepted, so the QEMU patch series is blocked until thatdependency has been dealt with, but is worth reviewing anyway

• the patch set is not finished yet (perhaps it doesn’t cover all usecases or work with all targets) but you want early review of a majorAPI change or design structure before continuing

In general, since it’s asking other people to do review work on apatchset that the submitter themselves is saying shouldn’t be applied,it’s best to:

• use it sparingly

• in the cover letter, be clear about why a patch is an RFC, what areasof the patchset you’re looking for review on, and why reviewersshould care

Consider whether your patch is applicable for stable

If your patch fixes a severe issue or a regression, it may be applicablefor stable. In that case, consider addingCc:qemu-stable@nongnu.orgto your patch to notify the stable maintainers.

For more details on how QEMU’s stable process works, refer to theQEMU and the stable process page.

Stay around to fix problems raised in code review

Not many patches get into QEMU straight away – it is quite common thatdevelopers will identify bugs, or suggest a cleaner approach, or evenjust point out code style issues or commit message typos. You’ll need torespond to these, and then send a second version of your patches withthe issues fixed. This takes a little time and effort on your part, butif you don’t do it then your changes will never get into QEMU.

Remember that a maintainer is under no obligation to take yourpatches. If someone has spent the time reviewing your code andsuggesting improvements and you simply re-post without eitheraddressing the comment directly or providing additional justificationfor the change then it becomes wasted effort. You cannot demand othersmerge and then fix up your code after the fact.

When replying to comments on your patchesreply to all and not justthe sender – keeping discussion on the mailing list means everybodycan follow it. Remember the spirit of theCode of Conduct andkeep discussions respectful and collaborative and avoid makingpersonal comments.

Pay attention to review comments

Someone took their time to review your work, and it pays to respect thateffort; repeatedly submitting a series without addressing all commentsfrom the previous round tends to alienate reviewers and stall yourpatch. Reviewers aren’t always perfect, so it is okay if you want toargue that your code was correct in the first place instead of blindlydoing everything the reviewer asked. On the other hand, if someonepointed out a potential issue during review, then even if your codeturns out to be correct, it’s probably a sign that you should improveyour commit message and/or comments in the code explaining why the codeis correct.

If you fix issues that are raised during reviewresend the entirepatch series not just the one patch that was changed. This allowsmaintainers to easily apply the fixed series without having to manuallyidentify which patches are relevant. Send the new version as a completefresh email or series of emails – don’t try to make it a followup toversion 1. (This helps automatic patch email handling tools distinguishbetween v1 and v2 emails.)

When resending patches add a version tag

All patches beyond the first version should include a version tag – forexample, “[PATCH v2]”. This means people can easily identify whetherthey’re looking at the most recent version. (The first version of apatch need not say “v1”, just [PATCH] is sufficient.) For patch series,the version applies to the whole series – even if you only change onepatch, you resend the entire series and mark it as “v2”. Don’t try totrack versions of different patches in the series separately.gitformat-patch andgitsend-email both understandthe-v2 option to make this easier. Send each new revision as a newtop-level thread, rather than burying it in-reply-to an earlierrevision, as many reviewers are not looking inside deep threads for newpatches.

Include version history in patchset revisions

For later versions of patches, include a summary of changes fromprevious versions, but not in the commit message itself. In an emailformatted as a git patch, the commit message is the part above the---line, and this will go into the git changelog when the patch iscommitted. This part should be a self-contained description of what thisversion of the patch does, written to make sense to anybody who comesback to look at this commit in git in six months’ time. The part belowthe--- line and above the patch proper (git format-patch puts thediffstat here) is a good place to put remarks for people reading thepatch email, and this is where the “changes since previous version”summary belongs. Thegit-publish script can help withtracking a good summary across versions. Also, thegit-backport-diff script can help focusreviewers on what changed between revisions.

Proper use of Reviewed-by: tags can aid review

When reviewing a large series, a reviewer can reply to some of thepatches with a Reviewed-by tag, stating that they are happy with thatpatch in isolation (sometimes conditional on minor cleanup, like fixingwhitespace, that doesn’t affect code content). You should then updatethose commit messages by hand to include the Reviewed-by tag, so that inthe next revision, reviewers can spot which patches were already cleanfrom the previous round. Conversely, if you significantly modify a patchthat was previously reviewed, remove the reviewed-by tag out of thecommit message, as well as listing the changes from the previousversion, to make it easier to focus a reviewer’s attention to yourchanges.

If your patch seems to have been ignored

If your patchset has received no replies you should “ping” it after aweek or two, by sending an email as a reply-to-all to the patch mail,including the word “ping” and ideally also a link to the page for thepatch onpatchew orlore.kernel.org. It’s worthdouble-checking for reasons why your patch might have been ignored(forgot to CC the maintainer? annoyed people by failing to respond toreview comments on an earlier version?), but often for less-maintainedareas of QEMU patches do just slip through the cracks. If your ping isalso ignored, ping again after another week or so. As the submitter, youare the person with the most motivation to get your patch applied, soyou have to be persistent.

Is my patch in?

QEMU has some Continuous Integration machines that try to catch patchsubmission problems as soon as possible.patchew includes a web interface for tracking thestatus of various threads that have been posted to the list, and maysend you an automated mail if it detected a problem with your patch.

Once your patch has had enough review on list, the maintainer for thatarea of code will send notification to the list that they are includingyour patch in a particular staging branch. Periodically, the maintainerthen takes care ofSubmitting a Pull Requestfor aggregating topic branches into mainline QEMU. Generally, you do notneed to send a pull request unless you have contributed enough patchesto become a maintainer over a particular section of code. Maintainersmay further modify your commit, by resolving simple merge conflicts orfixing minor typos pointed out during review, but will always add aSigned-off-by line in addition to yours, indicating that it went throughtheir tree. Occasionally, the maintainer’s pull request may hit moredifficult merge conflicts, where you may be requested to help rebase andresolve the problems. It may take a couple of weeks between when yourpatch first had a positive review to when it finally lands in qemu.git;release cycle freezes may extend that time even longer.

Return the favor

Peer review only works if everyone chips in a bit of review time. Ifeveryone submitted more patches than they reviewed, we would have apatch backlog. A good goal is to try to review at least as many patchesfrom others as what you submit. Don’t worry if you don’t know the codebase as well as a maintainer; it’s perfectly fine to admit when yourreview is weak because you are unfamiliar with the code.