From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39341) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THQot-0006Cc-NG for qemu-devel@nongnu.org; Thu, 27 Sep 2012 23:00:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1THQos-0005bV-Fd for qemu-devel@nongnu.org; Thu, 27 Sep 2012 23:00:51 -0400 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:37768) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1THQor-0005ZD-JZ for qemu-devel@nongnu.org; Thu, 27 Sep 2012 23:00:50 -0400 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 28 Sep 2012 08:30:43 +0530 Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q8S30ftH39583864 for ; Fri, 28 Sep 2012 08:30:41 +0530 Received: from d28av03.in.ibm.com (loopback [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q8S30eO4030130 for ; Fri, 28 Sep 2012 13:00:41 +1000 Message-ID: <506512B8.5090903@linux.vnet.ibm.com> Date: Fri, 28 Sep 2012 11:00:08 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1348712642-4427-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1348712642-4427-3-git-send-email-xiawenc@linux.vnet.ibm.com> <50640D90.7040005@redhat.com> <506421E8.2020105@linux.vnet.ibm.com> <50642765.7090900@redhat.com> In-Reply-To: <50642765.7090900@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V4 2/5] libqblock type defines List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, eblake@redhat.com 于 2012-9-27 18:16, Paolo Bonzini 写道: > Il 27/09/2012 11:52, Wenchao Xia ha scritto: >>> >>> Please use QBO_ instead of QB_ throughout. Also write COMPAT instead of >>> CPT, and remove CPT_NONE since 0.10 is the default: >>> >> __NONE is used to indicate whether this property is set or get, so >> it is actually have 3 status than just 2: Not set, V010, V110. It >> is the same reason that I use __NONE in bool values, especially __NONE >> could represent it is not got in information retrieving. > > Yes, that I guessed. I thought in many cases we can anticipate that the > default is not going to change, but perhaps it's better to be safe > (which is what you did). > > Please do change FALSE/TRUE to OFF/METADATA for preallocation enums, and > please remove MONOLITHIC from QBO_FMT_VMDK_SUBFMT_MONOLITHIC_NONE. > OK, these should be changed. >> Maybe I should rename them to __NOTSET. > > Perhaps "_DEFAULT" is even bette? > > Paolo > Yep, _DEFAULT seems good to me. >>>> >>>> +typedef struct QBlockStaticInfoAddr { >>>> + uint64_t *virt_size; >>>> + QBlockProtInfo *backing_loc; >>>> + bool *encrypt; >>>> +} QBlockStaticInfoAddr; >>> >>> Why the indirection? >>> >> It helps user to get these important members, otherwise >> user will need >> Switch (fmt) { >> case RAW: >> .... >> case QCOW2: >> ... >> } >> for every attribute. The indirection address will let user directly >> access the members. > > Ah, ok, now I understand. Interesting. An alternative could be to add > generic accessors like these: > > uint64_t qblock_get_virt_size(QBlockFmtInfo *fmt); > QBlockProtInfo *qblock_get_backing_loc(QBlockFmtInfo *fmt); > bool qblock_get_encrypt(QBlockFmtInfo *fmt); > > etc. that include the switch statement. > I think it is reasonable use generic accessors instead of embbed structure, maybe an additional helper function should be added as: QBlockStaticInfoAddr *qblock_get_static_info_addr(QBlockStaticInfo *info) In this way only one switch statement is needed in source code, and only one new API is added. > Paolo > -- Best Regards Wenchao Xia