From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33243) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0AeJ-0001YU-HV for qemu-devel@nongnu.org; Sat, 11 Aug 2012 08:18:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T0AeH-0000dG-9w for qemu-devel@nongnu.org; Sat, 11 Aug 2012 08:18:35 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:52458) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T0AeH-0000d8-4J for qemu-devel@nongnu.org; Sat, 11 Aug 2012 08:18:33 -0400 Received: by yenm4 with SMTP id m4so2200864yen.4 for ; Sat, 11 Aug 2012 05:18:32 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5024C096.9040007@linux.vnet.ibm.com> References: <1344507050-11054-1-git-send-email-xiawenc@linux.vnet.ibm.com> <5024C096.9040007@linux.vnet.ibm.com> From: Blue Swirl Date: Sat, 11 Aug 2012 12:18:11 +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 Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia w= rote: > 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 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 i= t. >>> ALL flags used were defined in libqblock.h. >>> >>> Something need discuss: >>> Embbed structure or union could make the model more friendly, but th= at >>> 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 =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). > > 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 >