[Qemu-devel] [PATCH RFC 00/48] Convert device_add to QObject / QError

[Markus Armbruster]

Why this is such a big job?  There are two issues with a naive
conversion:

* Error message degradation

  The error messages are worded for -device.  They aren't so hot to
  begin with: we typically have many -device options, and to which one
  a message applies is often not obvious.

  Now, QMP wants relatively generic errors.  For instance, "-device:
  no driver specified" becomes "Parameter 'driver' is missing".
  Emitting such an error with our lengthy command lines is just too
  mean to users.

  However, if you know *where* the parameter is missing, the generic
  message is perfectly adequate: "-device a=b: Parameter 'driver' is
  missing".  In fact, it's even superior to our current message.

  So the first part of the patch series is about error locations.  I
  feel it's very useful all by itself.  I can split it off into its
  own patch series.  But then the rest of this series depends on it,
  so I'm not sure splitting is all that useful.

  We may still encounter cases where a generic message is not adequate
  even with precise location information.  Let's solve that problem
  when we actually encounter it.

* String argument with option syntax, i.e. NAME=VALUE,...

  QMP uses JSON to encode collections of name/value pairs.  Adding a
  second encoding for the same thing would be a mistake, in my
  opinion. 

  Note that we already have two competing encodings in our code: QDict
  and QemuOpts.  But we should not permit that to leak into an
  external interface like QMP.

  QemuOpts originated in the command line and spread from there into a
  few monitor commands, including device_add, and a few internal
  interfaces.

  QDict originated in the monitor.  It sits right at the interface
  between monitor core and command handlers.

  My proposed solution is modest and pragmatic:

  * Lift the parsing of arguments into QemuOpts from monitor handlers
    up into the human monitor core.  This removes QemuOpts from the
    handler interface, and thus avoids leaking it into QMP.  It's
    exactly what we did for other argument types with syntax
    inappropriate for QMP, such as arguments of migrate_set_speed and
    migrate_set_downtime (commits 9da92c49..b0fbf7d3).

  * Monitor handlers that need to pass their arguments in
    QemuOpts-form to internal interfaces use a converter function to
    translate from QDict to QemuOpts.

  This is what the last part of the patch series is about.  If you'd
  prefer a different solution, let's talk.

  I can split this part off into its own patch series if that helps.
  However, the patches before it aren't all that useful without it, so
  I'm not sure splitting buys us much.

  A possible alternative is to add the concept of optional named
  arguments to the monitor.  Instead of encoding multiple optional
  named arguments in a single positional argument, we encode them as
  multiple named arguments.  For instance, "device_add
  ide-drive,drive=hda,bus=ide.0,unit=0" becomes "device_add ide-drive
  drive=hda bus=ide.0 unit=0".

  Of course, if you think that adding a second encoding for
  collections of name/value pairs to QMP is fine, then this last part
  can be dropped.

So, the series starts with error locations (part I), and ends with
keeping QemuOpts out of QMP (part III).  Wedged in between is the
conversion of device_add to QError (part II).  In more detail:

Part I: Error Locations

  [01-07] Preliminary cleanup & fixes
     [08] Separate "default monitor" and "current monitor" cleanly
  [09-16] More cleanup
  [17-21] Error Locations

Part II: Convert device_add to QError

  [22-25] Preliminary qdev cleanup & fixes
  [26-42] Convert device_add to QError

Part III

     [43] Conversions between QDict and QemuOpts
  [44-46] New monitor argument type O
  [47-48] Convert device_add to QObject

I cut a few corners clearly marked in commit messages and code.  I'll
fix them up for the non-RFC submit.


