qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Wenchao Xia
Subject: Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block
Date: Mon, 23 Sep 2013 15:06:51 -0400
User-agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; zh-CN; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9

于 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?




reply via email to

[Prev in Thread] Current Thread [Next in Thread]