From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49876) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0soa-0006kh-I8 for qemu-devel@nongnu.org; Mon, 13 Aug 2012 07:28:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0soZ-0004rW-3C for qemu-devel@nongnu.org; Mon, 13 Aug 2012 07:28:08 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:47036) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0soY-0004rH-9e for qemu-devel@nongnu.org; Mon, 13 Aug 2012 07:28:07 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 13 Aug 2012 21:27:48 +1000 Received: from d23av03.au.ibm.com (d23av03.au.ibm.com [9.190.234.97]) by d23relay04.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q7DBJSgC11796708 for ; Mon, 13 Aug 2012 21:19:29 +1000 Received: from d23av03.au.ibm.com (loopback [127.0.0.1]) by d23av03.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q7DBRxMN027951 for ; Mon, 13 Aug 2012 21:28:00 +1000 Message-ID: <5028E4B6.5020708@linux.vnet.ibm.com> Date: Mon, 13 Aug 2012 19:27:50 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1344507050-11054-1-git-send-email-xiawenc@linux.vnet.ibm.com> <5024C096.9040007@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [qemu-devel] [PATCH V2 0/3] [RFC] libqblock draft code v2 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: stefanha@gmail.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com 于 2012-8-11 20:18, Blue Swirl 写道: > On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia wrote: >> Thanks for your review, sorry I have forgot some fixing you >> mentioned before, will correct them this time. >> >> 于 2012-8-10 1:12, Blue Swirl 写道: >> >>> On Thu, Aug 9, 2012 at 10:10 AM, Wenchao Xia >>> wrote: >>>> >>>> This patch intrudce libqblock API, libqblock-test is used as a test >>>> case. >>>> >>>> V2: >>>> Using struct_size and [xxx]_new [xxx]_free to keep ABI. >>>> Using embbed structure to class format options in image creating. >>>> Format specific options were brought to surface. >>>> Image format was changed to enum interger instead of string. >>>> Some API were renamed. >>>> Internel error with errno was saved and with an API caller can get it. >>>> ALL flags used were defined in libqblock.h. >>>> >>>> Something need discuss: >>>> Embbed structure or union could make the model more friendly, but that >>>> make ABI more difficult, because we need to check every embbed >>>> structure's >>>> size and guess compiler's memory arrangement. This means #pragma pack(4) >>>> or struct_size, offset_next in every structure. Any better way to solve >>>> it? >>>> or make every structure a plain one? >>> >>> >>> I'd still use accessor functions instead of structure passing, it >>> would avoid these problems. >>> >> Do you mean some function like : >> CreateOption_Filename_Set(const char *filename) >> CreateOption_Format_Set(const char *filename) > > Something like this: > int qb_create_cow(struct QBlockState *qbs, const char *filename, > size_t virt_size, const char *backing_file, int backing_flag); > etc. for rest of the formats. > > Alternatively, we could have more generic versions like you describe, > or even more generic still: > > void qb_set_property(struct QBlockState *qbs, const char *prop_name, > const char *prop_value); > > so the create sequence (ignoring error handling) would be: > s = qb_init(); > qb_set_property(s, "filename", "c:\\\\autoexec.bat"); > qb_set_property(s, "format", "cow"); > qb_set_property(s, "virt_size", "10GB"); > // use defaults for backing_file and backing_flag > qb_create(s); > > Likewise for info structure: > char *qb_get_property(struct QBlockState *qbs, const char *prop_name); > > foo = qb_get_property(s, "format"); > foo = qb_get_property(s, "encrypted"); > foo = qb_get_property(s, "num_backing_files"); > foo = qb_get_property(s, "virt_size"); > > This would be helpful for the client to display parameters without > much understanding of their contents: > char **qb_list_properties(struct QBlockState *qbs); /* returns array > of property names available for this file, use get_property to > retrieve their contents */ > > But the clients can't be completely ignorant of all formats available, > for example a second UI dialog needs to be added for formats with > backing files, otherwise it won't be able to access some files at all. > Maybe by adding type descriptions for each property (type for > "filename" is "path", for others "string", "bool", "enum" etc). > Thanks. This seems heavy document is needed for that no structure can indicate what options sub format have, user can only get that info from returns or documents. I am not sure if this is good, because it looks more like a object oriented API that C. >> >> It can solve the issue, with a cost of more small APIs in header >> files that user should use. Not sure if there is a good way to make >> it more friendly as an object language: >> "oc.filename = name;" automatically call CreateOption_Filename_Set, >> API CreateOption_Filename_Set is invisible to user. >> >> >> >>> Packing can even introduce a new set of problems since we don't >>> control the CFLAGS of the client of the library. >> >> >> indeed, I tried to handle the difference in function qboo_adjust_o2n, >> found that structure member alignment is hard to deal. >> >> >>>> AIO is missing, need a prototype. >>>> >>>> Wenchao Xia (3): >>>> adding libqblock >>>> libqblock API >>>> libqblock test case >>>> >>>> Makefile | 3 + >>>> block.c | 2 +- >>>> block.h | 1 + >>>> libqblock-test.c | 197 ++++++++++++++++ >>>> libqblock.c | 670 >>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >>>> libqblock.h | 447 ++++++++++++++++++++++++++++++++++++ >>>> 6 files changed, 1319 insertions(+), 1 deletions(-) >>>> create mode 100644 libqblock-test.c >>>> create mode 100644 libqblock.c >>>> create mode 100644 libqblock.h >>>> >>>> >>>> >>> >> >> >> -- >> Best Regards >> >> Wenchao Xia >> > -- Best Regards Wenchao Xia