Re: [Qemu-devel] [PATCH 2/6] qapi: rename MonitorEvent to QEvent

[Wenchao Xia]

于 2013/11/4 21:33, Luiz Capitulino 写道:
> On Mon, 04 Nov 2013 09:59:50 +0800
> Wenchao Xia <address@hidden> wrote:
>
> > 于 2013/11/1 22:02, Luiz Capitulino 写道:
> > > On Mon, 21 Oct 2013 10:16:01 +0800
> > > Wenchao Xia <address@hidden> wrote:
> > >
> > > > Signed-off-by: Wenchao Xia <address@hidden>
> > > > ---
> > > >    block.c                    |    2 +-
> > > >    include/block/block_int.h  |    2 +-
> > > >    include/monitor/monitor.h  |    6 +++---
> > > >    monitor.c                  |   12 ++++++------
> > > >    stubs/mon-protocol-event.c |    2 +-
> > > >    ui/vnc.c                   |    2 +-
> > > >    6 files changed, 13 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/block.c b/block.c
> > > > index 2c15e5d..458a4f8 100644
> > > > --- a/block.c
> > > > +++ b/block.c
> > > > @@ -1760,7 +1760,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 bcc72e2..bfdaf84 100644
> > > > --- a/include/block/block_int.h
> > > > +++ b/include/block/block_int.h
> > > > @@ -337,7 +337,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 10fa0e3..8b14a6f 100644
> > > > --- a/include/monitor/monitor.h
> > > > +++ b/include/monitor/monitor.h
> > > > @@ -20,7 +20,7 @@ extern Monitor *default_mon;
> > > >    #define MONITOR_CMD_ASYNC       0x0001
> > > >
> > > >    /* QMP events */
> > > > -typedef enum MonitorEvent {
> > > > +typedef enum QEvent {
> > >
> > > Qt has a QEvent class, so QEvent is not a good name for us if we're
> > > considering making it public in the schema (which could become an
> > > external library in the distant future).
> > >
> > > I suggest calling it QMPEvent.
> >
> >     Maybe QMPEventType, since QMPEvent should be used an union?
>
> If we add the 'event' type, like:
>
> { 'event': 'BLOCK_IO_ERROR', 'data': { ... } }
>
> Then we don't need an union.

  It would bring a little trouble to C code caller, for example the
event generate function(just like monitor_protocol_event) would be:
event_generate(QMPEvent *e);
  We may need to make *e as opaque and handly writing code for "switch
", but with union, the generated code would do that for us, so I think
union is better.