qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH RFC 00/48] Convert device_add to QObject / QError
@ 2010-02-24 17:55 Markus Armbruster
  2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 01/48] monitor: Factor monitor_set_error() out of qemu_error_internal() Markus Armbruster
                   ` (49 more replies)
  0 siblings, 50 replies; 77+ messages in thread
From: Markus Armbruster @ 2010-02-24 17:55 UTC (permalink / raw)
  To: qemu-devel

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

^ permalink raw reply	[flat|nested] 77+ messages in thread

end of thread, other threads:[~2010-03-02 12:37 UTC | newest]

Thread overview: 77+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-24 17:55 [Qemu-devel] [PATCH RFC 00/48] Convert device_add to QObject / QError Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 01/48] monitor: Factor monitor_set_error() out of qemu_error_internal() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 02/48] error: Move qemu_error() & friends from monitor.c to own file Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 03/48] usb: Remove disabled monitor_printf() in usb_read_file() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 04/48] savevm: Fix -loadvm to report errors to stderr, not the monitor Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 05/48] pc: Fix error reporting for -boot once Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 06/48] pc: Factor common code out of pc_boot_set() and cmos_init() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 07/48] tools: Remove unused cur_mon from qemu-tool.c Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 08/48] monitor: Separate "default monitor" and "current monitor" cleanly Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 09/48] block: Simplify usb_msd_initfn() test for "can read bdrv key" Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 10/48] error: Simplify error sink setup Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 11/48] error: Move qemu_error & friends into their own header Markus Armbruster
2010-02-26 19:43   ` Luiz Capitulino
2010-03-01  8:48     ` Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 12/48] error: New error_printf() and error_vprintf() Markus Armbruster
2010-02-26 19:43   ` Luiz Capitulino
2010-03-01  8:54     ` Markus Armbruster
2010-03-01 12:45       ` [Qemu-devel] " Paolo Bonzini
2010-03-01 16:09       ` [Qemu-devel] " Luiz Capitulino
2010-03-02  8:33         ` Markus Armbruster
2010-03-02 12:37           ` [Qemu-devel] " Paolo Bonzini
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 13/48] error: Make qemu_error() add a newline, strip it from arguments Markus Armbruster
2010-02-26 19:44   ` Luiz Capitulino
2010-03-01  8:55     ` Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 14/48] error: Don't abuse qemu_error() for non-error in scsi_hot_add() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 15/48] error: Don't abuse qemu_error() for non-error in qdev_device_help() Markus Armbruster
2010-02-26 19:44   ` Luiz Capitulino
2010-03-01  9:05     ` Markus Armbruster
2010-03-01 16:11       ` Luiz Capitulino
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 16/48] error: Don't abuse qemu_error() for non-error in qbus_find() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 17/48] error: Infrastructure to track locations for error reporting Markus Armbruster
2010-02-26 19:45   ` Luiz Capitulino
2010-03-01  9:19     ` Markus Armbruster
2010-03-01 16:15       ` Luiz Capitulino
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 18/48] error: Include the program name in error messages to stderr Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 19/48] error: Track locations in configuration files Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 20/48] QemuOpts: Fix qemu_config_parse() to catch file read errors Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 21/48] error: Track locations on command line Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 22/48] qdev: Fix -device and device_add to handle unsuitable bus gracefully Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 23/48] qdev: Factor qdev_create_from_info() out of qdev_create() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 24/48] qdev: Hide "no_user" devices from users Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 25/48] qdev: Hide "ptr" properties " Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 26/48] error: Polish human-readable error descriptions Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 27/48] error: New QERR_PROPERTY_NOT_FOUND Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 28/48] error: New QERR_PROPERTY_VALUE_BAD Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 29/48] qdev: convert setting device properties to QError Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 30/48] qdev: Relax parsing of bus option Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 31/48] error: New QERR_BUS_NOT_FOUND Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 32/48] error: New QERR_DEVICE_MULTIPLE_BUSSES Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 33/48] error: New QERR_DEVICE_NO_BUS Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 34/48] qdev: Convert qbus_find() to QError Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 35/48] monitor: New in_qmp_mon() Markus Armbruster
2010-02-26 19:46   ` Luiz Capitulino
2010-03-01  9:19     ` Markus Armbruster
2010-03-01 16:18       ` Luiz Capitulino
2010-03-02  8:53         ` Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 36/48] error: New error_printf_unless_qmp() Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 37/48] error: New QERR_BAD_BUS_FOR_DEVICE Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 38/48] error: New QERR_BUS_NO_HOTPLUG Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 39/48] error: New QERR_DEVICE_INIT_FAILED Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 40/48] error: New QERR_NO_BUS_FOR_DEVICE Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 41/48] Revert "qdev: Use QError for 'device not found' error" Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 42/48] error: Convert do_device_add() to QError Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 43/48] qemu-option: Functions to convert to/from QDict Markus Armbruster
2010-02-26 19:46   ` Luiz Capitulino
2010-03-01  9:24     ` Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 44/48] qemu-option: Move the implied first name into QemuOptsList Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 45/48] qemu-option: Rename find_list() to qemu_find_opts() & external linkage Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 46/48] monitor: New argument type 'O' Markus Armbruster
2010-02-24 17:55 ` [Qemu-devel] [PATCH RFC 47/48] monitor: Use argument type 'O' for device_add Markus Armbruster
2010-02-24 17:56 ` [Qemu-devel] [PATCH RFC 48/48] monitor: convert do_device_add() to QObject Markus Armbruster
2010-02-26 19:47   ` Luiz Capitulino
2010-03-01  9:25     ` Markus Armbruster
2010-02-25 11:59 ` [Qemu-devel] Re: [PATCH RFC 00/48] Convert device_add to QObject / QError Michael S. Tsirkin
2010-02-26 19:43 ` [Qemu-devel] " Luiz Capitulino
2010-03-01  7:59   ` Markus Armbruster
2010-03-01 15:52     ` Luiz Capitulino

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).