Re: [RFC PATCH v2 06/10] .agents/skills: add qemu-code-reviewer skill

["Alex Bennée" <alex.bennee@linaro.org>]

Chao Liu <chao.liu.zevorn@gmail.com> writes:

> On Mon, May 11, 2026 at 06:04:55PM +0100, Alex Bennée wrote:
>> This provides basic instructions for how to download and apply a
>> patch series from the mailing list.
>> 
>> Currently it is only taught about checkpatch but we could consider
>> adding common code smells and review comments that come up frequently.
>> 
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> 
>> ---
>> v2
>>   - merge checkpatch skill
>>   - moved later in series to leverage build and test skills
>>   - properly reference qemu-build and qemu-tester skills
>>   - mention the qemu-code-explorer skill for navigation
>> ---
>>  .agents/skills/qemu-code-reviewer/SKILL.md | 93 ++++++++++++++++++++++
>>  AGENTS.md                                  |  1 +
>>  2 files changed, 94 insertions(+)
>>  create mode 100644 .agents/skills/qemu-code-reviewer/SKILL.md
>> 
>> diff --git a/.agents/skills/qemu-code-reviewer/SKILL.md b/.agents/skills/qemu-code-reviewer/SKILL.md
>> new file mode 100644
>> index 00000000000..48ec6b07520
>> --- /dev/null
>> +++ b/.agents/skills/qemu-code-reviewer/SKILL.md
>> @@ -0,0 +1,93 @@
>> +---
>> +name: qemu-code-reviewer
>> +description: Pull and apply patch series from mailing lists for review and testing in QEMU, including style and build validation.
>> +license: GPL-2.0-or-later
>> +---
>> +
>> +# QEMU Code Reviewer Skill
>> +
>> +This skill provides instructions on how to retrieve patch series submitted to the QEMU mailing list (`qemu-devel@nongnu.org`) using `b4` or manual methods.
>> +
>> +## Using b4 (Recommended)
>> +
>> +`b4` is the preferred tool for working with patch series from public-inbox instances like `lore.kernel.org`.
>> +
>> +### 1. Fetching a series
>> +To download a series and prepare it for `git am`:
>> +```bash
>> +b4 am <message-id-or-url>
>> +```
>> +This creates a `.mbx` file containing the entire series, properly ordered.
>> +
>> +### 2. Applying a series directly
>> +To apply a series directly to your current branch:
>> +```bash
>> +b4 shazam <message-id-or-url>
>> +```
>> +This is often the fastest way to get a series ready for testing.
>> +
>> +### 3. Creating a local branch for the series
>> +```bash
>> +b4 am -t <message-id-or-url>
>> +git am ./*.mbx
>> +```
>> +The `-t` flag (or `--trust-all`) can be useful if you know the source.
> I suggest using the git worktree command for workspace isolation before
> applying patches.

I don't know if this gets too much into the weeds of per-developer
preference. Everyone has slightly different ways of working but they do
want confidence that when they hand the wheel to their agent it isn't go
to mess up their nice flow.

>
>> +
>> +## Manual mbox Retrieval (Alternative)
>> +
>> +If `b4` is unavailable, you can fetch the mbox manually from `lore.kernel.org`.
>> +
>> +### 1. Locate the thread
>> +Find the patch series on [lore.kernel.org/qemu-devel/](https://lore.kernel.org/qemu-devel/).
>> +
>> +### 2. Download the mbox
>> +Every thread on lore has an `mbox.gz` link. You can use `curl` or `wget`:
>> +```bash
>> +curl -L "https://lore.kernel.org/qemu-devel/<message-id>/raw" -o series.mbox
>> +```
>> +*Note: Appending `/raw` to the message URL usually provides the mbox format.*
>> +
>> +### 3. Apply with git am
>> +```bash
>> +git am series.mbox
>> +```
>> +
>> +## Post-Application Steps
>> +
>> +Once the patches are applied, you should perform initial validation:
>> +
>> +### 1. Style Check
>> +Run the QEMU checkpatch script to ensure the patches follow the project's coding style.
>> +
>> +- **Check applied patches**:
>> +  ```bash
>> +  ./scripts/checkpatch.pl master..HEAD
>> +  ```
>> +- **Check a specific commit**:
>> +  ```bash
>> +  ./scripts/checkpatch.pl <commit-hash>^..
>> +  ```
>> +- **Check a specific file**:
>> +  ```bash
>> +  ./scripts/checkpatch.pl -f <file-path>
>> +  ```
>> +- **Strict mode** (often required for new code or specific subsystems):
>> +  ```bash
>> +  ./scripts/checkpatch.pl --strict <commit-range>
>> +  ```
>> +
>> +### 2. Build and Test
>> +Refer to the `AGENTS.md` or the `qemu-build` and `qemu-testing` skills for build and test instructions.
>> +- Ensure you are in a clean build directory.
>> +- Run `ninja` or `make`.
>> +- Run relevant tests (e.g., `make check-qtest`).
>> +
>> +### 3. Reviewing Patches
>> +Refer to the `qemu-code-explorer` skill for navigating the code base and resolving functions and where they are called from.
>> +
> If the patches have multiple versions, should we ask the agent to summarize
> the review history of the previous versions, as well as briefly summarize
> the discussion across the current patch thread? This can be easy to miss
> during manual review, but an agent can handle it well.

That's a good idea. I've had good success in getting agents to review
threads and summarise what was asked for. I have even had them apply the
changes for me to review before committing.

>
> However, this seems better suited as a new skill.

The qemu-mail-thread skill is a general purpose thread handling skill we
could teach about versioning?

>
> Thanks,
> Chao
>> +
>> +
>> +## Common Troubleshooting
>> +
>> +- **Applying fails**: If `git am` fails due to conflicts, you may need to use `git am --3way` or manually resolve conflicts.
>> +- **Missing dependencies**: Ensure your tree is up to date with the base branch the patches were intended for (usually `master`).
>> diff --git a/AGENTS.md b/AGENTS.md
>> index d9d1964d45a..fbbc3b65ed0 100644
>> --- a/AGENTS.md
>> +++ b/AGENTS.md
>> @@ -27,6 +27,7 @@ You should use the following specialized skills for common tasks:
>>  - `qemu-code-explorer`: For finding where things are defined, how they're used, or understanding a specific subsystem.
>>  - `qemu-build`: For configuring and building QEMU (including debug and sanitizer builds).
>>  - `qemu-testing`: For finding, listing, and running individual tests (Unit, QTest, Functional, TCG).
>> +- `qemu-code-reviewer`: For pulling and applying patch series from mailing lists.
>>  
>>  ## Source Code Layout (see `docs/devel/codebase.rst`)
>>  - **`accel/`**: Hardware accelerators (KVM, TCG, HVF, Xen, etc.) and architecture-agnostic acceleration code.
>> -- 
>> 2.47.3
>> 
>> 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro