[Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Wenchao Xia]

This series will remove the usage of symbols of mon-protocol-event in
qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
layer.

Background:
  I am tring to decouple block layer code with other unnnessary components,
and in ./stub there many symbols that qemu-img linked as fake implemtion.
As a first step, I am decouple monitor with block layer code, this is the
first part of it.
  There are still other stub symbols for monitor, which will be solved later.
It seems error handlering is also link with those symbols, and will adjust
that.

Wenchao Xia (8):
  1 block: use type MonitorEvent directly
  2 block: do not include monitor.h in block.c
  3 qapi: move MonitorEvent define
  4 qapi: rename MonitorEvent to QEvent
  5 block: add a callback layer for common functions
  6 block: replace monitor_protocol_event() with callback
  7 block: do not include monitor.h
  7 stubs: remove mon-protocol-event.o in stub obj

 block.c                    |   22 ++++++++++++++++++----
 block/qcow2-refcount.c     |    4 +++-
 blockjob.c                 |   10 ++++++++--
 include/block/block.h      |   12 ++++++++++++
 include/block/block_int.h  |    3 +--
 include/monitor/monitor.h  |   40 ++--------------------------------------
 include/qapi/qmp/qevent.h  |   41 +++++++++++++++++++++++++++++++++++++++++
 include/qapi/qmp/types.h   |    1 +
 monitor.c                  |   12 ++++++------
 stubs/Makefile.objs        |    1 -
 stubs/mon-protocol-event.c |    2 +-
 tests/Makefile             |    3 ++-
 ui/vnc.c                   |    2 +-
 vl.c                       |    4 ++++
 14 files changed, 100 insertions(+), 57 deletions(-)
 create mode 100644 include/qapi/qmp/qevent.h

[Qemu-devel] [RFC PATCH 1/8] block: use type MonitorEvent directly

[Wenchao Xia]

block_int.h included monitor.h, so it knows the typedef.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c                   |    2 +-
 include/block/block_int.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index a325efc..2e2774d 100644
--- a/block.c
+++ b/block.c
@@ -1711,7 +1711,7 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
 }
 
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               enum MonitorEvent ev,
+                               MonitorEvent ev,
                                BlockErrorAction action, bool is_read)
 {
     QObject *data;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 7c35198..5de45a1 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -315,7 +315,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
 int is_windows_drive(const char *filename);
 #endif
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               enum MonitorEvent ev,
+                               MonitorEvent ev,
                                BlockErrorAction action, bool is_read);
 
 /**
-- 
1.7.1

[Qemu-devel] [RFC PATCH 2/8] block: do not include monitor.h in block.c

[Wenchao Xia]

block_int.h already included it.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 2e2774d..a532eaa 100644
--- a/block.c
+++ b/block.c
@@ -24,7 +24,6 @@
 #include "config-host.h"
 #include "qemu-common.h"
 #include "trace.h"
-#include "monitor/monitor.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/module.h"
-- 
1.7.1

[Qemu-devel] [RFC PATCH 3/8] qapi: move MonitorEvent define

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 include/monitor/monitor.h |   38 +-------------------------------------
 include/qapi/qmp/qevent.h |   41 +++++++++++++++++++++++++++++++++++++++++
 include/qapi/qmp/types.h  |    1 +
 3 files changed, 43 insertions(+), 37 deletions(-)
 create mode 100644 include/qapi/qmp/qevent.h

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 10fa0e3..686c0eb 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -4,6 +4,7 @@
 #include "qemu-common.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qevent.h"
 #include "block/block.h"
 #include "monitor/readline.h"
 
@@ -19,43 +20,6 @@ extern Monitor *default_mon;
 /* flags for monitor commands */
 #define MONITOR_CMD_ASYNC       0x0001
 
-/* QMP events */
-typedef enum MonitorEvent {
-    QEVENT_SHUTDOWN,
-    QEVENT_RESET,
-    QEVENT_POWERDOWN,
-    QEVENT_STOP,
-    QEVENT_RESUME,
-    QEVENT_VNC_CONNECTED,
-    QEVENT_VNC_INITIALIZED,
-    QEVENT_VNC_DISCONNECTED,
-    QEVENT_BLOCK_IO_ERROR,
-    QEVENT_RTC_CHANGE,
-    QEVENT_WATCHDOG,
-    QEVENT_SPICE_CONNECTED,
-    QEVENT_SPICE_INITIALIZED,
-    QEVENT_SPICE_DISCONNECTED,
-    QEVENT_BLOCK_JOB_COMPLETED,
-    QEVENT_BLOCK_JOB_CANCELLED,
-    QEVENT_BLOCK_JOB_ERROR,
-    QEVENT_BLOCK_JOB_READY,
-    QEVENT_DEVICE_DELETED,
-    QEVENT_DEVICE_TRAY_MOVED,
-    QEVENT_NIC_RX_FILTER_CHANGED,
-    QEVENT_SUSPEND,
-    QEVENT_SUSPEND_DISK,
-    QEVENT_WAKEUP,
-    QEVENT_BALLOON_CHANGE,
-    QEVENT_SPICE_MIGRATE_COMPLETED,
-    QEVENT_GUEST_PANICKED,
-    QEVENT_BLOCK_IMAGE_CORRUPTED,
-
-    /* Add to 'monitor_event_names' array in monitor.c when
-     * defining new events here */
-
-    QEVENT_MAX,
-} MonitorEvent;
-
 int monitor_cur_is_qmp(void);
 
 void monitor_protocol_event(MonitorEvent event, QObject *data);
