Re: [Qemu-devel] [PATCH v10 00/20] gdbstub: Refactor command packets handler

[Alex Bennée]

Jon Doron <address@hidden> writes:

> This patch series refactors the old gdbstub command packets handler
> with a new infrastructure which should ease extending and adding new
> and missing gdb command packets.
>
> version 10 changes:
> - Remove kvm added API as this is not really required and can be
>   accomplished by defining a coprocessor callback with a system
>   specific xml (see: 200bf5b7ffea635079cc05fdfb363372b9544ce7)
> - Remove the new QEMU extended commands to read KVM MSRs
> - Various fixes from Code Review by Alex Bennee
> - Change the QEMU specific command to read physical memory to non-User
> QEMU

You don't seem to have applied an of my Reviewed-by: tags from previous
iterations. It is mentioned in:

  
https://wiki.qemu.org/Contribute/SubmitAPatch#Proper_use_of_Reviewed-by:_tags_can_aid_review

but admittedly it's a bit buried in a wall of text. Hopefully the
process diagram makes it clearer:

  https://wiki.qemu.org/Contribute/DevelopmentProcess

It's also worth including a list of which patches remain un-reviewed in
your summary cover text. This helps reviewers skip patches that don't
need attention which is important as there are always too many patches
and not enough time...

Could you please re-spin v11 with tags applied to the relevant patches?

>
> version 9 changes:
> - checkpatch fixes
>
> version 8 changes:
> - Add new command to display the Supported qemu generic query/sets
> - kvm: Add API to read/write a MSR
> - Add new commands specific for qemu:
>   * Command to swap the memory GDB sees to be the physical memory
>   * Commands to read and write a MSR
>
> version 7 changes:
> - Fixed few checkpatch complaints
> - Feedback from Alex Bennee
>
> version 4-6 changes:
> - mostly feedback from Richard Henderson
>
> version 3 changes
> - Split the single patch to many individual patches for easier reviewing
>
> version 2 changes
> - Code convention fixes
>
> Jon Doron (20):
>   gdbstub: Add infrastructure to parse cmd packets
>   gdbstub: Implement deatch (D pkt) with new infra
>   gdbstub: Implement thread_alive (T pkt) with new infra
>   gdbstub: Implement continue (c pkt) with new infra
>   gdbstub: Implement continue with signal (C pkt) with new infra
>   gdbstub: Implement set_thread (H pkt) with new infra
>   gdbstub: Implement breakpoint commands (Z/z pkt) with new infra
>   gdbstub: Implement set register (P pkt) with new infra
>   gdbstub: Implement get register (p pkt) with new infra
>   gdbstub: Implement write memory (M pkt) with new infra
>   gdbstub: Implement read memory (m pkt) with new infra
>   gdbstub: Implement write all registers (G pkt) with new infra
>   gdbstub: Implement read all registers (g pkt) with new infra
>   gdbstub: Implement file io (F pkt) with new infra
>   gdbstub: Implement step (s pkt) with new infra
>   gdbstub: Implement v commands with new infra
>   gdbstub: Implement generic set/query (Q/q pkt) with new infra
>   gdbstub: Implement target halted (? pkt) with new infra
>   gdbstub: Clear unused variables in gdb_handle_packet
>   gdbstub: Implement qemu physical memory mode
>
>  gdbstub.c | 1753 ++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 1262 insertions(+), 491 deletions(-)


--
Alex Bennée