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 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 >
