Re: [Qemu-devel] [RFC 00/14]: add printf-like human msg to error_set()

[Anthony Liguori]

Luiz Capitulino <address@hidden> writes:

> Basically, this series changes a call like:
>
>  error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>
> to:
>
>  error_set(errp, QERR_DEVICE_NOT_FOUND,
>            "Device 'device=%s' not found", device);
>
> In the first call, QERR_DEVICE_NOT_FOUND is a string containing a json dict:
>
>     "{ 'class': 'DeviceNotFound', 'data': { 'device': %s } }"

This is the wrong direction.  Looking through the patch, this makes the
code much more redundant overall.  You have dozens of calls that are
duplicating the same error message.  This is not progress.

We should just stick with a simple QERR_GENERIC and call it a day.
Let's not needlessly complicate existing code.

Regards,

Anthony Liguori

>
> error_set() then uses that string and the 'device' argument to build the
> error object, which is a QDict. The human error message is fixed, and exists
> in the qerror_table[] array. That array is indexed, and the human error
> message is added to the error object.
>
> In the new way, QERR_DEVICE_NOT_FOUND is an enumeration value and the
> human error message is passed as an argument. qerror_table[] is gone. The
> error object is built by using QERR_DEVICE_NOT_FOUND as an index for a table
> containing all those json dict strings (this can, and probably will, be
> eliminated, as the QMP code can generate its error object from the Error
> object).
>
> An important detail is that, the error object data member is constructed
> by parsing the human message string and looking for name=value pairs. If
> a required data member (like 'device' above) is not found in the human 
> message,
> a default value is used ('unknown' for strings and 0 for integers).
>
> This series unlocks several possible simplification, like moving from:
>
>     struct Error
>     {
>         QDict *obj;
>         const char *fmt;
>         char *msg;
>     };
>
> to:
>
>     struct Error
>     {
>         ErrClass err_class;
>         char *msg;
>     };
>
> But I haven't done it all yet.
>
> Please, check individual patches for more details.
>
>  balloon.c                   |  10 +-
>  block.c                     |   4 +-
>  block/cow.c                 |   3 +-
>  block/qcow.c                |   7 +-
>  block/qcow2.c               |   3 +-
>  block/qed.c                 |   3 +-
>  block/stream.c              |   2 +-
>  block/vdi.c                 |   4 +-
>  block/vmdk.c                |   4 +-
>  block/vpc.c                 |   4 +-
>  block/vvfat.c               |   4 +-
>  blockdev.c                  |  79 +++++++-------
>  configure                   |  10 --
>  cpus.c                      |  13 ++-
>  dump-stub.c                 |   2 +-
>  dump.c                      |  18 ++--
>  error.c                     |  45 ++------
>  error.h                     |   5 +-
>  hmp.c                       |   2 +-
>  hw/9pfs/virtio-9p.c         |   3 +-
>  hw/ivshmem.c                |   2 +-
>  hw/pci-stub.c               |   2 +-
>  hw/pci.c                    |   4 +-
>  hw/qdev-addr.c              |   6 +-
>  hw/qdev-monitor.c           |  26 +++--
>  hw/qdev-properties.c        |  49 ++++-----
>  hw/qdev.c                   |   6 +-
>  hw/usb/bus.c                |   3 +-
>  hw/usb/hcd-ehci.c           |   6 +-
>  hw/usb/redirect.c           |   5 +-
>  json-parser.c               |   2 +-
>  migration.c                 |   6 +-
>  monitor.c                   | 127 +++++------------------
>  net.c                       |  20 ++--
>  qapi/qapi-visit-core.c      |  22 ++--
>  qapi/qmp-dispatch.c         |  14 ++-
>  qapi/qmp-input-visitor.c    |  22 ++--
>  qapi/string-input-visitor.c |  12 +--
>  qemu-config.c               |   2 +-
>  qemu-ga.c                   |   4 +-
>  qemu-option.c               |  20 ++--
>  qemu-sockets.c              |  22 ++--
>  qerror.c                    | 243 
> ++++++++++++++------------------------------
>  qerror.h                    |  22 +---
>  qga/commands-posix.c        |  78 +++++++-------
>  qga/commands-win32.c        |  53 ++++------
>  qmp.c                       |  47 +++++----
>  qom/object.c                |  20 ++--
>  savevm.c                    |   6 +-
>  scripts/qapi-errors.py      |  80 ++++++++-------
>  target-i386/cpu.c           |  15 +--
>  ui/vnc.c                    |   4 +-
>  52 files changed, 450 insertions(+), 725 deletions(-)