From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57014) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLl0u-0000J1-Cv for qemu-devel@nongnu.org; Mon, 16 Sep 2013 22:27:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VLl0l-0004zA-EW for qemu-devel@nongnu.org; Mon, 16 Sep 2013 22:27:40 -0400 Received: from e28smtp02.in.ibm.com ([122.248.162.2]:60440) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VLl0k-0004xT-PH for qemu-devel@nongnu.org; Mon, 16 Sep 2013 22:27:31 -0400 Received: from /spool/local by e28smtp02.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 17 Sep 2013 07:57:20 +0530 Received: from d28relay01.in.ibm.com (d28relay01.in.ibm.com [9.184.220.58]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id A1F431258051 for ; Tue, 17 Sep 2013 07:57:23 +0530 (IST) Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay01.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r8H2TKag37945536 for ; Tue, 17 Sep 2013 07:59:22 +0530 Received: from d28av01.in.ibm.com (localhost [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r8H2RFU5015222 for ; Tue, 17 Sep 2013 07:57:15 +0530 Message-ID: <5237BE02.5030001@linux.vnet.ibm.com> Date: Tue, 17 Sep 2013 10:27:14 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1378977312-17696-1-git-send-email-xiawenc@linux.vnet.ibm.com> <523189F2.6050107@redhat.com> <5236904B.1050907@linux.vnet.ibm.com> <5236D743.1000704@redhat.com> In-Reply-To: <5236D743.1000704@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH 0/8] Remove stub mon-protocol-event for block List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, anthony@codemonkey.ws, qemu-devel@nongnu.org, stefanha@redhat.com 于 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 >