From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43788) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T10pA-0003Jv-2j for qemu-devel@nongnu.org; Mon, 13 Aug 2012 16:01:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T10p5-00005Y-7u for qemu-devel@nongnu.org; Mon, 13 Aug 2012 16:01:16 -0400 Received: from mail-qa0-f45.google.com ([209.85.216.45]:52912) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T10p5-00005S-3d for qemu-devel@nongnu.org; Mon, 13 Aug 2012 16:01:11 -0400 Received: by qadc10 with SMTP id c10so2418866qad.4 for ; Mon, 13 Aug 2012 13:01:10 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5028E4B6.5020708@linux.vnet.ibm.com> References: <1344507050-11054-1-git-send-email-xiawenc@linux.vnet.ibm.com> <5024C096.9040007@linux.vnet.ibm.com> <5028E4B6.5020708@linux.vnet.ibm.com> From: Blue Swirl Date: Mon, 13 Aug 2012 20:00:49 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: Wenchao Xia Cc: stefanha@gmail.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, pbonzini@redhat.com On Mon, Aug 13, 2012 at 11:27 AM, Wenchao Xia wrote: > =E4=BA=8E 2012-8-11 20:18, Blue Swirl =E5=86=99=E9=81=93: > >> 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. >>> >>> =E4=BA=8E 2012-8-10 1:12, Blue Swirl =E5=86=99=E9=81=93: >>> >>>> On Thu, Aug 9, 2012 at 10:10 AM, Wenchao Xia >>>> >>>> wrote: >>>>> >>>>> >>>>> This patch intrudce libqblock API, libqblock-test is used as a te= st >>>>> 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 ge= t >>>>> 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 sol= ve >>>>> 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 =3D 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 =3D qb_get_property(s, "format"); >> foo =3D qb_get_property(s, "encrypted"); >> foo =3D qb_get_property(s, "num_backing_files"); >> foo =3D 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. This approach may be a bit over-engineered, but it may be simpler to tie to an UI. What do you think of the simple version then: int qb_create_cow(struct QBlockState *qbs, const char *filename, size_t virt_size, const char *backing_file, int backing_flag); > > > >>> >>> 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 =3D 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 >