diff --git a/include/qapi/qmp/qevent.h b/include/qapi/qmp/qevent.h
new file mode 100644
index 0000000..aef71d9
--- /dev/null
+++ b/include/qapi/qmp/qevent.h
@@ -0,0 +1,41 @@
+#ifndef QEVENT_H
+#define QEVENT_H
+
+/* QMP events */
+typedef enum MonitorEvent {
+    QEVENT_SHUTDOWN,
+    QEVENT_RESET,
+    QEVENT_POWERDOWN,
+    QEVENT_STOP,
+    QEVENT_RESUME,
+    QEVENT_VNC_CONNECTED,
+    QEVENT_VNC_INITIALIZED,
+    QEVENT_VNC_DISCONNECTED,
+    QEVENT_BLOCK_IO_ERROR,
+    QEVENT_RTC_CHANGE,
+    QEVENT_WATCHDOG,
+    QEVENT_SPICE_CONNECTED,
+    QEVENT_SPICE_INITIALIZED,
+    QEVENT_SPICE_DISCONNECTED,
+    QEVENT_BLOCK_JOB_COMPLETED,
+    QEVENT_BLOCK_JOB_CANCELLED,
+    QEVENT_BLOCK_JOB_ERROR,
+    QEVENT_BLOCK_JOB_READY,
+    QEVENT_DEVICE_DELETED,
+    QEVENT_DEVICE_TRAY_MOVED,
+    QEVENT_NIC_RX_FILTER_CHANGED,
+    QEVENT_SUSPEND,
+    QEVENT_SUSPEND_DISK,
+    QEVENT_WAKEUP,
+    QEVENT_BALLOON_CHANGE,
+    QEVENT_SPICE_MIGRATE_COMPLETED,
+    QEVENT_GUEST_PANICKED,
+    QEVENT_BLOCK_IMAGE_CORRUPTED,
+
+    /* Add to 'monitor_event_names' array in monitor.c when
+     * defining new events here */
+
+    QEVENT_MAX,
+} MonitorEvent;
+
+#endif
diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
index 7782ec5..ba317bf 100644
--- a/include/qapi/qmp/types.h
+++ b/include/qapi/qmp/types.h
@@ -21,5 +21,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qevent.h"
 
 #endif /* QEMU_OBJECTS_H */
-- 
1.7.1

[Qemu-devel] [RFC PATCH 4/8] qapi: rename MonitorEvent to QEvent

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c                    |    2 +-
 include/block/block_int.h  |    2 +-
 include/monitor/monitor.h  |    2 +-
 include/qapi/qmp/qevent.h  |    4 ++--
 monitor.c                  |   12 ++++++------
 stubs/mon-protocol-event.c |    2 +-
 ui/vnc.c                   |    2 +-
 7 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index a532eaa..07385bf 100644
--- a/block.c
+++ b/block.c
@@ -1710,7 +1710,7 @@ void bdrv_set_dev_ops(BlockDriverState *bs, const BlockDevOps *ops,
 }
 
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               MonitorEvent ev,
+                               QEvent ev,
                                BlockErrorAction action, bool is_read)
 {
     QObject *data;
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 5de45a1..db2cb49 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -315,7 +315,7 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs);
 int is_windows_drive(const char *filename);
 #endif
 void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
-                               MonitorEvent ev,
+                               QEvent ev,
                                BlockErrorAction action, bool is_read);
 
 /**
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 686c0eb..97fcee3 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -22,7 +22,7 @@ extern Monitor *default_mon;
 
 int monitor_cur_is_qmp(void);
 
-void monitor_protocol_event(MonitorEvent event, QObject *data);
+void monitor_protocol_event(QEvent event, QObject *data);
 void monitor_init(CharDriverState *chr, int flags);
 
 int monitor_suspend(Monitor *mon);
diff --git a/include/qapi/qmp/qevent.h b/include/qapi/qmp/qevent.h
index aef71d9..dda9511 100644
--- a/include/qapi/qmp/qevent.h
+++ b/include/qapi/qmp/qevent.h
@@ -2,7 +2,7 @@
 #define QEVENT_H
 
 /* QMP events */
