Re: [PATCH 2/4] virtio: forward errors into qdev_report_runtime_error()

[Vladimir Sementsov-Ogievskiy]

On 5/19/22 17:19, Konstantin Khlebnikov wrote:

Repalce virtio_error() with macro which forms structured Error and
reports it as device runtime-error in addition to present actions.

Signed-off-by: Konstantin Khlebnikov <khlebni...@yandex-team.ru>
---
  hw/virtio/virtio.c         |    9 +++------
  include/hw/virtio/virtio.h |   10 +++++++++-
  2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..638d779bf2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -3642,13 +3642,10 @@ void virtio_device_set_child_bus_name(VirtIODevice 
*vdev, char *bus_name)
      vdev->bus_name = g_strdup(bus_name);
  }

-void G_GNUC_PRINTF(2, 3) virtio_error(VirtIODevice *vdev, const char *fmt, ...)

+void virtio_fatal_error(VirtIODevice *vdev, Error *err)
  {
-    va_list ap;
-
-    va_start(ap, fmt);
-    error_vreport(fmt, ap);
-    va_end(ap);
+    qdev_report_runtime_error(&vdev->parent_obj, err);
+    error_report_err(err);

if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {

          vdev->status = vdev->status | VIRTIO_CONFIG_S_NEEDS_RESET;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..a165e35b0b 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -16,6 +16,7 @@

#include "exec/memory.h"

  #include "hw/qdev-core.h"
+#include "qapi/error.h"
  #include "net/net.h"
  #include "migration/vmstate.h"
  #include "qemu/event_notifier.h"
@@ -172,7 +173,14 @@ void virtio_init(VirtIODevice *vdev, uint16_t device_id, 
size_t config_size);

void virtio_cleanup(VirtIODevice *vdev); -void virtio_error(VirtIODevice *vdev, const char *fmt, ...) G_GNUC_PRINTF(2, 3);

+#define virtio_error(vdev, fmt, ...) {              \
+    Error *_err = NULL;                             \
+    error_setg(&_err, (fmt), ## __VA_ARGS__);       \
+    virtio_fatal_error(vdev, _err);                 \
+} while (0)
+
+/* Reports and frees error, breaks device */
+void virtio_fatal_error(VirtIODevice *vdev, Error *err);

/* Set the child bus name. */

  void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name);

Hmm. So we create temporary Error object just to pass it to 
qdev_report_runtime_error..

I think we can avoid introducing this intermediate Error object together with 
new macro:
just convert argument list to string with help of g_strdup_vprintf() in 
original virtio_error(), error_report this string and pass to 
qdev_report_runtime_error() (which should be simplified to get just a string).

--
Best regards,
Vladimir