ToDo/CodeTransitions: Difference between revisions

From QEMU
No edit summary
(→‎Error reporting: error_is_set disappearing)
Line 40: Line 40:


* Use error_report() & friends instead of fprintf(stderr, ...).  Unlike fprintf(), it does the right thing in human monitor context.  It also adds program name and error location when appropriate, and supports timestamps (-msg timestamp=on).
* Use error_report() & friends instead of fprintf(stderr, ...).  Unlike fprintf(), it does the right thing in human monitor context.  It also adds program name and error location when appropriate, and supports timestamps (-msg timestamp=on).
* Avoid error_is_set(errp); it is simpler to just test for non-NULL *errp in the first place.


== Completed transitions ==
== Completed transitions ==

Revision as of 14:31, 28 April 2014

Code Transitions

This page exists to keep a record of API and style transitions in the QEMU codebase. It's quite common for us to introduce a new API or design guideline, but not be able to convert the whole of QEMU's existing code over to it at once. So at any point in time some of QEMU will still be using the old deprecated approaches.

The aim of this page is twofold:

  • as a quick reference for less frequent contributors, to give an idea of things to avoid copying from old code
  • to embarrass ourselves into maybe finishing off some of these transitions

Ongoing transitions

Making all devices QOM objects

Ideally all device models should be QOM objects (usually deriving from DeviceState or one of its subclasses). However we still have a fairly large number of devices which are coded in an ad-hoc way. These should all be converted to QOM but it is a big job. See QOMConventions for the latest guidance on how to structure a QOM device.

MemoryRegionOps old_mmio

The MemoryRegionOps struct has an old_mmio field which was added to make it simpler to convert devices to the MemoryRegion API (it allows use of separate byte, halfword and word read and write functions, rather than combined read and write functions which take the access size as a parameter). This is still being used by about 40 devices -- it would be nice to convert those to the new style and remove old_mmio entirely.

Using VMStateDescription rather than load and save functions

The preferred way to implement state save/load for migration is to describe the device state using a VMStateDescription struct. Some devices are still using the old vmstate_register() API, however; these all need converting. (This is often going to involve also converting the device to use QOM.)

Coding whitespace and brace style

New QEMU code should follow the style guidelines in CODING_STYLE, and in particular this means 4-space indent, no hardcoded tab, braces on all if() statements. However much of the codebase is old and doesn't follow this. Changes to areas of existing code should generally update the lines of code they're touching anyway, but we prefer to avoid wholesale "fix coding style" patches because they obscure the change history for tools like "git blame".

Error reporting

Several transitions are in flight here:

  • Avoid ErrorClass values other than ERROR_CLASS_GENERIC_ERROR unless you have a specific reason. Prefer error_setg() & friends over error_set() & friends.
  • The QError macros QERR_ are a hack to help with the transition to the current error.h API (human-readable message rather than JSON argument, see commit df1e608). Avoid them in new code, use simple message strings instead. If you use them for a new error in old code, double check the ErrorClass is ERROR_CLASS_GENERIC_ERROR.
  • qerror_report() is a transitional interface to help with converting existing HMP commands to QMP. It should not be used elsewhere. Use error_report() or error_setg() instead.
  • Use error_report() & friends instead of fprintf(stderr, ...). Unlike fprintf(), it does the right thing in human monitor context. It also adds program name and error location when appropriate, and supports timestamps (-msg timestamp=on).
  • Avoid error_is_set(errp); it is simpler to just test for non-NULL *errp in the first place.

Completed transitions

  • Everything is using the MemoryRegion API now!
  • Users of old_portio have all been updated and the support removed