-typedef enum MonitorEvent {
+typedef enum QEvent {
     QEVENT_SHUTDOWN,
     QEVENT_RESET,
     QEVENT_POWERDOWN,
@@ -36,6 +36,6 @@ typedef enum MonitorEvent {
      * defining new events here */
 
     QEVENT_MAX,
-} MonitorEvent;
+} QEvent;
 
 #endif
diff --git a/monitor.c b/monitor.c
index 74f3f1b..9377834 100644
--- a/monitor.c
+++ b/monitor.c
@@ -175,7 +175,7 @@ typedef struct MonitorControl {
  * instance.
  */
 typedef struct MonitorEventState {
-    MonitorEvent event; /* Event being tracked */
+    QEvent event;       /* Event being tracked */
     int64_t rate;       /* Period over which to throttle. 0 to disable */
     int64_t last;       /* Time at which event was last emitted */
     QEMUTimer *timer;   /* Timer for handling delayed events */
@@ -517,7 +517,7 @@ QemuMutex monitor_event_state_lock;
  * Emits the event to every monitor instance
  */
 static void
-monitor_protocol_event_emit(MonitorEvent event,
+monitor_protocol_event_emit(QEvent event,
                             QObject *data)
 {
     Monitor *mon;
@@ -536,7 +536,7 @@ monitor_protocol_event_emit(MonitorEvent event,
  * applying any rate limiting if required.
  */
 static void
-monitor_protocol_event_queue(MonitorEvent event,
+monitor_protocol_event_queue(QEvent event,
                              QObject *data)
 {
     MonitorEventState *evstate;
@@ -614,7 +614,7 @@ static void monitor_protocol_event_handler(void *opaque)
  * milliseconds
  */
 static void
-monitor_protocol_event_throttle(MonitorEvent event,
+monitor_protocol_event_throttle(QEvent event,
                                 int64_t rate)
 {
     MonitorEventState *evstate;
@@ -650,7 +650,7 @@ static void monitor_protocol_event_init(void)
  *
  * Event-specific data can be emitted through the (optional) 'data' parameter.
  */
-void monitor_protocol_event(MonitorEvent event, QObject *data)
+void monitor_protocol_event(QEvent event, QObject *data)
 {
     QDict *qmp;
     const char *event_name;
@@ -1067,7 +1067,7 @@ CommandInfoList *qmp_query_commands(Error **errp)
 EventInfoList *qmp_query_events(Error **errp)
 {
     EventInfoList *info, *ev_list = NULL;
-    MonitorEvent e;
+    QEvent e;
 
     for (e = 0 ; e < QEVENT_MAX ; e++) {
         const char *event_name = monitor_event_names[e];
diff --git a/stubs/mon-protocol-event.c b/stubs/mon-protocol-event.c
index 0946e94..e769729 100644
--- a/stubs/mon-protocol-event.c
+++ b/stubs/mon-protocol-event.c
@@ -1,6 +1,6 @@
 #include "qemu-common.h"
 #include "monitor/monitor.h"
 
-void monitor_protocol_event(MonitorEvent event, QObject *data)
+void monitor_protocol_event(QEvent event, QObject *data)
 {
 }
diff --git a/ui/vnc.c b/ui/vnc.c
index 5601cc3..47fda54 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -275,7 +275,7 @@ static void vnc_client_cache_addr(VncState *client)
     client->info = QOBJECT(qdict);
 }
 
-static void vnc_qmp_event(VncState *vs, MonitorEvent event)
+static void vnc_qmp_event(VncState *vs, QEvent event)
 {
     QDict *server;
     QObject *data;
-- 
1.7.1

[Qemu-devel] [RFC PATCH 5/8] block: add a callback layer for common functions

[Wenchao Xia]

This structure can hold some call back functions, such as
event emit, error printf. By using call back, block layer
can be decoupled with other components.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c               |    7 +++++++
 include/block/block.h |   11 +++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 07385bf..576b86e 100644
--- a/block.c
+++ b/block.c
@@ -55,6 +55,13 @@ typedef enum {
     BDRV_REQ_ZERO_WRITE   = 0x2,
 } BdrvRequestFlags;
 
+BDRVCommonHooks bdrv_common_hooks;
+
+void bdrv_set_common_hooks(BDRVCommonHooks *hooks)
+{
+    bdrv_common_hooks = *hooks;
+}
+
 static void bdrv_dev_change_media_cb(BlockDriverState *bs, bool load);
 static BlockDriverAIOCB *bdrv_aio_readv_em(BlockDriverState *bs,
         int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
diff --git a/include/block/block.h b/include/block/block.h
index 728ec1a..7913f48 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -119,6 +119,17 @@ typedef struct BDRVReopenState {
     void *opaque;
 } BDRVReopenState;
 
+/*
+ * Now all block layer use same hooks, If needed it can be changed as per
+ * bds.
+ */
+typedef struct BDRVCommonHooks {
+    void (*hooks)(void *);
+} BDRVCommonHooks;
+
+extern BDRVCommonHooks bdrv_common_hooks;
+
+void bdrv_set_common_hooks(BDRVCommonHooks *hooks);
 
 void bdrv_iostatus_enable(BlockDriverState *bs);
 void bdrv_iostatus_reset(BlockDriverState *bs);
-- 
1.7.1

[Qemu-devel] [RFC PATCH 6/8] block: replace monitor_protocol_event() with callback

[Wenchao Xia]

For code inside block layer, it is replaced with a call back
function. vl.c will tell block layer how to emit the event.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block.c                |   12 ++++++++++--
 block/qcow2-refcount.c |    4 +++-
 blockjob.c             |   10 ++++++++--
 include/block/block.h  |    3 ++-
 vl.c                   |    4 ++++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 576b86e..5695064 100644
--- a/block.c
+++ b/block.c
@@ -1723,6 +1723,10 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
     QObject *data;
     const char *action_str;
 
+    if (!bdrv_common_hooks.event_emit) {
+        return;
+    }
+
     switch (action) {
     case BDRV_ACTION_REPORT:
         action_str = "report";
@@ -1741,7 +1745,7 @@ void bdrv_emit_qmp_error_event(const BlockDriverState *bdrv,
                               bdrv->device_name,
                               action_str,
                               is_read ? "read" : "write");
-    monitor_protocol_event(ev, data);
+    bdrv_common_hooks.event_emit(ev, data);
 
     qobject_decref(data);
 }
@@ -1750,9 +1754,13 @@ static void bdrv_emit_qmp_eject_event(BlockDriverState *bs, bool ejected)
 {
     QObject *data;
 
+    if (!bdrv_common_hooks.event_emit) {
+        return;
+    }
+
     data = qobject_from_jsonf("{ 'device': %s, 'tray-open': %i }",
                               bdrv_get_device_name(bs), ejected);
-    monitor_protocol_event(QEVENT_DEVICE_TRAY_MOVED, data);
+    bdrv_common_hooks.event_emit(QEVENT_DEVICE_TRAY_MOVED, data);
 
     qobject_decref(data);
 }
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ba129de..829083f 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1790,7 +1790,9 @@ int qcow2_pre_write_overlap_check(BlockDriverState *bs, int chk, int64_t offset,
         data = qobject_from_jsonf("{ 'device': %s, 'msg': %s, 'offset': %"
                 PRId64 ", 'size': %" PRId64 " }", bs->device_name, message,
                 offset, size);
-        monitor_protocol_event(QEVENT_BLOCK_IMAGE_CORRUPTED, data);
+        if (bdrv_common_hooks.event_emit) {
+            bdrv_common_hooks.event_emit(QEVENT_BLOCK_IMAGE_CORRUPTED, data);
+        }
         g_free(message);
         qobject_decref(data);
 
diff --git a/blockjob.c b/blockjob.c
index e7d49b7..7ff356c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -245,8 +245,14 @@ QObject *qobject_from_block_job(BlockJob *job)
 
 void block_job_ready(BlockJob *job)
 {
-    QObject *data = qobject_from_block_job(job);
-    monitor_protocol_event(QEVENT_BLOCK_JOB_READY, data);
+    QObject *data;
+
+    if (!bdrv_common_hooks.event_emit) {
+        return;
+    }
+
+    data = qobject_from_block_job(job);
+    bdrv_common_hooks.event_emit(QEVENT_BLOCK_JOB_READY, data);
     qobject_decref(data);
 }
 
diff --git a/include/block/block.h b/include/block/block.h
index 7913f48..fadbf5b 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -7,6 +7,7 @@
 #include "block/coroutine.h"
 #include "qapi/qmp/qobject.h"
 #include "qapi-types.h"
+#include "qapi/qmp/qevent.h"
 
 /* block.c */
 typedef struct BlockDriver BlockDriver;
@@ -124,7 +125,7 @@ typedef struct BDRVReopenState {
  * bds.
  */
 typedef struct BDRVCommonHooks {
-    void (*hooks)(void *);
+    void (*event_emit)(QEvent event, QObject *data);
 } BDRVCommonHooks;
 
 extern BDRVCommonHooks bdrv_common_hooks;
diff --git a/vl.c b/vl.c
index 4e709d5..a277598 100644
--- a/vl.c
+++ b/vl.c
@@ -2852,6 +2852,7 @@ int main(int argc, char **argv, char **envp)
     };
     const char *trace_events = NULL;
     const char *trace_file = NULL;
+    BDRVCommonHooks bdrv_hooks;
 
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
@@ -2915,6 +2916,9 @@ int main(int argc, char **argv, char **envp)
     nb_nics = 0;
 
     bdrv_init_with_whitelist();
+    memset(&bdrv_hooks, 0, sizeof(bdrv_hooks));
+    bdrv_hooks.event_emit = monitor_protocol_event;
+    bdrv_set_common_hooks(&bdrv_hooks);
 
     autostart= 1;
 
-- 
1.7.1

[Qemu-devel] [RFC PATCH 7/8] block: do not include monitor.h

[Wenchao Xia]

Block layer do not need it any more.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 include/block/block_int.h |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index db2cb49..ec92f16 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -31,7 +31,6 @@
 #include "qemu/timer.h"
 #include "qapi-types.h"
 #include "qapi/qmp/qerror.h"
-#include "monitor/monitor.h"
 #include "qemu/hbitmap.h"
 #include "block/snapshot.h"
 #include "qemu/main-loop.h"
-- 
1.7.1

[Qemu-devel] [RFC PATCH 8/8] stubs: remove mon-protocol-event.o in stub obj

[Wenchao Xia]

Now block layer do not use this symbol now, so qemg-img, qemu-io
and qemu-nbd do not link with this file any more. The test program
tests/test-qdev-global-props still need it, so add this obj in
rule of test/Makefile, and keeps the c file now.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 stubs/Makefile.objs |    1 -
 tests/Makefile      |    3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
index f306cba..6fe1e3f 100644
--- a/stubs/Makefile.objs
+++ b/stubs/Makefile.objs
@@ -15,7 +15,6 @@ stub-obj-y += migr-blocker.o
 stub-obj-y += mon-is-qmp.o
 stub-obj-y += mon-printf.o
 stub-obj-y += mon-print-filename.o
-stub-obj-y += mon-protocol-event.o
 stub-obj-y += mon-set-error.o
 stub-obj-y += pci-drive-hot-add.o
 stub-obj-y += reset.o
diff --git a/tests/Makefile b/tests/Makefile
index c13fefc..7d3ee3b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -134,7 +134,8 @@ tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
 	hw/core/irq.o \
 	qom/object.o qom/container.o qom/qom-qobject.o \
 	$(test-qapi-obj-y) \
-	libqemuutil.a libqemustub.a
+	libqemuutil.a libqemustub.a \
+        stubs/mon-protocol-event.o
 
 tests/test-qapi-types.c tests/test-qapi-types.h :\
 $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
-- 
1.7.1

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Paolo Bonzini]

Il 12/09/2013 11:15, Wenchao Xia ha scritto:
> This series will remove the usage of symbols of mon-protocol-event in
> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
> layer.
> 
> Background:
>   I am tring to decouple block layer code with other unnnessary components,
> and in ./stub there many symbols that qemu-img linked as fake implemtion.
> As a first step, I am decouple monitor with block layer code, this is the
> first part of it.
>   There are still other stub symbols for monitor, which will be solved later.
> It seems error handlering is also link with those symbols, and will adjust
> that.
> 
> Wenchao Xia (8):
>   1 block: use type MonitorEvent directly
>   2 block: do not include monitor.h in block.c
>   3 qapi: move MonitorEvent define
>   4 qapi: rename MonitorEvent to QEvent
>   5 block: add a callback layer for common functions
>   6 block: replace monitor_protocol_event() with callback
>   7 block: do not include monitor.h
>   7 stubs: remove mon-protocol-event.o in stub obj
> 
>  block.c                    |   22 ++++++++++++++++++----
>  block/qcow2-refcount.c     |    4 +++-
>  blockjob.c                 |   10 ++++++++--
>  include/block/block.h      |   12 ++++++++++++
>  include/block/block_int.h  |    3 +--
>  include/monitor/monitor.h  |   40 ++--------------------------------------
>  include/qapi/qmp/qevent.h  |   41 +++++++++++++++++++++++++++++++++++++++++
>  include/qapi/qmp/types.h   |    1 +
>  monitor.c                  |   12 ++++++------
>  stubs/Makefile.objs        |    1 -
>  stubs/mon-protocol-event.c |    2 +-
>  tests/Makefile             |    3 ++-
>  ui/vnc.c                   |    2 +-
>  vl.c                       |    4 ++++
>  14 files changed, 100 insertions(+), 57 deletions(-)
>  create mode 100644 include/qapi/qmp/qevent.h
> 

Patches 1-4 look good.  I'm not sure of the advantage of the last four,
however.  The ugly part of monitor_protocol_event is not really the
stub, but the dependency on QObject.

So, in my opinion a more interesting approach would be to describe
events using QAPI types.  Generating the events would require a small
amount of code to build QObjects manually, because the event syntax
doesn't match exactly a QAPI union, but that is only a technical detail.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Kevin Wolf]

Am 12.09.2013 um 11:31 hat Paolo Bonzini geschrieben:
> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
> > This series will remove the usage of symbols of mon-protocol-event in
> > qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
> > layer.
> > 
> > Background:
> >   I am tring to decouple block layer code with other unnnessary components,
> > and in ./stub there many symbols that qemu-img linked as fake implemtion.
> > As a first step, I am decouple monitor with block layer code, this is the
> > first part of it.
> >   There are still other stub symbols for monitor, which will be solved later.
> > It seems error handlering is also link with those symbols, and will adjust
> > that.

> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
> however.  The ugly part of monitor_protocol_event is not really the
> stub, but the dependency on QObject.
> 
> So, in my opinion a more interesting approach would be to describe
> events using QAPI types.  Generating the events would require a small
> amount of code to build QObjects manually, because the event syntax
> doesn't match exactly a QAPI union, but that is only a technical detail.

You mean the QAPI schema cannot describe events? Why do you think so,
and wouldn't this be a reason to extend the schema language?

The QMP spec describes events like this:

{ "event": json-string, "data": json-object,
  "timestamp": { "seconds": json-number, "microseconds": json-number } }

Looks like it could be described as:

{ 'type': 'EventTypeA',
  'data': {
      'data': {
          # event-specific data
      } } }
{ 'type': 'EventBase',
  'data': { 'event': 'str'. 'timestamp': {
      'seconds': 'int', 'microseconds': 'int' } }
{ 'union': 'Event',
  'base:' EventBase',
  'discriminator': 'event',
  'data': {
      'EVENT_NAME_A': 'EventTypeA'
  } }

Kevin

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Markus Armbruster]

[Note cc: Luiz]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>> This series will remove the usage of symbols of mon-protocol-event in
>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
>> layer.
>> 
>> Background:
>>   I am tring to decouple block layer code with other unnnessary components,
>> and in ./stub there many symbols that qemu-img linked as fake implemtion.
>> As a first step, I am decouple monitor with block layer code, this is the
>> first part of it.
>>   There are still other stub symbols for monitor, which will be solved later.
>> It seems error handlering is also link with those symbols, and will adjust
>> that.
>> 
>> Wenchao Xia (8):
>>   1 block: use type MonitorEvent directly
>>   2 block: do not include monitor.h in block.c
>>   3 qapi: move MonitorEvent define
>>   4 qapi: rename MonitorEvent to QEvent
>>   5 block: add a callback layer for common functions
>>   6 block: replace monitor_protocol_event() with callback
>>   7 block: do not include monitor.h
>>   7 stubs: remove mon-protocol-event.o in stub obj
>> 
>>  block.c                    |   22 ++++++++++++++++++----
>>  block/qcow2-refcount.c     |    4 +++-
>>  blockjob.c                 |   10 ++++++++--
>>  include/block/block.h      |   12 ++++++++++++
>>  include/block/block_int.h  |    3 +--
>>  include/monitor/monitor.h  |   40 ++--------------------------------------
>>  include/qapi/qmp/qevent.h  |   41 +++++++++++++++++++++++++++++++++++++++++
>>  include/qapi/qmp/types.h   |    1 +
>>  monitor.c                  |   12 ++++++------
>>  stubs/Makefile.objs        |    1 -
>>  stubs/mon-protocol-event.c |    2 +-
>>  tests/Makefile             |    3 ++-
>>  ui/vnc.c                   |    2 +-
>>  vl.c                       |    4 ++++
>>  14 files changed, 100 insertions(+), 57 deletions(-)
>>  create mode 100644 include/qapi/qmp/qevent.h
>> 
>
> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
> however.  The ugly part of monitor_protocol_event is not really the
> stub, but the dependency on QObject.
>
> So, in my opinion a more interesting approach would be to describe
> events using QAPI types.  Generating the events would require a small
> amount of code to build QObjects manually, because the event syntax
> doesn't match exactly a QAPI union, but that is only a technical detail.
>
> Paolo

See QMP TODO list, item "Add events support to the QAPI"
http://wiki.qemu.org/QMP#TODO
https://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg01816.html

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Paolo Bonzini]

Il 12/09/2013 14:08, Kevin Wolf ha scritto:
> Am 12.09.2013 um 11:31 hat Paolo Bonzini geschrieben:
>> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>>> This series will remove the usage of symbols of mon-protocol-event in
>>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
>>> layer.
>>>
>>> Background:
>>>   I am tring to decouple block layer code with other unnnessary components,
>>> and in ./stub there many symbols that qemu-img linked as fake implemtion.
>>> As a first step, I am decouple monitor with block layer code, this is the
>>> first part of it.
>>>   There are still other stub symbols for monitor, which will be solved later.
>>> It seems error handlering is also link with those symbols, and will adjust
>>> that.
> 
>> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
>> however.  The ugly part of monitor_protocol_event is not really the
>> stub, but the dependency on QObject.
>>
>> So, in my opinion a more interesting approach would be to describe
>> events using QAPI types.  Generating the events would require a small
>> amount of code to build QObjects manually, because the event syntax
>> doesn't match exactly a QAPI union, but that is only a technical detail.
> 
> You mean the QAPI schema cannot describe events? Why do you think so,
> and wouldn't this be a reason to extend the schema language?
> 
> The QMP spec describes events like this:
> 
> { "event": json-string, "data": json-object,
>   "timestamp": { "seconds": json-number, "microseconds": json-number } }
> 
> Looks like it could be described as:
> 
> { 'type': 'EventTypeA',
>   'data': {
>       'data': {
>           # event-specific data
>       } } }
> { 'type': 'EventBase',
>   'data': { 'event': 'str'. 'timestamp': {
>       'seconds': 'int', 'microseconds': 'int' } }
> { 'union': 'Event',
>   'base:' EventBase',
>   'discriminator': 'event',
>   'data': {
>       'EVENT_NAME_A': 'EventTypeA'
>   } }

I didn't know about all this cool stuff you added. :)

Paolo

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Wenchao Xia]

于 2013/9/12 17:31, Paolo Bonzini 写道:
> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>> This series will remove the usage of symbols of mon-protocol-event in
>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
>> layer.
>>
>> Background:
>>    I am tring to decouple block layer code with other unnnessary components,
>> and in ./stub there many symbols that qemu-img linked as fake implemtion.
>> As a first step, I am decouple monitor with block layer code, this is the
>> first part of it.
>>    There are still other stub symbols for monitor, which will be solved later.
>> It seems error handlering is also link with those symbols, and will adjust
>> that.
>>
>> Wenchao Xia (8):
>>    1 block: use type MonitorEvent directly
>>    2 block: do not include monitor.h in block.c
>>    3 qapi: move MonitorEvent define
>>    4 qapi: rename MonitorEvent to QEvent
>>    5 block: add a callback layer for common functions
>>    6 block: replace monitor_protocol_event() with callback
>>    7 block: do not include monitor.h
>>    7 stubs: remove mon-protocol-event.o in stub obj
>>
>>   block.c                    |   22 ++++++++++++++++++----
>>   block/qcow2-refcount.c     |    4 +++-
>>   blockjob.c                 |   10 ++++++++--
>>   include/block/block.h      |   12 ++++++++++++
>>   include/block/block_int.h  |    3 +--
>>   include/monitor/monitor.h  |   40 ++--------------------------------------
>>   include/qapi/qmp/qevent.h  |   41 +++++++++++++++++++++++++++++++++++++++++
>>   include/qapi/qmp/types.h   |    1 +
>>   monitor.c                  |   12 ++++++------
>>   stubs/Makefile.objs        |    1 -
>>   stubs/mon-protocol-event.c |    2 +-
>>   tests/Makefile             |    3 ++-
>>   ui/vnc.c                   |    2 +-
>>   vl.c                       |    4 ++++
>>   14 files changed, 100 insertions(+), 57 deletions(-)
>>   create mode 100644 include/qapi/qmp/qevent.h
>>
> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
> however.  The ugly part of monitor_protocol_event is not really the
> stub, but the dependency on QObject.
I think replacing QObject with QAPI types is another issue we could 
improve. The last
foure patches tries decouple monitor code with block layer code from 
build time.
The files in ./stubs is needed since some symbols are needed in some 
program which don't
need it really, and I think, that tips the code are not organized very 
well in .c file level.
After removing ./stubs, the code will be more clear. And as a by- 
prodcut, the components
in qemu will have a clear boundary, and I can select several components 
to form library,
and qemu can naturlly link with it.

> So, in my opinion a more interesting approach would be to describe
> events using QAPI types.  Generating the events would require a small
> amount of code to build QObjects manually, because the event syntax
> doesn't match exactly a QAPI union, but that is only a technical detail.
>
> Paolo
>

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Paolo Bonzini]

Il 16/09/2013 06:59, Wenchao Xia ha scritto:
> 于 2013/9/12 17:31, Paolo Bonzini 写道:
>> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>>> This series will remove the usage of symbols of mon-protocol-event in
>>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
>>> layer.
>>>
>>> Background:
>>>    I am tring to decouple block layer code with other unnnessary
>>> components,
>>> and in ./stub there many symbols that qemu-img linked as fake
>>> implemtion.
>>> As a first step, I am decouple monitor with block layer code, this is
>>> the
>>> first part of it.
>>>    There are still other stub symbols for monitor, which will be
>>> solved later.
>>> It seems error handlering is also link with those symbols, and will
>>> adjust
>>> that.
>>>
>>> Wenchao Xia (8):
>>>    1 block: use type MonitorEvent directly
>>>    2 block: do not include monitor.h in block.c
>>>    3 qapi: move MonitorEvent define
>>>    4 qapi: rename MonitorEvent to QEvent
>>>    5 block: add a callback layer for common functions
>>>    6 block: replace monitor_protocol_event() with callback
>>>    7 block: do not include monitor.h
>>>    7 stubs: remove mon-protocol-event.o in stub obj
>>>
>>>   block.c                    |   22 ++++++++++++++++++----
>>>   block/qcow2-refcount.c     |    4 +++-
>>>   blockjob.c                 |   10 ++++++++--
>>>   include/block/block.h      |   12 ++++++++++++
>>>   include/block/block_int.h  |    3 +--
>>>   include/monitor/monitor.h  |   40
>>> ++--------------------------------------
>>>   include/qapi/qmp/qevent.h  |   41
>>> +++++++++++++++++++++++++++++++++++++++++
>>>   include/qapi/qmp/types.h   |    1 +
>>>   monitor.c                  |   12 ++++++------
>>>   stubs/Makefile.objs        |    1 -
>>>   stubs/mon-protocol-event.c |    2 +-
>>>   tests/Makefile             |    3 ++-
>>>   ui/vnc.c                   |    2 +-
>>>   vl.c                       |    4 ++++
>>>   14 files changed, 100 insertions(+), 57 deletions(-)
>>>   create mode 100644 include/qapi/qmp/qevent.h
>>>
>> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
>> however.  The ugly part of monitor_protocol_event is not really the
>> stub, but the dependency on QObject.
> I think replacing QObject with QAPI types is another issue we could
> improve. The last
> foure patches tries decouple monitor code with block layer code from
> build time.
> The files in ./stubs is needed since some symbols are needed in some
> program which don't
> need it really, and I think, that tips the code are not organized very
> well in .c file level.

That may very well be the case.  However, picking a function, replacing
it with a function pointer, and throwing it into a struct, will not
improve the structure at the .c file level very much.

It does not abstract anything.  In order to provide a useful
abstractions, the questions to answer are:

1) If a library wanted to provide a callback for events, would the
current design (using QObject) be the right thing to do?  (If you change
the design, it might happen that the stub goes away naturally and that
the work in these patches does nothing except obscure the history).

2) Are there other services that the block layer could desire from the
monitor?

3) Could any tool (e.g. qemu-io) desire to implement
monitor_protocol_event?  If so, what other services could it provide
that the QEMU monitor provides?

> After removing ./stubs, the code will be more clear

If done in the right way, yes.  If done in the wrong way, the code will
be less clear.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Wenchao Xia]

于 2013/9/16 18:02, Paolo Bonzini 写道:
> Il 16/09/2013 06:59, Wenchao Xia ha scritto:
>> 于 2013/9/12 17:31, Paolo Bonzini 写道:
>>> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>>>> This series will remove the usage of symbols of mon-protocol-event in
>>>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for block
>>>> layer.
>>>>
>>>> Background:
>>>>     I am tring to decouple block layer code with other unnnessary
>>>> components,
>>>> and in ./stub there many symbols that qemu-img linked as fake
>>>> implemtion.
>>>> As a first step, I am decouple monitor with block layer code, this is
>>>> the
>>>> first part of it.
>>>>     There are still other stub symbols for monitor, which will be
>>>> solved later.
>>>> It seems error handlering is also link with those symbols, and will
>>>> adjust
>>>> that.
>>>>
>>>> Wenchao Xia (8):
>>>>     1 block: use type MonitorEvent directly
>>>>     2 block: do not include monitor.h in block.c
>>>>     3 qapi: move MonitorEvent define
>>>>     4 qapi: rename MonitorEvent to QEvent
>>>>     5 block: add a callback layer for common functions
>>>>     6 block: replace monitor_protocol_event() with callback
>>>>     7 block: do not include monitor.h
>>>>     7 stubs: remove mon-protocol-event.o in stub obj
>>>>
>>>>    block.c                    |   22 ++++++++++++++++++----
>>>>    block/qcow2-refcount.c     |    4 +++-
>>>>    blockjob.c                 |   10 ++++++++--
>>>>    include/block/block.h      |   12 ++++++++++++
>>>>    include/block/block_int.h  |    3 +--
>>>>    include/monitor/monitor.h  |   40
>>>> ++--------------------------------------
>>>>    include/qapi/qmp/qevent.h  |   41
>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>    include/qapi/qmp/types.h   |    1 +
>>>>    monitor.c                  |   12 ++++++------
>>>>    stubs/Makefile.objs        |    1 -
>>>>    stubs/mon-protocol-event.c |    2 +-
>>>>    tests/Makefile             |    3 ++-
>>>>    ui/vnc.c                   |    2 +-
>>>>    vl.c                       |    4 ++++
>>>>    14 files changed, 100 insertions(+), 57 deletions(-)
>>>>    create mode 100644 include/qapi/qmp/qevent.h
>>>>
>>> Patches 1-4 look good.  I'm not sure of the advantage of the last four,
>>> however.  The ugly part of monitor_protocol_event is not really the
>>> stub, but the dependency on QObject.
>> I think replacing QObject with QAPI types is another issue we could
>> improve. The last
>> foure patches tries decouple monitor code with block layer code from
>> build time.
>> The files in ./stubs is needed since some symbols are needed in some
>> program which don't
>> need it really, and I think, that tips the code are not organized very
>> well in .c file level.
> That may very well be the case.  However, picking a function, replacing
> it with a function pointer, and throwing it into a struct, will not
> improve the structure at the .c file level very much.
>
> It does not abstract anything.  In order to provide a useful
> abstractions, the questions to answer are:
>
> 1) If a library wanted to provide a callback for events, would the
> current design (using QObject) be the right thing to do?  (If you change
> the design, it might happen that the stub goes away naturally and that
   I think it would not goes away even QAPI is used. The ./stubs is a symbol
issue at link time, so whenever block layer use a symbol belong to 
monitor component,
the stubs would be needed.
   In my opinion, there could be two steps: remove this symbol in block 
code,
replace it with QAPI in monitor code.

> the work in these patches does nothing except obscure the history).
>
> 2) Are there other services that the block layer could desire from the
> monitor?
>
   I have a draft makefile which link the core block code, it
shows that only event and error printf(include mon_is_qmp) service
are used by block code now. Those symbols basically are listed in ./stubs.

> 3) Could any tool (e.g. qemu-io) desire to implement
> monitor_protocol_event?  If so, what other services could it provide
> that the QEMU monitor provides?
>
   It seems tools do not need event service now, they link with stubs.

>> After removing ./stubs, the code will be more clear
> If done in the right way, yes.  If done in the wrong way, the code will
> be less clear.
>
> Paolo
>

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Paolo Bonzini]

Il 23/09/2013 21:06, Wenchao Xia ha scritto:
>>
> 
> Hi Paolo,
>   This series tries to find a mechanism, which can avoid link with unused
> symbols. Assume QObject was replaced with other design, such as QAPI, it
> will still call monitor funcions to emit the event, so it will still
> require
> resolving that function in linking of qemu-img.
>   Do you have some suggestion for it?

No, I don't.  Still, it seems to me that (for now) this series does not
solve any real problem.

Paolo

Re: [Qemu-devel] [RFC PATCH 1/8] block: use type MonitorEvent directly

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 465 bytes --]

On 09/12/2013 03:15 AM, Wenchao Xia wrote:
> block_int.h included monitor.h, so it knows the typedef.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c                   |    2 +-
>  include/block/block_int.h |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [RFC PATCH 2/8] block: do not include monitor.h in block.c

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

On 09/12/2013 03:15 AM, Wenchao Xia wrote:
> block_int.h already included it.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/block.c b/block.c
> index 2e2774d..a532eaa 100644
> --- a/block.c
> +++ b/block.c
> @@ -24,7 +24,6 @@
>  #include "config-host.h"
>  #include "qemu-common.h"
>  #include "trace.h"
> -#include "monitor/monitor.h"
>  #include "block/block_int.h"
>  #include "block/blockjob.h"
>  #include "qemu/module.h"
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [RFC PATCH 3/8] qapi: move MonitorEvent define

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 401 bytes --]

On 09/12/2013 03:15 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---

> new file mode 100644
> index 0000000..aef71d9
> --- /dev/null
> +++ b/include/qapi/qmp/qevent.h
> @@ -0,0 +1,41 @@
> +#ifndef QEVENT_H

No copyright notice?  Fix that.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [RFC PATCH 4/8] qapi: rename MonitorEvent to QEvent

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 622 bytes --]

On 09/12/2013 03:15 AM, Wenchao Xia wrote:
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  block.c                    |    2 +-
>  include/block/block_int.h  |    2 +-
>  include/monitor/monitor.h  |    2 +-
>  include/qapi/qmp/qevent.h  |    4 ++--
>  monitor.c                  |   12 ++++++------
>  stubs/mon-protocol-event.c |    2 +-
>  ui/vnc.c                   |    2 +-
>  7 files changed, 13 insertions(+), 13 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block

[Wenchao Xia]

于 2013/9/16 22:27, Wenchao Xia 写道:
> 于 2013/9/16 18:02, Paolo Bonzini 写道:
>> Il 16/09/2013 06:59, Wenchao Xia ha scritto:
>>> 于 2013/9/12 17:31, Paolo Bonzini 写道:
>>>> Il 12/09/2013 11:15, Wenchao Xia ha scritto:
>>>>> This series will remove the usage of symbols of mon-protocol-event in
>>>>> qemu-img, qemu-nbd and qemu-io, in short remove the connetion for 
>>>>> block
>>>>> layer.
>>>>>
>>>>> Background:
>>>>>     I am tring to decouple block layer code with other unnnessary
>>>>> components,
>>>>> and in ./stub there many symbols that qemu-img linked as fake
>>>>> implemtion.
>>>>> As a first step, I am decouple monitor with block layer code, this is
>>>>> the
>>>>> first part of it.
>>>>>     There are still other stub symbols for monitor, which will be
>>>>> solved later.
>>>>> It seems error handlering is also link with those symbols, and will
>>>>> adjust
>>>>> that.
>>>>>
>>>>> Wenchao Xia (8):
>>>>>     1 block: use type MonitorEvent directly
>>>>>     2 block: do not include monitor.h in block.c
>>>>>     3 qapi: move MonitorEvent define
>>>>>     4 qapi: rename MonitorEvent to QEvent
>>>>>     5 block: add a callback layer for common functions
>>>>>     6 block: replace monitor_protocol_event() with callback
>>>>>     7 block: do not include monitor.h
>>>>>     7 stubs: remove mon-protocol-event.o in stub obj
>>>>>
>>>>>    block.c                    |   22 ++++++++++++++++++----
>>>>>    block/qcow2-refcount.c     |    4 +++-
>>>>>    blockjob.c                 |   10 ++++++++--
>>>>>    include/block/block.h      |   12 ++++++++++++
>>>>>    include/block/block_int.h  |    3 +--
>>>>>    include/monitor/monitor.h  |   40
>>>>> ++--------------------------------------
>>>>>    include/qapi/qmp/qevent.h  |   41
>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>    include/qapi/qmp/types.h   |    1 +
>>>>>    monitor.c                  |   12 ++++++------
>>>>>    stubs/Makefile.objs        |    1 -
>>>>>    stubs/mon-protocol-event.c |    2 +-
>>>>>    tests/Makefile             |    3 ++-
>>>>>    ui/vnc.c                   |    2 +-
>>>>>    vl.c                       |    4 ++++
>>>>>    14 files changed, 100 insertions(+), 57 deletions(-)
>>>>>    create mode 100644 include/qapi/qmp/qevent.h
>>>>>
>>>> Patches 1-4 look good.  I'm not sure of the advantage of the last 
>>>> four,
>>>> however.  The ugly part of monitor_protocol_event is not really the
>>>> stub, but the dependency on QObject.
>>> I think replacing QObject with QAPI types is another issue we could
>>> improve. The last
>>> foure patches tries decouple monitor code with block layer code from
>>> build time.
>>> The files in ./stubs is needed since some symbols are needed in some
>>> program which don't
>>> need it really, and I think, that tips the code are not organized very
>>> well in .c file level.
>> That may very well be the case.  However, picking a function, replacing
>> it with a function pointer, and throwing it into a struct, will not
>> improve the structure at the .c file level very much.
>>
>> It does not abstract anything.  In order to provide a useful
>> abstractions, the questions to answer are:
>>
>> 1) If a library wanted to provide a callback for events, would the
>> current design (using QObject) be the right thing to do?  (If you change
>> the design, it might happen that the stub goes away naturally and that
>   I think it would not goes away even QAPI is used. The ./stubs is a 
> symbol
> issue at link time, so whenever block layer use a symbol belong to 
> monitor component,
> the stubs would be needed.
>   In my opinion, there could be two steps: remove this symbol in block 
> code,
> replace it with QAPI in monitor code.
>
>> the work in these patches does nothing except obscure the history).
>>
>> 2) Are there other services that the block layer could desire from the
>> monitor?
>>
>   I have a draft makefile which link the core block code, it
> shows that only event and error printf(include mon_is_qmp) service
> are used by block code now. Those symbols basically are listed in 
> ./stubs.
>
>> 3) Could any tool (e.g. qemu-io) desire to implement
>> monitor_protocol_event?  If so, what other services could it provide
>> that the QEMU monitor provides?
>>
>   It seems tools do not need event service now, they link with stubs.
>
>>> After removing ./stubs, the code will be more clear
>> If done in the right way, yes.  If done in the wrong way, the code will
>> be less clear.
>>
>> Paolo
>>
>
>

Hi Paolo,
   This series tries to find a mechanism, which can avoid link with unused
symbols. Assume QObject was replaced with other design, such as QAPI, it
will still call monitor funcions to emit the event, so it will still require
resolving that function in linking of qemu-img.
   Do you have some suggestion for it?

Re: [Qemu-devel] [RFC PATCH 3/8] qapi: move MonitorEvent define

[Wenchao Xia]

于 2013/9/23 11:55, Eric Blake 写道:
> On 09/12/2013 03:15 AM, Wenchao Xia wrote:
>> Signed-off-by: Wenchao Xia<xiawenc@linux.vnet.ibm.com>
>> ---
>> new file mode 100644
>> index 0000000..aef71d9
>> --- /dev/null
>> +++ b/include/qapi/qmp/qevent.h
>> @@ -0,0 +1,41 @@
>> +#ifndef QEVENT_H
> No copyright notice?  Fix that.
>
  Will add one in next version.