Markus Armbruster (48):
  monitor: Factor monitor_set_error() out of qemu_error_internal()
  error: Move qemu_error() & friends from monitor.c to own file
  usb: Remove disabled monitor_printf() in usb_read_file()
  savevm: Fix -loadvm to report errors to stderr, not the monitor
  pc: Fix error reporting for -boot once
  pc: Factor common code out of pc_boot_set() and cmos_init()
  tools: Remove unused cur_mon from qemu-tool.c
  monitor: Separate "default monitor" and "current monitor" cleanly
  block: Simplify usb_msd_initfn() test for "can read bdrv key"
  error: Simplify error sink setup
  error: Move qemu_error & friends into their own header
  error: New error_printf() and error_vprintf()
  error: Make qemu_error() add a newline, strip it from arguments
  error: Don't abuse qemu_error() for non-error in scsi_hot_add()
  error: Don't abuse qemu_error() for non-error in qdev_device_help()
  error: Don't abuse qemu_error() for non-error in qbus_find()
  error: Infrastructure to track locations for error reporting
  error: Include the program name in error messages to stderr
  error: Track locations in configuration files
  QemuOpts: Fix qemu_config_parse() to catch file read errors
  error: Track locations on command line
  qdev: Fix -device and device_add to handle unsuitable bus gracefully
  qdev: Factor qdev_create_from_info() out of qdev_create()
  qdev: Hide "no_user" devices from users
  qdev: Hide "ptr" properties from users
  error: Polish human-readable error descriptions
  error: New QERR_PROPERTY_NOT_FOUND
  error: New QERR_PROPERTY_VALUE_BAD
  qdev: convert setting device properties to QError
  qdev: Relax parsing of bus option
  error: New QERR_BUS_NOT_FOUND
  error: New QERR_DEVICE_MULTIPLE_BUSSES
  error: New QERR_DEVICE_NO_BUS
  qdev: Convert qbus_find() to QError
  monitor: New in_qmp_mon()
  error: New error_printf_unless_qmp()
  error: New QERR_BAD_BUS_FOR_DEVICE
  error: New QERR_BUS_NO_HOTPLUG
  error: New QERR_DEVICE_INIT_FAILED
  error: New QERR_NO_BUS_FOR_DEVICE
  Revert "qdev: Use QError for 'device not found' error"
  error: Convert do_device_add() to QError
  qemu-option: Functions to convert to/from QDict.
  qemu-option: Move the implied first name into QemuOptsList
  qemu-option: Rename find_list() to qemu_find_opts() & external
    linkage
  monitor: New argument type 'O'
  monitor: Use argument type 'O' for device_add
  monitor: convert do_device_add() to QObject

 Makefile.target        |    1 +
 audio/audio.c          |    4 +-
 hw/pc.c                |   35 +++-----
 hw/pci-hotplug.c       |   13 ++--
 hw/pci.c               |    8 +-
 hw/qdev-properties.c   |   27 ++----
 hw/qdev.c              |  221 +++++++++++++++++++++++++++--------------------
 hw/qdev.h              |    2 +-
 hw/scsi-bus.c          |    4 +-
 hw/scsi-disk.c         |    5 +-
 hw/scsi-generic.c      |    9 +-
 hw/usb-bus.c           |    4 +-
 hw/usb-msd.c           |    4 +-
 hw/usb-net.c           |    2 +-
 hw/usb-serial.c        |    9 +-
 hw/virtio-net.c        |    5 +-
 hw/virtio-pci.c        |    4 +-
 hw/virtio-serial-bus.c |    2 +-
 monitor.c              |  191 ++++++++++++++++++------------------------
 monitor.h              |    7 ++
 net.c                  |   28 +++---
 net/dump.c             |    5 +-
 net/slirp.c            |   20 ++--
 net/socket.c           |   12 ++--
 net/tap-bsd.c          |    3 +-
 net/tap-linux.c        |    5 +-
 net/tap-win32.c        |    2 +-
 net/tap.c              |    3 +-
 qemu-config.c          |   54 ++++++++----
 qemu-config.h          |    3 +-
 qemu-error.c           |  222 ++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-error.h           |   47 ++++++++++
 qemu-monitor.hx        |    7 +-
 qemu-option.c          |   68 +++++++++++++++-
 qemu-option.h          |    6 +-
 qemu-tool.c            |   28 ++++++-
 qerror.c               |   73 ++++++++++++----
 qerror.h               |   45 ++++++++--
 savevm.c               |   27 +++---
 slirp/misc.c           |    2 +-
 sysemu.h               |   13 +---
 usb-linux.c            |    8 --
 vl.c                   |   44 ++++++----
 vnc.c                  |    5 +-
 44 files changed, 858 insertions(+), 429 deletions(-)
 create mode 100644 qemu-error.c
 create mode 100644 qemu-error.h