On Tue, Aug 14, 2012 at 8:52 AM, Wenchao Xia <xiaw...@linux.vnet.ibm.com> wrote: > 于 2012-8-14 4:00, Blue Swirl 写道: > >> On Mon, Aug 13, 2012 at 11:27 AM, Wenchao Xia >> <xiaw...@linux.vnet.ibm.com> wrote: >>> >>> 于 2012-8-11 20:18, Blue Swirl 写道: >>> >>>> On Fri, Aug 10, 2012 at 8:04 AM, Wenchao Xia >>>> <xiaw...@linux.vnet.ibm.com> >>>> 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 >>>>>> <xiaw...@linux.vnet.ibm.com> >>>>>> 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. >> >> >> 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 is hard to extend more options, if for every format a API is needed, > then > int qb_create_cow(struct QBlockState *qbs, struct QBlockOptionFmtCow *op) > seems better, while keep struct QBlockOptionFmtCow a plain struct > without embbed struct(using pointer instead).
OK, practically there isn't much difference and if a lot of parameters are needed, a structure is needed anyway. > > >>> >>> >>> >>>>> >>>>> 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 >>> >> > > > -- > Best Regards > > Wenchao Xia >
